Repository navigation
refactor(onboarding): build benefits and engagement agreement details once through useHeadlessForm - #1442
Merged
gabrielseco merged 3 commits intoOct 5, 2026
Conversation
… once through useHeadlessForm Both schema hooks now fetch the raw schema and hand it to useHeadlessForm with the buildOnce strategy, seeded with the saved values (partner initialValues plus the saved benefit offers / engagement agreement details). Validation, submit parsing and value changes for both steps go through the hook instead of the hand-written parseJSFToValidate branches in hooks.tsx, and the forms are no longer rebuilt on every keystroke from the live field values. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…d to buildOnce Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
📦 Bundle Size Report
Size Limits
Largest Files (Top 5)
View All Files (283 total)
✅ Bundle size check passed |
Contributor
|
Deploy preview for adp-cost-calculator ready!
Deployed with vercel-action |
Contributor
|
Deploy preview for remote-flows ready!
Deployed with vercel-action |
Contributor
📊 Coverage Report✅ Coverage increased! 🎉
Detailed BreakdownLines Coverage
Statements Coverage
Functions Coverage
Branches Coverage
✅ Coverage check passed |
jordividaller
approved these changes
Oct 5, 2026
gabrielseco
deleted the
refactor/onboarding-benefits-engagement-build-once
branch
October 5, 2026 16:30
gabrielseco
added a commit
that referenced
this pull request
Oct 5, 2026
…mports (#1439) * fix(onboarding): submit computed money forced values in the right unit Portugal's extended work hours allowance is a forced money value computed from the salary. Onboarding built its render-time form without converting money fields to cents, so the allowance was computed from the salary in major units (100x too small), and the forced value was then written into the form state in cents while the form keeps money in major units. Validation recomputed the correct value and rejected the submission on a field with no input to show the error, blocking the step. - Onboarding's useJSONSchemaForm passes transformMoneyFields: true, since createHeadlessForm only defaults it on when no options are passed. - Forced money values are converted from cents before being set in the form. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(forms): default transformMoneyFields per key Default transformMoneyFields to true whenever it's not explicitly set, instead of only when the whole options object is missing. Callers that pass options without transformMoneyFields (e.g. only jsfModify) now get money fields converted to cents too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(invoice-schedules): drop redundant money flag Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(onboarding): cover money forced values on the legacy contract details path The flow regression test only covers the legacy path while PRT is outside JSF_V1_CONTRACT_DETAILS_COUNTRIES. These test the Onboarding useJSONSchemaForm hook and JSONSchemaFormFields directly, so they keep covering the fix whichever countries are on v1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(form): drop getForcedValue Submitting a forced value already uses the field's `const` in cents (parseFormValuesToAPI overwrites the form value with it), and the default forced value component doesn't render `value`. The `transformMoneyFields` change alone fixes the PRT allowance; getForcedValue would only change what custom forcedValue components receive in `fieldData.value`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(form): pin that forced values are submitted as their const Correct submission of forced money values relies on parseFormValuesToAPI overwriting the form value with the field's const. Covers a top-level field and one nested in a fieldset; both fail with 100x the amount without it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(onboarding): drop explicit transformMoneyFields createHeadlessForm now defaults transformMoneyFields per key (#1426), so the explicit override in useJSONSchemaForm is redundant. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(form): drop parseSubmitValues forced value test Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(form): add useHeadlessForm with rebuild and buildOnce strategies Every flow wires createHeadlessForm, parseJSFToValidate and handleValidation by hand, and each copy can drift on its own (the money unit regression lived in one of them). useHeadlessForm puts that wiring behind one interface with the two strategies the codebase uses today: - rebuild: rebuild the form whenever values change (current behaviour of most call sites). - buildOnce: build once per schema/options and resolve conditionals through handleValidation. It validates once after every build, so visibility is right without a mounted step and an inline jsfModify reference does not reset it; a replaced form's pending validation is cancelled before it mutates the old fields. The engine contract test renders the real useJSONSchemaForm + JSONSchemaFormFields (so ForcedValueField's setValue loop runs) and runs each schema situation across jsf v0, jsf v1, no meta and both strategies. New situations are one row. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(onboarding): build legacy contract details through useHeadlessForm First call site on the shared hook, with the rebuild strategy so behaviour is unchanged. useLegacyContractDetailsSchema fetches the raw schema and leaves building to the hook; the shared useJSONSchema stays as it is for the other steps. mergedFormValues moves out of that wrapper so both can use it. The existing PRT extended hours allowance flow test covers this step: it fails when the hook's rebuild stops converting money to cents. The jsonSchemaVersion test's mocked schema gains x-jsf-presentation, like every real schema has: the hook builds during render instead of inside a React Query select, so the money conversion's lookup of it now surfaces instead of turning into a silent query error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(form): stop buildOnce from rebuilding on every render with inline options buildOnce rebuilt the form whenever the options object changed identity, and every build validates and re-renders. A caller that creates options during render (as the Onboarding contract details hooks do) looped forever: 132 renders in 500ms in a probe, "Too many re-renders" in the test. Options are now compared by value with fast-deep-equal, the helper useJSONSchemaForm already uses. Functions and components inside jsfModify keep their identity across our own renders, so they compare equal by reference. React elements cannot reach jsfModify: the jsf engine deep-clones it and rejects them, so the React-aware variant is not needed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(onboarding): build jsf v1 contract details through useHeadlessForm The v1 contract details (FRA, ITA, DEU, ESP) already built the form once and resolved conditionals through handleValidation with invisible values kept, which is the hook's buildOnce strategy. useContractDetailsSchema now hands building, validation and submit parsing to the hook, so contract details covers both strategies: the legacy step with rebuild, v1 with buildOnce. The fieldsCount state that forced a re-render after each v1 validation goes away, since the hook re-renders itself. The comment on why invisible values are kept moves with that decision into the hook. The replay after a rebuild uses the step's form values, not the server employment data, so money is never converted from cents a second time on this path. France's flow tests and useOnboardingJsfV1ContractDetails fail when the hook's buildOnce validation is broken, so they cover this step. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(onboarding): refetch the contract details schema per employment Both contract details schema requests send employment_id, but their query keys only held the country and schema version, so React Query served the first employment's schema to every other employment of the same country in the session, and never refetched once an employment was created mid-flow. The keys now include the request query. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(form): cover options shapes in the jsf engine contract The contract only built forms without options, which is the one shape where createHeadlessForm's money default is irrelevant, so reverting that default to the old `options || { transformMoneyFields: true }` stayed green here and was only caught by flow and hook tests. Run every situation with no options, with an options object carrying no jsfModify (what Onboarding passes when the consumer gives none), and with a real jsfModify whose effect is asserted. Mutation checked: the old default fails the 6 rebuild money rows, and dropping options inside useHeadlessForm fails the 6 jsfModify rows. The Onboarding useJSONSchemaForm money forced value test is removed: its engine check is now in the contract and its wiring is covered end to end by the OnboardingFlow allowance-in-cents test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(form): add useHeadlessForm rollout plan Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore(lint): enforce useHeadlessForm over direct createHeadlessForm imports Adds a no-restricted-imports rule that bans importing createHeadlessForm from the kit or the common wrapper. Legacy call sites are allowlisted in an override; each migration PR removes its file from the list. Rewrites the CLAUDE.md forms guidance, the json-schema-form-usage Cursor rule and the BUGBOT.md forms section around the useHeadlessForm pattern, and moves the lint rule out of the rollout doc's phase 3. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * chore(lint): mark legacy createHeadlessForm imports inline instead of an allowlist The rule now always fails. Each legacy import carries an oxlint-disable-next-line comment with a TODO, so the remaining debt sits next to the code and is greppable. The overrides only keep permanent exceptions: the wrapper, the hook, tests and scripts. Lint now reports unused disable directives as errors, so migrating a file forces removing its comment. That surfaced three directives that no longer suppressed anything, which are removed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(bugbot): drop unclear useHeadlessForm flag line Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * remove no effect * chore(lint): fail example lint on unused disable directives Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(lint): catch relative createHeadlessForm imports Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * refactor(form): seed buildOnce forms from initialValues and move basic information onto useHeadlessForm useHeadlessForm's buildOnce strategy now takes initialValues and resolves conditional visibility at build time instead of replaying values in a post-mount effect. Rebuilds triggered by jsfModify reuse the last validated values for the same schema, while a new schema starts from initialValues. Onboarding's basic_information step now goes through useBasicInformationSchema (buildOnce) and contract details receives saved employment values as initialValues rather than the live fieldValues. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test(form): fold useHeadlessForm lifecycle cases into the jsf engine contract test The buildOnce rebuild/seed rules (initialValues on first render and when they arrive late, last validated values surviving a jsfModify change and beating initialValues, a new schema starting from initialValues) now run through the real form on jsf v0, jsf v1 and no meta, instead of a renderHook test on the jsf v1 fixture only. The render-count guard is dropped: the hook no longer validates after each build, so recreated options cannot cause a render loop anymore. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(form): mark basic information as migrated to buildOnce in the rollout Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test(onboarding): drop the contract details employment_id query key test Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(form): pass saved initialValues to buildOnce in the useHeadlessForm guidance buildOnce now takes initialValues instead of values (#1433), so the CLAUDE.md, Cursor rule and Bugbot examples would have produced a type error. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * chore(lint): exempt __tests__ and colocated test files from the createHeadlessForm ban Tests also live in __tests__ folders and next to source as *.test.ts(x), not only under tests/, so the override now covers them too, matching the guidance that tests may use the engine directly. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * refactor(onboarding): build benefits and engagement agreement details once through useHeadlessForm (#1442) * refactor(onboarding): build benefits and engagement agreement details once through useHeadlessForm Both schema hooks now fetch the raw schema and hand it to useHeadlessForm with the buildOnce strategy, seeded with the saved values (partner initialValues plus the saved benefit offers / engagement agreement details). Validation, submit parsing and value changes for both steps go through the hook instead of the hand-written parseJSFToValidate branches in hooks.tsx, and the forms are no longer rebuilt on every keystroke from the live field values. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(form): mark benefits and engagement agreement details as migrated to buildOnce Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * add tests to verify values --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Andrea Maria Piana <andrea.maria.piana@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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
2 participants
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.
Summary
The benefits and engagement agreement steps of the employee onboarding flow now build their forms once and keep them up to date as the user types, instead of rebuilding the whole form on every keystroke.
Why
What changed
Toggle details
useBenefitOffersSchemaanduseEngagementAgreementDetailsSchema(src/flows/Onboarding/api.ts, internal): the query now selects the raw schema, anduseHeadlessForm({ strategy: 'buildOnce' })builds the form. Both return{ data, isLoading, handleValidation, onValuesChange, parseFormValues }instead of the whole React Query result;hooks.tsxonly ever readdataandisLoading.initialValues+ the saved benefit offers.initialValues+ the saved engagement agreement details.initialValuesmemo for engagement agreement details reuses the same saved-values object.hooks.tsx: the hand-writtenparseJSFToValidatebranches for both steps inparseFormValuesandhandleValidationare replaced by the hook's functions, andcheckFieldUpdatescalls each step'sonValuesChange.handleValidationhas resolved visibility, instead of dropping them first. Submit parsing still drops them.initialValuesstill merge live field values while on the step. That memo only feeds the step's form defaults, and changing it is outside this refactor.no-restricted-importsTODO onOnboarding/api.tsstays:useJSONSchemaForm(used byJsonSchemaComparison) anduseCountriesSchemaFieldstill callcreateHeadlessForm.No public API change; both hooks are internal.
Screenshots
N/A
Related Resources
docs/USE_HEADLESS_FORM_ROLLOUT.mdTesting
I broke the new wiring on purpose to see what fails today:
Benefits validation or submit parsing broken:
OnboardingFlow.test.tsxfails.Engagement agreement details starting empty, or
onValuesChangenot called:OnboardingFlowGermanyDynamicSteps.test.tsxfails.Engagement agreement details validation broken:
OnboardingFlowGermanyDynamicSteps.test.tsxfails (salary range checked against the computed minimums).Engagement agreement details submit parsing broken, or saved salaries converted to cents twice:
OnboardingFlowGermanyDynamicSteps.test.tsxfails (checks the exact request body).onboard-germany-employee.spec.ts(e2e) also submits that step to the real API.Benefits starting values on resume with a non-default filter: not covered, as before this change.
npx vitest run src/flows/Onboarding(251 passed)Tested against the
example/app in a browserFeature flag: N/A
🤖 Generated with Claude Code