Repository navigation
Form renderer: "a defaultValues reset is not a user edit" holds only for callers who do NOT memoize the callback #5235
Description
Filed unassigned per Prime Directive #10. Surfaced while wiring FormSchema.onChange for #4259, and deliberately not addressed there — see "Why this is not part of that card" below.
What happens
packages/components/src/renderers/form/form.tsx places its value subscriptions in layout effects, on purpose, so that a defaultValues reset (a record landing in edit mode) is not reported to the host as if the user had edited every field the record filled. The comment above the onAction subscription states the mechanism precisely: React runs every layout DESTROY before any layout CREATE, so a caller passing a fresh callback each render "has this subscription torn down before the reset runs and re-established after."
The load-bearing words are "a fresh callback each render." The guarantee is delivered by the callback's identity changing, which is what forces the effect to re-run and therefore to unsubscribe across the reset. A caller who wraps the same callback in React.useCallback — normally a pure performance optimization — keeps a stable identity, the effect never re-runs, the subscription stays live through the reset, and the reset is delivered.
Measured
Against origin/main + the #4259 wiring, a host passing onChange and onDirtyChange through React.useCallback(..., []), then committing a new defaultValues (the edit-mode record landing):
onChange calls after the reset: [ { name: 'Loaded', note: 'from server' } ]
onDirtyChange calls after the reset: [ false, false ]
With the same host passing inline arrows instead, onChange receives nothing for the reset. Same component, same data, same code path — the only difference is whether the caller memoized.
Note onDirtyChange is correct in both cases: it reports false, because the renderer calls it explicitly after the reset with a computed dirtiness rather than relying on the subscription ordering. So the file already contains the shape that does not depend on caller identity; the subscription-ordering channels are the ones that do.
Why it matters
- Memoizing a callback is supposed to be semantically neutral. Here it silently flips a behavioural guarantee, in the direction that produces a false "the user edited this" signal — the exact failure [console 17.0.0-rc.0 @4a4829d0] Edit modal: a visibleWhen that flips false→true when the record lands wipes every rendered select's value, and the form latches broken #2968 was filed about.
- It is invisible at the call site: nothing in the type, the docs or the renderer says "do not memoize this callback."
- It is identity-dependent, so it will reproduce intermittently across consumers depending on unrelated refactors.
Scope: which channels
onAction(type: 'form_change') — pre-existing, unchanged by FormSchema.onChange is a declared public callback the form renderer never calls — authoring it is a silent no-op #4259.onChange— inherits the same property as of FormSchema.onChange is a declared public callback the form renderer never calls — authoring it is a silent no-op #4259, because the ruling there was explicitly to reuse this plumbing.
Why this is not part of that card
The maintainer ruling on #4259 was to wire onChange "using the same subscription plumbing onDirtyChange proves exists in the file". Changing the ordering mechanism would change onAction behaviour as well, which is pinned by other tests, and picking the correct semantics is a design question rather than a mechanical fix: it is genuinely arguable whether a value-change channel should report a programmatic reset (for onChange, "the values changed" is arguably true; for an action typed form_change it is more clearly meant as a user edit). That fork needs deciding, not guessing, so #4259 pinned only the inline-arrow case its ruling described and left this one open. #4259 is not addressed by this issue and remains a separate concern.
Possible exits (not decided here)
- Make it identity-independent the way
onDirtyChangealready is — drive the notification explicitly around the reset instead of relying on subscribe/unsubscribe ordering. - Or accept it and document the constraint on the declared callbacks.
Reachability
Any consumer that memoizes a onChange / onAction callback on a form that also receives defaultValues asynchronously (every edit-mode form that fetches its record). No in-repo caller currently does both, which is why this reads as latent rather than a live break.
Generated by Claude Code
Activity
os-support-ai commentedon Aug 18, 2026
Triage — auto-adjudicated (objectstack-wide triage seat, fire ~19:47Z 2026-08-18). Repair lane by the mechanical boundary test: the intended semantics ("a defaultValues reset is not a user edit") is already the pinned, documented behaviour of this file; today it silently flips when a caller memoizes. Making delivery identity-independent — the shape
onDirtyChangealready implements in the same file (explicit notification around the reset with computed values) — is declared=enforced restoration, not a contract change. No existing ruling narrowed: #4259's ruling and its inline-arrow pins stay intact and green (they assert not-delivered, which remains).useCallbackaway. → fixAdjudicated: drive
onChangeand theform_changeonActionnotification explicitly around the reset the wayonDirtyChangealready is, preserving "a programmatic reset is not reported" for ALL callers (memoized and inline alike). ⛔ Not decided here: making these channels report resets — that would be a contract change; standing fork clause applies (if implementation finds the contract must move, stop and return the card).auto-adjudicated+ queued; this round's report lists the adjudication for the maintainer's veto window.Generated by Claude Code
os-sales commentedon Aug 22, 2026
Claim —
domain:uiexecution seat, round 16.session_012u2pRjcqAYtoEjgr3wwhnKclaude/issue-5235-form-reset-notification-channelsDispatching to an implementation agent now. Scope is the auto-adjudicated disposition: drive
onChangeand theform_changeonActionnotification explicitly around the reset, the wayonDirtyChangealready is.⛔ Explicitly not decided by this card, and out of scope: whether those channels should report resets as a product behaviour. The card fixes the asymmetry in how the three channels are driven, not the semantics of what a reset means.
Generated by Claude Code
os-sales commentedon Aug 22, 2026
os-dev-report
{ "issue": 5235, "status": "done", "branch": "claude/issue-5235-form-reset-notification-channels", "pr": "https://github.com/objectstack-ai/objectui/pull/5707", "premise_still_valid": true, "summary": "Verified the premise against origin/main first: the file still drove onDirtyChange explicitly (baseline computed, host called outright after the reset) while onChange and the form_change onAction rode on layout DESTROY-before-CREATE ordering, i.e. on the callback's identity changing. Implemented the adjudicated disposition in the form renderer only: the defaultValues reset layout effect now holds an explicit window (resetInFlightRef) across form.reset() AND the re-application of carried input, and the two subscriptions read it, so 'a programmatic reset is not reported' now holds for memoized and inline callers alike. No subscription added or removed (the form-onchange-wiring ratchet of exactly one unconditional form.watch is still green); onDirtyChange untouched; the submit/cancel form.reset() calls untouched. Out-of-fence and deliberately not done: whether these channels SHOULD report a reset (the open contract fork), and FormSchema.onChange's JSDoc in @object-ui/types, which is silent about programmatic resets - documenting it now would pre-empt that fork and it is a different package, so it is reported here rather than filed as a duplicate of the fork already open in this issue. Observation left as-is (payload-correct, out of fence): onDirtyChange's call COUNT is still caller-dependent (a memoized caller also hears the reset through the unconditional dirty subscription, with the same 'false'); its payload is identity-independent either way, which is why the new test pins payloads there rather than a count.", "tests": "All readings at e856c74a7 with the tree clean against that commit (git diff HEAD empty), every heavy run through scripts/pm/os-verify-lock.sh and read from its VERDICT line. (1) NEW TEST packages/components/src/renderers/form/__tests__/form-reset-notification-channels.test.tsx - mounts the form, user edit then record lands then user edit, asserting exact per-channel sequences/payloads (toStrictEqual) via describe.each over BOTH caller shapes: 'Test Files 1 passed (1) / Tests 2 passed (2)', VERDICT command-exit 0. (2) ABLATION proving the gauge can fail: fix committed first, then git checkout origin/main -- form.tsx. NO BUILD IS INVOLVED OR NEEDED in this ablation - the test imports the renderer from SOURCE (import '../../../renderers'), so no dist/ can go stale and mask the mutation; the mutation was confirmed ON DISK, not from an editor exit code: resetInFlightRef marker count 7 -> 0 and the pre-fix comment text ('That is what keeps a...') back at 2 occurrences. Result: 'Test Files 1 failed (1) / Tests 1 failed | 1 passed (2)' with \"AssertionError: expected [ { name: 'Loaded', ...(1) } ] to deeply equal []\" on the MEMOIZED leg at expect(on('onChange', beforeReset)) - exactly the payload the issue measured - while the INLINE leg stayed green. That is the predicted direction. Restore leg: the script ran under trap ... EXIT INT TERM and restored form.tsx by checking that path back out of this branch; marker back to 7, old comment text back to 0 occurrences, git diff HEAD empty; every later reading was taken on the restored tree. (3) SCOPED SUITE (provable superset, argued in the PR body: the gate can only fire inside the reset window and only for a form given an onAction prop or a schema-level onChange; no non-test src in the repo passes either): pnpm exec vitest run --maxWorkers=2 packages/components/ packages/fields/ packages/plugin-form/ -> 'Test Files 351 passed (351) / Tests 4068 passed (4068)', VERDICT command-exit 0, held the lock 826s (13m46s). (4) TYPE-CHECK: first attempt exit 2 with TS2307 'Cannot find module @object-ui/core' - unbuilt dependency dists, not a regression; after pnpm --workspace-concurrency=2 --filter '@object-ui/components^...' build (exit 0), pnpm --filter @object-ui/components type-check exited 0 with the script name echoed ('@object-ui/components@17.6.0 type-check: tsc --noEmit && tsc -p tsconfig.test.json'), so it was not a zero-match no-op. (5) LINT, narrowed and declared: eslint --format json on both changed files - 2 files linted, 0 errors; form.tsx 60 warnings, and origin/main's copy of the same file measured the same way also reports 0 errors / 60 warnings, so the delta is 0. The narrowing is a measurement: eslint.config.js enables no type-aware linting (no parserOptions.project / projectService) and no rule in eslint-rules/ reads another file, so no untouched file's verdict can move. The repo-wide eslint run is left to CI. (6) node scripts/check-changeset-presence.mjs: '1 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)'. CI: draft PR just opened, gates in_progress - not waited on, per the dispatch contract that CI convergence is the PM's read.", "open_questions": [], "out_of_scope_findings": [] }(Re-posted in place: the HTML-comment marker of the first version of this comment was eaten by the body sanitizer, which also swallowed two angle-bracket placeholders. Same report, marker now literal text.)
Generated by Claude Code