Skip to content

fix(form-core): Only allow array deep keys in array methods #1495

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 9, 2025

Conversation

LeCarbonator
Copy link
Contributor

This PR restricts array methods on the FormApi to only allow paths to array keys.

Consider the following example:

type FormValues = {
    name: string
    age: number
    startDate: Date
    title: string | null | undefined
    relatives: { name: string }[]
    counts: (number | null | undefined)[]
  }

  const defaultValues: FormValues = {
    name: '',
    age: 0,
    startDate: new Date(),
    title: null,
    relatives: [{ name: '' }],
    counts: [5, null, undefined, 3],
  }

  const form = new FormApi({
    defaultValues,
  })

What's the behaviour on v1.10?

With the setup given above, you can write the following commands:

  const fieldValue = {}
  // The field value is typed as `never`, so you're informed that it's not correct
  form.pushFieldValue('age', fieldValue as never)
  form.insertFieldValue('age', 0, fieldValue as never)
  form.replaceFieldValue('age', 0, fieldValue as never)
  // No error! It's silently ignored in the implementation
  form.removeFieldValue('age', 0)
  form.swapFieldValues('age', 0, 1)
  form.moveFieldValues('age', 0, 1)

What would change?

  const fieldValue = {}
  // Error: Argument of type '"age"' is not assignable to parameter of type '"relatives" | "counts"'.ts(2345)
  form.pushFieldValue('age', fieldValue)
  form.insertFieldValue('age', 0, fieldValue)
  form.replaceFieldValue('age', 0, fieldValue)
  form.removeFieldValue('age', 0)
  form.swapFieldValues('age', 0, 1)
  form.moveFieldValues('age', 0, 1)

How does it work?

This implementation first determines DeepKeys<T> and then extracts the keys that are of the desired type. Therefore, it should not have a big impact on performance or have problems with future fixes to DeepKeys<T>

This type will help with a lens API for forms
so that only names leading to the subset data
are allowed.
Copy link

nx-cloud bot commented May 9, 2025

View your CI Pipeline Execution ↗ for commit 3d15d5d.

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

☁️ Nx Cloud last updated this comment at 2025-05-09 18:51:39 UTC

Copy link

pkg-pr-new bot commented May 9, 2025

More templates

@tanstack/angular-form

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

@tanstack/form-core

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

@tanstack/react-form

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

@tanstack/lit-form

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

@tanstack/solid-form

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

@tanstack/vue-form

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

@tanstack/svelte-form

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

commit: 3d15d5d

Copy link

codecov bot commented May 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.13%. Comparing base (9143ed6) to head (3d15d5d).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1495   +/-   ##
=======================================
  Coverage   89.13%   89.13%           
=======================================
  Files          31       31           
  Lines        1417     1417           
  Branches      362      362           
=======================================
  Hits         1263     1263           
  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 force-pushed the feat-deep-keys-of-type branch from a0247f4 to 95176b2 Compare May 9, 2025 10:06
@LeCarbonator LeCarbonator reopened this May 9, 2025
@LeCarbonator
Copy link
Contributor Author

Not sure why it closed? It claims I did it, so it was unintentional.

Copy link
Member

@Balastrong Balastrong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's brilliant, thank you!

@Balastrong Balastrong changed the title Fix(form-core): Only allow array deep keys in array methods fix(form-core): Only allow array deep keys in array methods May 9, 2025
@Balastrong Balastrong merged commit 08c9bed into TanStack:main May 9, 2025
6 checks passed
@LeCarbonator LeCarbonator deleted the feat-deep-keys-of-type branch May 10, 2025 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants