fix(form-core): keep reset(values) baseline on update() with unchanged props defaultValues#2235
fix(form-core): keep reset(values) baseline on update() with unchanged props defaultValues#2235xianjianlf2 wants to merge 1 commit into
Conversation
…d props defaultValues Calling form.reset(newValues) inside onSubmit while a component subscribes to isDirty reverted the form to the original default values instead of the new ones. The isDirty change re-renders the component, re-running the formApi.update(opts) layout effect with the unchanged props defaultValues. update() compared against this.options.defaultValues, which reset() had just mutated, so the unchanged props looked like a real change and (because reset also cleared isTouched) clobbered the freshly reset values. Track the defaultValues seen on the previous update() call and compare against that instead; when the props defaultValues did not actually change, preserve the current baseline (e.g. one set by reset(values)) so values and isDirty/isDefaultValue stay correct after a reset. Fixed in form-core so it covers all framework adapters. Fixes TanStack#1681 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughFormApi now tracks the defaultValues baseline from the previous update() call in a new private field, using it to detect genuine prop changes versus reset()-driven mutations. This prevents update() from overriding values set by reset() calls made inside onSubmit. Tests added in form-core and react-form. ChangesReset/update defaultValues fix
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/form-core/tests/FormApi.spec.ts (1)
1235-1269: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider asserting
isDirtyafterupdate()to verify the PR objective at the unit level.The test verifies the reset value survives the subsequent
update(), but doesn't explicitly assertisDirtyafter theupdate()call. The PR objectives state thatisDirtyshould be correct after reset. Adding this assertion at the FormApi level would catch regressions even if the React adapter test is skipped.💚 Suggested addition
// the reset values must survive: props defaultValues never actually changed expect(form.getFieldValue('name')).toEqual('new-default') + expect(form.state.isDirty).toBe(false) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/form-core/tests/FormApi.spec.ts` around lines 1235 - 1269, The FormApi regression test covers that `reset()` values survive a later `update()`, but it does not verify the dirty state after the update. In `FormApi.spec.ts`, extend the `it('should keep reset() values when a subsequent update() reuses the original props defaultValues')` case to assert `form.state.isDirty` after `form.update(...)`, so the unit test directly checks the PR’s intended `isDirty` behavior in `FormApi.reset()`/`FormApi.update()`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/form-core/tests/FormApi.spec.ts`:
- Around line 1235-1269: The FormApi regression test covers that `reset()`
values survive a later `update()`, but it does not verify the dirty state after
the update. In `FormApi.spec.ts`, extend the `it('should keep reset() values
when a subsequent update() reuses the original props defaultValues')` case to
assert `form.state.isDirty` after `form.update(...)`, so the unit test directly
checks the PR’s intended `isDirty` behavior in
`FormApi.reset()`/`FormApi.update()`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1f38254d-e2a8-4cc0-b614-2cfa9926663a
📒 Files selected for processing (3)
packages/form-core/src/FormApi.tspackages/form-core/tests/FormApi.spec.tspackages/react-form/tests/useForm.test.tsx
Fixes #1681
Problem
Calling
form.reset(newValues)insideonSubmitwhile a component subscribes toisDirtyreverts the form to the original default values instead of the new ones.The
isDirtychange re-renders the component, which re-runs theformApi.update(opts)layout effect with the unchanged propsdefaultValues.update()compared the incomingdefaultValuesagainstthis.options.defaultValues— butreset(values)had just mutated that field, so the unchanged props looked like a real change and (becauseresetalso clearedisTouched) clobbered the freshly-reset values back to the original defaults.Fix
Track the
defaultValuesseen on the previousupdate()call and compare against that instead. When the propsdefaultValuesdid not actually change, preserve the current baseline (e.g. one set byreset(values)) so the values — andisDirty/isDefaultValue— stay correct after a reset. Fixed inform-core, so the fix covers all framework adapters.Tests
form-core: regression test for theupdate()/reset()race.react-form: integration test reproducing the exact issue (component-levelisDirtysubscription +resetinonSubmit).Both tests fail on
mainand pass with this change. Fullform-core(499) andreact-form(124) suites pass with no type errors.Summary by CodeRabbit