Skip to content

Say which formRef shapes attach once, and pin the in-flight reading - #2542

Merged
woksin merged 1 commit into
mainfrom
fix/2497-callback-ref-doc-accuracy
Aug 13, 2026
Merged

Say which formRef shapes attach once, and pin the in-flight reading#2542
woksin merged 1 commit into
mainfrom
fix/2497-callback-ref-doc-accuracy

Conversation

@woksin

@woksin woksin commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Added

Fixed

The handle object is created once, but the docs concluded from that a
callback ref attaches exactly once. React re-attaches whenever the ref
itself changes identity, so an inline arrow re-attaches every render -
and paired with a state setter that is a re-render loop, which is the
very failure the note claimed to rule out. Say stable ref instead.

The rejection spec asserted only that the form stopped executing, which
an implementation that never started executing also satisfies. It now
reads while the command is in flight: dropping the state publish leaves
the old assertion green and turns the new one red.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@woksin woksin added the minor label Aug 13, 2026
@woksin

woksin commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Reviewer context, kept out of the body.

Why the body lists the #2497 surface even though this PR is small. #2497 merged as #2538 (fd5c753c6) but never published. Both Arc PRs merged four seconds apart, so both Publish runs computed the same version, 21.13.0; #2539's won and #2497's failed with npm error You cannot publish over the previously published versions: 21.13.0. Verified against the registry rather than the run metadata — I unpacked @cratis/arc.react@21.13.0 and useIsCommandExecuting has zero hits across 446 files, with CommandForm (439 files) and isValid (231) as controls proving the search works. So the code is on main and unreleased. This PR is the release that first ships it, and the release notes are generated from this description verbatim, so they describe what actually reaches users in it.

The two changes here are both independent-review findings against #2538.

  1. Documentation accuracy. The formRef JSDoc and form-lifecycle.md reasoned from "the handle object is created once" to "a callback ref is invoked exactly once on mount and once on unmount, however often the parent re-renders". React appends ref to useImperativeHandle's dependency array, so re-attachment tracks the identity of the ref, not the handle. An inline formRef={h => setHandle(h)} is a new function every render and re-attaches every render — and paired with a state setter that is precisely the re-render loop the note claimed to rule out. Now says: stable ref (object ref, or useCallback with no deps) attaches once; inline works but does redundant attach/detach. The existing regression spec proves the once-only claim only for a useCallback(…, []) ref, and its own comment already admitted "the ref callback itself is stable here" — so the spec was never evidence for the broader claim.

  2. A vacuous assertion. and_the_command_rejects asserted only isExecuting === false after the rejection, which an implementation that never set it true also satisfies. It now takes a reading while the command is in flight. Demonstrated rather than asserted: with the state publish after the increment removed, the new assertion fails (expected false to be true) while the old one still passes — so the old spec genuinely could not catch it. Restore verified by shasum -a 256 (c4bc5f39…291288d).

Worth recording a process note: the first attempt at that mutation left the tree dirty. The trap … EXIT used a path relative to a directory a later cd had moved out of, so the restore silently failed and the mutation stayed in the file. It was caught by grepping for the marker rather than trusting the trap's own output, which had printed "RESTORED". Traps in mutation harnesses need absolute paths, and the restore needs verifying independently of whatever the trap reports.

Gate. yarn install; npx tsc -b --force clean; vitest 518 passed in Arc.React (517 before, +1 new assertion). No production behaviour changes in this PR — one JSDoc block, one docs section, one added spec assertion.

@woksin
woksin merged commit 6ad18c1 into main Aug 13, 2026
13 of 14 checks passed
@woksin
woksin deleted the fix/2497-callback-ref-doc-accuracy branch August 13, 2026 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants