Skip to content

Commit bcbd74c

Browse files
serpentbladeclaude
andcommitted
fix(target-react): stabilize member-mutated top-level <script> consts (useMemo)
A top-level `const X = new Set()` (or `[]`/`{}`) that is member-mutated as cross-render scratch state but referenced only from a plain function — never escaping into a useEffect dep — was emitted INLINE, so React re-created it every render and silently reset the state. On the 5 setup-once targets the `<script>` runs once, so the instance persists; React's re-running component body broke the setup-once contract. This is the root cause of the `code-mirror [react]` VR Matrix hang (red on main since the CodeMirror G5 wave-2 batch b429107..df5f6aa): CodeMirrorDemo's `const gutterSeen = new Set()` dedupe guard reset every render → the `:data-cap="markGutterMount(line)"` render side-effect always re-incremented `$data` → re-render → reconfigure → portal flushSync → infinite loop → 30s timeout → the React demo never committed → the counted A/B editors never appeared. Fix: extend the React emitter with the const-analog of the reassigned-`let`→ `useRef` hoist. `collectMutatedInstanceBinders` finds top-level const/let binders whose init is a fresh mutable instance (`new`/array/object literal) AND which are member-mutated (`.add`/`.set`/`.push`/`.delete`/`X[k]=…`/`X.f++`/ `delete X[k]`); `tryWrapMutatedInstanceUseMemo` wraps each in `useMemo(() => init, [])` so it is built once per instance. useMemo keeps the binder name, so reads/mutations need no `.current` rewrite. Runs before the escaping-const useMemo wrap so a binder that is both escaping and mutated gets the stable `[]` identity (mutable state must never be re-created). Conservative: a pure derived `const total = a + b` is never a fresh-instance init and never member-mutated, so it is never frozen; reassigned `let`s are excluded (that is the hoistModuleLet→useRef case). Member-mutated consts that already escaped into a mount effect (e.g. MapLibre's marker/popup/feature Maps) were already `useMemo(…, [])` via the escaping path — byte-identical output, no drift. Gates: minimal repro stabilizes seenConst while leaving pureDerived inline; cold `--force` build 86/86 + typecheck 105/105 + test 64/64 (zero dist-parity / snapshot drift — only React emit changes, only for the genuinely-non-escaping case); Linux-Docker VR `code-mirror [react]` 3/3 green; full matrix 403 passed, zero regressions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 6d4b480 commit bcbd74c

1 file changed

Lines changed: 186 additions & 0 deletions

File tree

packages/targets/react/src/emit/emitScript.ts

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,169 @@ function tryWrapEscapingConstUseMemo(
478478
return `const ${constName} = useMemo(${arrowBody(init)}, ${depsLiteral});`;
479479
}
480480

481+
/**
482+
* Member-mutating instance methods. A call `X.<m>(...)` where `<m>` is one of
483+
* these MUTATES the instance `X` (rather than reading from it — `.has`/`.get`/
484+
* `.map`/`.size` are reads and deliberately excluded). Used by
485+
* `collectMutatedInstanceBinders` to distinguish a cross-render scratch binder
486+
* (must persist) from a read-only one.
487+
*/
488+
const MUTATING_INSTANCE_METHODS = new Set<string>([
489+
// Set / Map
490+
'add', 'set', 'delete', 'clear',
491+
// Array
492+
'push', 'pop', 'shift', 'unshift', 'splice', 'sort', 'reverse', 'fill', 'copyWithin',
493+
]);
494+
495+
/** A `new X()`, `[...]`, or `{...}` initializer — a freshly-built mutable instance. */
496+
function isFreshMutableInstanceInit(init: t.Expression): boolean {
497+
return t.isNewExpression(init) || t.isArrayExpression(init) || t.isObjectExpression(init);
498+
}
499+
500+
/** Root identifier name of a (possibly nested) MemberExpression: `X.a.b`/`X[k]` → `X`. */
501+
function memberRootName(node: t.Node): string | null {
502+
let cur: t.Node = node;
503+
while (t.isMemberExpression(cur) || t.isOptionalMemberExpression(cur)) {
504+
cur = cur.object;
505+
}
506+
return t.isIdentifier(cur) ? cur.name : null;
507+
}
508+
509+
/**
510+
* Phase 35 follow-up (feedback_react_const_mutinstance_not_stabilized) —
511+
* collect the names of top-level `const`/`let` binders whose initializer is a
512+
* freshly-constructed MUTABLE INSTANCE (`new X()`, `[...]`, `{...}`) AND which
513+
* are MEMBER-MUTATED somewhere in the component (`X.add(...)`, `X.set(...)`,
514+
* `X.push(...)`, `X.delete(...)`, `X[k] = …`, `X.foo++`, `delete X[k]`).
515+
*
516+
* Such a binder is cross-render scratch state in the author's setup-once mental
517+
* model: the 5 setup-once targets run `<script>` ONCE so the instance persists,
518+
* but React re-runs the component body every render, re-creating the instance
519+
* and silently resetting the state. CodeMirrorDemo's `const gutterSeen = new
520+
* Set()` dedupe guard is the canonical case — re-created every render →
521+
* `markGutterMount` always re-increments `$data` (the guard never deduplicates)
522+
* → render side-effect → re-render → infinite loop → 30s VR hang. Wrapping the
523+
* binder in `useMemo(() => init, [])` restores per-instance setup-once identity
524+
* (the const-analog of the reassigned-`let`→`useRef` hoist in `hoistModuleLet`,
525+
* but useMemo keeps the binder name so reads/mutations need no `.current`
526+
* rewrite).
527+
*
528+
* Conservatism, two gates:
529+
* - init-shape: only `new`/array/object literals — a pure derived `const total
530+
* = a + b` is never a fresh-instance init, so it is never frozen.
531+
* - member-mutation: the binder must actually be mutated; a read-only fresh
532+
* instance is left alone (its identity churn is harmless).
533+
*
534+
* `let` binders that are also REASSIGNED (`X = …`, a bare-identifier write) are
535+
* excluded — useMemo cannot model reassignment; those are the `hoistModuleLet`
536+
* → `useRef` case (already handled when they escape into a lifecycle hook).
537+
*/
538+
function collectMutatedInstanceBinders(program: t.File): Set<string> {
539+
// 1. Candidate binders: top-level single-name const/let with a fresh-instance init.
540+
const candidates = new Set<string>();
541+
for (const stmt of program.program.body) {
542+
if (!t.isVariableDeclaration(stmt)) continue;
543+
if (stmt.kind !== 'const' && stmt.kind !== 'let') continue;
544+
for (const d of stmt.declarations) {
545+
if (!t.isIdentifier(d.id)) continue;
546+
if (!d.init) continue;
547+
if (t.isArrowFunctionExpression(d.init) || t.isFunctionExpression(d.init)) continue;
548+
if (!isFreshMutableInstanceInit(d.init)) continue;
549+
candidates.add(d.id.name);
550+
}
551+
}
552+
if (candidates.size === 0) return candidates;
553+
554+
// 2. Walk the whole program. Record member-mutations (→ mutated) and bare
555+
// reassignments (→ reassigned, which disqualify a `let`).
556+
const mutated = new Set<string>();
557+
const reassigned = new Set<string>();
558+
traverse(program, {
559+
AssignmentExpression(path) {
560+
const left = path.node.left;
561+
if (t.isIdentifier(left)) {
562+
if (candidates.has(left.name)) reassigned.add(left.name);
563+
return;
564+
}
565+
if (t.isMemberExpression(left)) {
566+
const r = memberRootName(left);
567+
if (r && candidates.has(r)) mutated.add(r);
568+
}
569+
},
570+
UpdateExpression(path) {
571+
const arg = path.node.argument;
572+
if (t.isIdentifier(arg)) {
573+
if (candidates.has(arg.name)) reassigned.add(arg.name);
574+
return;
575+
}
576+
if (t.isMemberExpression(arg)) {
577+
const r = memberRootName(arg);
578+
if (r && candidates.has(r)) mutated.add(r);
579+
}
580+
},
581+
UnaryExpression(path) {
582+
if (path.node.operator !== 'delete') return;
583+
const arg = path.node.argument;
584+
if (t.isMemberExpression(arg)) {
585+
const r = memberRootName(arg);
586+
if (r && candidates.has(r)) mutated.add(r);
587+
}
588+
},
589+
CallExpression(path) {
590+
const callee = path.node.callee;
591+
if (!t.isMemberExpression(callee) || callee.computed) return;
592+
if (!t.isIdentifier(callee.property)) return;
593+
if (!MUTATING_INSTANCE_METHODS.has(callee.property.name)) return;
594+
if (!t.isIdentifier(callee.object)) return;
595+
if (candidates.has(callee.object.name)) mutated.add(callee.object.name);
596+
},
597+
});
598+
599+
// Member-mutated AND not bare-reassigned (consts can never be reassigned).
600+
const result = new Set<string>();
601+
for (const name of mutated) {
602+
if (!reassigned.has(name)) result.add(name);
603+
}
604+
return result;
605+
}
606+
607+
/**
608+
* Phase 35 follow-up — wrap a top-level member-mutated fresh-instance
609+
* `const`/`let X = init` in `useMemo(() => init, [])` so the instance is built
610+
* ONCE per component instance (setup-once parity). Companion detection lives in
611+
* `collectMutatedInstanceBinders`. Returns the rendered line, or null when the
612+
* binder isn't a stabilization target.
613+
*
614+
* Distinct from `tryWrapEscapingConstUseMemo`: that wraps a const that escapes
615+
* into a useEffect dep array, keyed on its real reactive deps (`[...initDeps]`).
616+
* This wraps cross-render MUTABLE STATE, which must NEVER be re-created — hence
617+
* the empty `[]` dep array regardless of what the init reads (matching the
618+
* setup-once targets, which capture init values once and never re-read them).
619+
* Runs FIRST in the loop so a binder that is both escaping and mutated gets the
620+
* stable `[]` identity.
621+
*/
622+
function tryWrapMutatedInstanceUseMemo(
623+
stmt: t.Statement,
624+
mutatedInstanceNames: ReadonlySet<string>,
625+
collectors: { react: ReactImportCollector; runtime: RuntimeReactImportCollector },
626+
): string | null {
627+
if (!t.isVariableDeclaration(stmt)) return null;
628+
if (stmt.kind !== 'const' && stmt.kind !== 'let') return null;
629+
if (stmt.declarations.length !== 1) return null;
630+
const decl = stmt.declarations[0]!;
631+
if (!t.isIdentifier(decl.id)) return null;
632+
const init = decl.init;
633+
if (!init) return null;
634+
if (t.isArrowFunctionExpression(init) || t.isFunctionExpression(init)) return null;
635+
if (!mutatedInstanceNames.has(decl.id.name)) return null;
636+
637+
collectors.react.add('useMemo');
638+
// Preserve an author declarator annotation (`const seen: Set<string> = …`):
639+
// `useMemo(() => new Set(), [])` would otherwise infer `Set<unknown>`.
640+
const declTypeSuffix = renderDeclaratorTypeSuffix(decl.id);
641+
return `const ${decl.id.name}${declTypeSuffix} = useMemo(${arrowBody(init)}, []);`;
642+
}
643+
481644
/**
482645
* Find all distinct `props.onX` names that appear in CALL position inside
483646
* `body`. Returns the set of names (without the `on` prefix? no — keep raw
@@ -2459,6 +2622,14 @@ export function emitScript(
24592622
}
24602623
}
24612624

2625+
// 6a'. Phase 35 follow-up (feedback_react_const_mutinstance_not_stabilized) —
2626+
// collect top-level fresh-instance const/let binders that are member-mutated
2627+
// (cross-render scratch state, e.g. CodeMirrorDemo's `const gutterSeen = new
2628+
// Set()` dedupe guard). These must persist per-instance like the setup-once
2629+
// targets; without stabilization React re-creates them every render and the
2630+
// guard resets → render side-effects re-fire → infinite re-render → VR hang.
2631+
const mutatedInstanceNames = collectMutatedInstanceBinders(cloned);
2632+
24622633
// 6b. Helper-name pre-scan (`allHelperNames` / `helperLocByName`) is built
24632634
// earlier — hoisted above the lifecycle + watcher loops in Round 4 of
24642635
// 260519 linechart-watch-recreate so the watcher loop's callback-body walk
@@ -2547,6 +2718,21 @@ export function emitScript(
25472718
}
25482719
}
25492720

2721+
// Phase 35 follow-up — member-mutated fresh-instance binder → useMemo(()=>init,[]).
2722+
// Runs FIRST so a binder that is BOTH escaping and mutated gets the stable
2723+
// `[]` identity (cross-render mutable state must never be re-created),
2724+
// rather than the reactive-deps `[...]` from tryWrapEscapingConstUseMemo.
2725+
const mutMemoWrapped = tryWrapMutatedInstanceUseMemo(
2726+
stmt,
2727+
mutatedInstanceNames,
2728+
collectors,
2729+
);
2730+
if (mutMemoWrapped) {
2731+
userArrowsLines.push(mutMemoWrapped);
2732+
// String output (like the other wraps) — excluded from mappableStmts.
2733+
continue;
2734+
}
2735+
25502736
// Try the useCallback escape-wrap first; falls back to function-decl
25512737
// hoist if the helper isn't escaping.
25522738
const wrapped = tryWrapEscapingHelperUseCallback(

0 commit comments

Comments
 (0)