Uh oh!
There was an error while loading. Please reload this page.
fix(react): stop writing the useSchemaPersistence adapter ref during render - #6796
Merged
Conversation
…render The hook wrote `adapterRef.current` in the render body and read `defaultAdapter.current` there too, drawing three react-hooks/refs warnings (objectui#6745). A render React discards or replays still performed the write, so a save could be routed through an adapter from a render that never committed. The write moves to `useInsertionEffect` — the mutation phase, ahead of every layout effect in the tree, paint, and any event handler — so every call site that may legally invoke save/load/list/remove sees exactly what the old render-body write gave it. `useEffect` (after paint) and `useLayoutEffect` (a child's run before its parent's) would each have deferred the swap past a legal reader; a new pin holds that distinction. The default adapter moves from `useRef(createLocalStorageAdapter())` to `useMemo`, which also stops the factory running on every render. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This was referenced Aug 29, 2026
os-sales
marked this pull request as ready for review
August 29, 2026 22:15
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
Fixes#6745
useSchemaPersistencekept the live adapter in a ref that was written in therender body, drawing three
react-hooks/refswarnings.Measured on my own base, not the card's
The card's line numbers and counts were taken on
faac0d935. Re-measured on mybase
d06059f24— the file has grown since, so the lines differ:Package-wide,
pnpm exec eslint packages/react --format json:react-hooks/refsThe 3 that remain are on other files and out of scope here. The extra file is
the new pin.
The shape, and why it is not an effect
The four callbacks are created once (
[]deps) and read the adapter at calltime, so the newest adapter has to reach them without changing their identity —
that is the ref's whole job. The write now happens in
useInsertionEffect:useEffectlands after paint, so a call made earlier in the same commitreaches the previous adapter.
useLayoutEffectis not enough either: a child's layout effects runbefore its parent's, so a child calling
save()from its own layout effectstill sees the previous adapter.
useInsertionEffectruns in the mutation phase — ahead of every layouteffect in the tree, ahead of paint, ahead of any event handler.
useEffectEventwould be the idiomatic answer but is React 19.2+, and thispackage declares
react: ^18.0.0 || ^19.0.0as a peer.useInsertionEffecthas been available since React 18.0.
What changed observably: nothing any legal call site can reach. The single
deferred window is a read during the render phase itself, and
save/load/list/removeare side effects that are never callable duringrender. What the old code additionally did — and this is the defect the rule
names — was perform the write on renders React discards or replays (StrictMode,
a Suspense retry, a concurrent interruption), so a save could be routed through
an adapter belonging to a render that never committed.
Also fixed, named rather than slipped in
useRef(createLocalStorageAdapter())invoked the factory on every render anddiscarded all but the first result. It is now
useMemo. This is not a widening:the
237:40warning is a read ofdefaultAdapter.currentduring render, soclearing it requires changing how the default adapter is held, and the lazy-init
ref idiom would read and write a ref during render just the same. The adapter is
a stateless facade over
localStoragewhose identity is never exposed, so thechange is unobservable beyond the saved work.
Pins
packages/react/src/hooks/__tests__/useSchemaPersistence.adapterTracking.test.tsx,4 pins: a changed
adapterprop is what the next save reaches; the swap is inplace before a child layout effect of the same commit (the discriminating
one); the default localStorage adapter still persists across re-renders; an
explicit adapter takes over from the default.
Ablation, both directions
Implementation committed first; every leg confirmed on disk by comparing
git hash-objectagainst the HEAD blob, restored withgit checkout HEAD --under a trap. The pins import the hook by relative source path and this package
has no
dist, so the mutation bites the file Vitest actually loads — no rebuildleg applies.
origin/main, new pins keptuseInsertionEffecttouseEffectuseInsertionEffecttouseLayoutEffectLeg B's failure is the stale read itself:
Leg A is the honest headline: no test fails when the fix is reverted. The
pins pass against the old code and the new code alike — precisely because the
timing was preserved. This is a lint-cleanliness fix with no behavioural
regression behind it, not a bug fix; the new pins guard the next edit (legs B
and C) rather than a break that exists today. No user-visible break was measured
here, and none is claimed.
Verification
Union re-run after the final commit, at
a7be07cc5, tree clean:pnpm exec vitest runon both pin files (canonical invocation, repo root, no--):Test Files 2 passed (2),Tests 15 passed (15)— including the 11from the objectui#6658 callable guard, untouched.
pnpm --filter '@object-ui/react' run type-check: pass. It runstsc --noEmit && tsc -p tsconfig.test.json;--listFilesconfirms the newpin file is in that program, so the green covers it.
pnpm exec eslint packages/react: 0 errors, 343 warnings (table above).check:control-bytes:OK (scanned 5646 tracked text file(s)).check:vi-mock-specifiers:OK.Repo-wide
pnpm lintis left to CI, which runs the farm exactly once.Generated by Claude Code