fix(plugin-form): a wizard that ends on a field-less review step can finish - #2986
Merged
Merged
Conversation
…finish
A wizard step with no inputs — `{ label: 'Review', fields: [] }`, the shape
this component's own usage example ends on — rendered a bare hint div and NO
`<form>`. The footer Next/Create buttons submit the step form natively
(`type="submit" form={stepFormId}`), so on that step they pointed at a target
that did not exist: clicking Create did nothing at all and the wizard could
never be completed. Observed directly — `create` call count 0, no error, no
feedback.
This is fallout from the fix the regression suite guards: moving the buttons
off `onClick={() => handleStepSubmit(formData)}` onto native submission is what
made a missing form fatal. Render a real form on the field-less step; it
contributes no values of its own, so the record is whatever the earlier steps
accumulated.
Also correct that suite's docblock, which overstated what it covers. Two
redundant mechanisms carry values across steps (react-hook-form retains
unmounted fields; the wizard merges `{...formData, ...stepData}`), and
"submits the merged payload" passes when EITHER is broken alone — it catches a
total failure to accumulate, not a broken merge. The new field-less-step case
is what pins the merge down: with no react-hook-form instance on that step,
`formData` is the only carrier left. Verified by mutation — breaking only the
merge now fails with `expected {} to deeply equal {"name": "Alice"}`, and
reverting the form fix fails with `expected null not to be null`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #2983. While auditing what that regression suite actually covers, I found a reachable product bug.
The bug
A wizard step with no inputs —
{ label: 'Review', fields: [] }, the shapeWizardForm's own JSDoc example ends on — rendered a bare hint div and no<form>. The footer Next/Create buttons submit the step form natively (type="submit" form={stepFormId}), so on that step they pointed at a target that does not exist.Observed directly:
Clicking Create does nothing at all — no request, no error, no feedback. A wizard that ends on a review step can never be completed.
This is fallout from the very fix the regression suite guards: moving the buttons off
onClick={() => handleStepSubmit(formData)}onto native submission is what made a missing form fatal. Fix renders a real form on the field-less step; it contributes no values of its own, so the record is whatever the earlier steps accumulated.The docblock was overstating its coverage
The suite claimed to guard "the MERGED payload from every step". It doesn't. Two redundant mechanisms carry values across steps:
shouldUnregisteris false){...formData, ...stepData}Breaking either one alone still passes — I had to break both to make it fail. So
submits the merged payloadcatches a total failure to accumulate, not a broken merge. I also confirmed RHF carries everything even across Back/Next navigation.The new field-less-step case is what actually pins the merge down: that step has no react-hook-form instance, so
formDatais the only carrier left. Docblock rewritten to say what each case really covers.Verification
Mutation-tested, each reverted after:
expected null not to be nullmergedData = {...stepData})expected {} to deeply equal {"name": "Alice"}That second one is the point — the pre-existing test could not detect it.
Full
plugin-formsuite: 24 files / 218 tests pass. Product edit adds zero lint warnings; the 2 new ones areno-explicit-anyon the new test'sas anycasts, matching the 14 pre-existing instances in that file. Pure bug fix, so no changeset per AGENTS.md §9.🤖 Generated with Claude Code