Skip to content

fix(form-core): fix DeepValue from record values being wrong #1558

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

LeCarbonator
Copy link
Contributor

@LeCarbonator LeCarbonator commented Jun 5, 2025

Closes #1551

This PR fixes a wrong intersection that occurs when iterating over template strings (such as records).
The issue arose because of TypeScript inferring the following:

type DeepKeysExample = 'records' | `records.${string}` | `records.${string}.name`

type GetValueFrom = 'records.foo.name'
// This matches both `records.${string}.name` as well as `records.${string}`.

// Generated DeepValue: 
// { name: string } & string

// With PR fix:
// string

Important

TypeScript has a limitation with template string types. Nested records are not yet safe. Example below:

type NestedRecord = {
  numberRecord: Record<string, number>
  objectRecord: Record<string, { age: number }>
}

type RecordWrapper = {
  records: Record<string, NestedRecord>
}

type Test1 = DeepValue<RecordWrapper, 'records.foo.numberRecord.bar'>
// Type: NestedRecord & number
// `records.${'foo'}.numberRecord.${'bar'}`
// `records.${'foo.numberRecord.bar'}`

type Test2 = DeepValue<RecordWrapper, 'records.foo.objectRecord.bar'>
// Type: NestedRecord & { age: number }
// `records.${'foo'}.objectRecord.${'bar'}`
// `records.${'foo.objectRecord.bar'}`

type Test3 = DeepValue<RecordWrapper, 'records.foo.objectRecord.bar.age'>
// Type: number
// `records.${'foo'}.objectRecord.${'bar'}.age`

Copy link

nx-cloud bot commented Jun 5, 2025

View your CI Pipeline Execution ↗ for commit 07b3377.

Command Status Duration Result
nx affected --targets=test:sherif,test:knip,tes... ✅ Succeeded 1m 25s View ↗
nx run-many --target=build --exclude=examples/** ✅ Succeeded 22s View ↗

☁️ Nx Cloud last updated this comment at 2025-06-05 16:15:23 UTC

Copy link

pkg-pr-new bot commented Jun 5, 2025

More templates

@tanstack/angular-form

npm i https://pkg.pr.new/@tanstack/angular-form@1558

@tanstack/form-core

npm i https://pkg.pr.new/@tanstack/form-core@1558

@tanstack/lit-form

npm i https://pkg.pr.new/@tanstack/lit-form@1558

@tanstack/react-form

npm i https://pkg.pr.new/@tanstack/react-form@1558

@tanstack/solid-form

npm i https://pkg.pr.new/@tanstack/solid-form@1558

@tanstack/svelte-form

npm i https://pkg.pr.new/@tanstack/svelte-form@1558

@tanstack/vue-form

npm i https://pkg.pr.new/@tanstack/vue-form@1558

commit: 07b3377

Copy link

codecov bot commented Jun 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.24%. Comparing base (77acb33) to head (07b3377).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1558   +/-   ##
=======================================
  Coverage   89.24%   89.24%           
=======================================
  Files          31       31           
  Lines        1432     1432           
  Branches      366      366           
=======================================
  Hits         1278     1278           
  Misses        137      137           
  Partials       17       17           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@LeCarbonator LeCarbonator marked this pull request as ready for review June 8, 2025 11:50
@binajmen
Copy link

binajmen commented Aug 5, 2025

thank you for fixing this! is there a timeline on when it will be included in a release? 👀

@LeCarbonator
Copy link
Contributor Author

@binajmen Well, the main issue is that this isn't really a fix. This PR has been a bit stale because of that ... but I'm not sure there will be a convenient solution that doesn't need reworking all of the DeepKeys types

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Records fields have wrong types
2 participants