finding(react): useSchemaPersistence writes adapterRef.current during render — three react-hooks/refs warnings on a published hook

[claude]

Filed unassigned by the domain:ui execution seat as an out-of-scope finding while implementing objectui#6658 (the callable guard on the same file). Not fixed there — that PR is scoped to the save-door guard and touches only the save path plus a new pin file.

What ESLint reports today

packages/react/src/hooks/useSchemaPersistence.ts carries three react-hooks/refs warnings, all on lines this seat did not touch (they pre-date objectui#6658's branch — the diff there is three pure insertions, and the warned lines are byte-identical to origin/main).

Measured on faac0d935, pnpm exec eslint packages/react --format json: 131 files linted, 0 errors, 380 warnings package-wide, of which this file holds 3:

useSchemaPersistence.ts:211 react-hooks/refs Cannot access ref value during render
useSchemaPersistence.ts:213 react-hooks/refs Cannot update ref during render
useSchemaPersistence.ts:213 react-hooks/refs Cannot access ref value during render

The shape:

const defaultAdapter = useRef(createLocalStorageAdapter());
const adapterRef = useRef(adapter ?? defaultAdapter.current);
// Keep the ref up to date if the adapter prop changes
adapterRef.current = adapter ?? defaultAdapter.current;

Both reading defaultAdapter.current and assigning adapterRef.current happen in the render body.

Why it may matter

React's own rule flags this because a ref written during render is not safe under concurrent rendering: a render that React discards or replays still performed the write. Here the written value is the persistence adapter, so the observable risk is a save routed through an adapter from a render that never committed.

This is a warning, not an error, and pnpm lint is green (0 errors package-wide), so nothing is red today. No user-visible break has been measured — filing as an observation for triage rather than asserting impact.

Reproduce

pnpm exec eslint packages/react/src/hooks/useSchemaPersistence.ts --format json

Dedupe search run before filing: the repo-scoped /search/issues REST endpoint answers 403 for this seat, so this went through the REST list endpoint plus a local grep — 239 open issues collected, zero hits for useSchemaPersistence, react-hooks/refs or "ref during render"; control words hit in the same read (36 titles match "schema", and objectui#6658 itself is present), so the empty result is a real reading rather than a broken one.

---

Generated by Claude Code

[claude]

Claim + dispatch order — domain:ui execution seat, PM session session_01CRJge11jso9TpXRWFt1Z49, round R15.
Branch: claude/issue-6745-ref-during-render. One os-dev agent, one worktree.

Terrain I verified myself

packages/react/src/hooks/useSchemaPersistence.ts exists on origin/main at d06059f24, alongside hooks/index.ts and hooks/__tests__/useSchemaPersistence.callableGuard.test.tsx (the pin #6658 left behind). The three warned line numbers and the ESLint counts in the card body were measured on faac0d935 by the filing seat — that is what the card says, on an older ref. Re-measure on your own base and report the numbers you actually get; do not quote the card's.

What to do

Stop writing and reading the ref during the render body, so the three react-hooks/refs warnings go away for the right reason.

⚠️ The obvious fix is a behaviour change, and that is the whole difficulty of this card. Moving the assignment into an effect defers it until after commit. If anything reads adapterRef.current during the same render or commit that changed the adapter prop, an effect-based fix routes that read to the previous adapter. Measure whether such a reader exists before choosing a shape. If the only safe fix changes observable behaviour, stop and report that — this card is a lint-warning finding with no measured user-visible break, and it is not authority to change what the hook does.

Also present and worth reporting, but do not widen the PR for it without saying so: useRef(createLocalStorageAdapter()) invokes the factory on every render and discards the result after the first.

⛔ Do not silence the rule. An eslint-disable, a rule downgrade, or a config exclusion is not a fix here — the warning is the finding.

Acceptance

1. The three react-hooks/refs warnings on this file are gone, measured with the same command the card names, on your base, with the before and after counts both reported.
2. A pin proving the adapter still tracks a changed adapter prop — including the timing the old code gave, if you preserved it, or an explicit statement of what changed if you did not.
3. The existing callable-guard pin from finding(react): the designer save door JSON.stringifys the schema with no callable guard — a function-valued handler key is silently dropped, and reload yields a schema that lost it without an error #6658 still passes untouched.

Reporting

Ablation both directions as usual. In particular, report whether any test at all fails when you revert your change — if the warnings are the only thing that moves, say so plainly: that makes this a lint-cleanliness fix with no behavioural pin behind it, which is a legitimate outcome but must be labelled as one rather than dressed up as a bug fix.

---

Generated by Claude Code

[claude]

os-dev-report

{
 "issue": 6745,
 "status": "done",
 "branch": "claude/issue-6745-ref-during-render",
 "pr": "https://github.com/objectstack-ai/objectui/pull/6796",
 "premise_still_valid": true,
 "summary": "Premise held on my base d06059f24, though the card's line numbers did not: the three react-hooks/refs warnings sit at 237:40, 239:3 and 239:35, not the card's 211/213/213 (measured on the older faac0d935; the file grew by #6658's insertions). The render-body write is gone: the adapter ref is now written in useInsertionEffect, and the default adapter comes from useMemo instead of a useRef whose .current was read during render. useInsertionEffect rather than useEffect/useLayoutEffect is the whole point — it runs in the mutation phase, ahead of every layout effect in the tree, ahead of 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. useEffectEvent would be idiomatic but is React 19.2+ and this package declares react ^18.0.0 || ^19.0.0 as a peer; useInsertionEffect is React 18.0+. Reader measurement, which drove the choice: all six reads of adapterRef.current are inside async useCallback bodies (lines 257/275/290/291/306/307), the ref is never returned, and the only in-repo references to the hook are the barrel export and its two pin files — so no in-repo reader can observe the deferral, but the hook is a published export of @object-ui/react 17.6.0, which is why I closed the window to something no legal consumer can reach rather than accepting an effect-based deferral. The one window that did change is a read during the render phase itself, which no legal consumer has. The factory-per-render defect you flagged is fixed and named explicitly in the PR body rather than slipped in: it is not a widening, because the 237:40 warning IS the render-phase read of defaultAdapter.current, so clearing it requires changing how the default adapter is held (the lazy-init ref idiom would read and write a ref during render just the same).",
 "tests": "All heavy commands through scripts/pm/os-verify-lock.sh; exit codes captured before any pipe. UNION RE-RUN AFTER THE FINAL COMMIT at a7be07cc5, tree clean (0 modified entries). [1] LINT, the command the card names, on my base d06059f24: `pnpm exec eslint packages/react/src/hooks/useSchemaPersistence.ts --format json` BEFORE = 3 warnings (237:40 'Cannot access ref value during render', 239:3 'Cannot update ref during render', 239:35 'Cannot access ref value during render'), AFTER = 0 warnings. Package-wide `pnpm exec eslint packages/react --format json` BEFORE = 126 files / 0 errors / 346 warnings / 6 react-hooks/refs; AFTER = 127 files / 0 errors / 343 warnings / 3 react-hooks/refs (the extra file is the new pin; the 3 survivors are on other files and are filed as objectui#6797). [2] TESTS, canonical invocation from the repo root with no `--`: `pnpm exec vitest run packages/react/src/hooks/__tests__/useSchemaPersistence.adapterTracking.test.tsx packages/react/src/hooks/__tests__/useSchemaPersistence.callableGuard.test.tsx` -> 'Test Files 2 passed (2)', 'Tests 15 passed (15)' — 4 new pins plus the 11 from #6658, untouched. [3] TYPE-CHECK: `pnpm --filter '@object-ui/react' run type-check` (hyphenated in this repo) echoed `tsc --noEmit && tsc -p tsconfig.test.json` and passed; NOT-MEASURED check done — `tsc -p tsconfig.test.json --listFiles | grep -c adapterTracking.test.tsx` = 1, so the green really covers the new pin file. [4] GATES derived by hand from this repo's own package.json and .github/workflows (objectui has no scripts/pm/dispatch-gates.mjs — that script lives in objectstack and only ever answers about its own tree): check:control-bytes -> 'OK (scanned 5646 tracked text file(s); skipped 85 binary)'; check:vi-mock-specifiers -> 'OK'. Also a manual control-byte scan of all three touched files: no hits. Repo-wide `pnpm lint` left to CI, which runs the farm exactly once. ABLATION, three legs, implementation COMMITTED FIRST (a7be07cc5). No rebuild leg applies and I verified why rather than assuming: both pin files import the hook by relative source path ('../useSchemaPersistence') and packages/react/dist does not exist, so the mutation bites the file Vitest actually loads. Every leg confirmed ON DISK by comparing `git hash-object` against the HEAD blob 7ec02606bb36043ff40d605be3e380c8897d3817 (empty hash treated as failure), plus grep counts of the injected and removed text; each restored with `git checkout HEAD -- $ABSOLUTE_PATH` under a trap, and the restore proven by the hash returning to the HEAD blob and `git diff HEAD` being empty. LEG A (revert the implementation to origin/main, keep the new pins): mutated hash 71e9bc36, useInsertionEffect count 0 / render-body-write count 1 -> 'Tests 15 passed (15)', ZERO failures; lint returns to 3 warnings at 237/239/239. LEG B (useInsertionEffect -> useEffect): mutated hash 2a6a9ca1 -> 1 failed / 14 passed, pin 2 fails with exactly the stale read: expected 'second:from-layout-second', received 'first:from-layout-second'. LEG C (useInsertionEffect -> useLayoutEffect): mutated hash 276c03b3 -> 1 failed / 14 passed, same pin, confirming empirically that a child's layout effects run before its parent's. Tree verified clean after all three.",
 "mcp_calls": "0 — every GitHub read and write went through repo-scoped REST via curl (issue read, comments, open-issue listing for dedupe, PR create, PR body read-back, issue create, this comment); no MCP GitHub tool was called",
 "open_questions": [],
 "out_of_scope_findings": [
 "filed as #6797: the other three react-hooks/refs warnings in packages/react — useETagCache.ts:204, useGlobalUndo.ts:57, useOffline.ts:262, one 'Cannot access refs during render' apiece; labelled `finding`, unassigned, deliberately NOT folded into this PR because each needs its own reader/timing analysis and useInsertionEffect may not be the right shape for them",
 "not filed, fixed in this PR and named in its body: useRef(createLocalStorageAdapter()) invoked the factory on every render and discarded all but the first result — now useMemo. Forced by acceptance 1 (the 237:40 warning is the render-phase read of defaultAdapter.current), same defect class, same file, same gate family, so it is the bounded in-place fix rather than a widening"
 ]
}

---

Generated by Claude Code