refactor(core): split useForm internals by concern#278
Conversation
📝 WalkthroughWalkthroughThe PR refactors form management by extracting monolithic logic from useForm.ts into specialized internal modules for array fields, bindings, state, submission, and wizards. The public API remains unchanged while internal complexity is distributed across focused modules. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes The refactoring spans five new substantial modules with intricate state management, validation orchestration across sync/async paths, wizard step transitions, and field array operations. Multiple interdependent flows require verification of correctness, state immutability, and error propagation patterns. Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/forms/internal/arrayState.ts`:
- Around line 168-181: The array branch logic (in append() and move()) is
reusing prev.dirty[fieldKey] slices instead of recomputing dirtiness for each
index against the initial values; update the code that currently builds
nextDirty (the blocks around nextValuesArray, previousDirty, appendedDirty) to
call computeFieldDirty(fieldKey, nextValuesArray,
options.initialValuesRef.current) (or equivalent) so dirty flags are derived
from nextValuesArray and initialValuesRef.current rather than splicing
prev.dirty[fieldKey]; ensure the move() and append() sites (and the similar
blocks at the noted ranges) use computeFieldDirty instead of reshaping
previousDirty so isDirty stays correct after move/remove/append.
- Around line 187-190: When appending/removing/moving array elements the code
currently calls normalizeErrorArray(prev.errors[fieldKey], ...) which converts
scalar (string) array-level errors into undefined[] and clears messages when
validateOnChange is disabled; instead, add the same guard used in remove() and
move(): if validateOnChange is false and prev.errors[fieldKey] is a scalar (not
an array), preserve prev.errors[fieldKey] as-is for nextErrors[fieldKey];
otherwise call normalizeErrorArray and update the array (e.g., for the append
branch that builds nextErrors = {...prev.errors, [fieldKey]: [...currentErrors,
undefined]}). Apply this same conditional logic to the other affected blocks
(the similar append/remove/move handling at the noted sections).
In `@packages/core/src/forms/internal/submit.ts`:
- Around line 32-44: The reset can race with in-flight async submit/validation;
update createSubmitAction to capture a unique "attempt" token (e.g., read a new
local const attempt = options.attemptRef.current or increment a local counter)
before each async flow (validation and onSubmit), and after every await
immediately check that the current attempt/token still matches and bail out if
not; ensure all post-await branches that set errors, submitError, call
finishSuccessfulSubmit(), or call updateFormState are skipped when the token is
stale, and keep createResetAction unchanged except that it should update the
shared attempt/token (so reset invalidates ongoing attempts).
In `@packages/core/src/forms/internal/wizard.ts`:
- Around line 172-192: runWizardStepValidation is incorrectly merging stale
errors from options.source.errors for the same stepFields back into the current
step validation result; remove the merge that re-inserts
pickValidationFields(options.filterDisabledValidationErrors(options.source.errors,
options.source), stepFields) so that current-step validation only returns
syncStepErrors and the current step's customStepErrors (and do not reapply old
source errors for those same fields). If you still need to preserve errors for
other steps, keep merging options.source.errors only for fields outside
stepFields (i.e., exclude stepFields before merging) and use the existing
helpers mergeValidationErrors and pickValidationFields to implement that
exclusion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c3dbb22e-910e-4279-97eb-0f6556cdbc9e
📒 Files selected for processing (7)
packages/core/src/forms/internal/arrayState.tspackages/core/src/forms/internal/bindings.tspackages/core/src/forms/internal/dev.tspackages/core/src/forms/internal/state.tspackages/core/src/forms/internal/submit.tspackages/core/src/forms/internal/wizard.tspackages/core/src/forms/useForm.ts
Summary
packages/core/src/forms/useForm.tsinto internal helper modules.useFormAPI unchanged.Why
Validation
ln -s /home/k3nig/Rezi/node_modules /home/k3nig/Rezi-split-useform/node_modulesnpm run lintnpm run typechecknpm run buildnode scripts/run-tests.mjs --filter "packages/core/dist/forms/__tests__/"rm node_modulesSummary by CodeRabbit
Release Notes