Forensic render-perf & cleanup audit (post §4 / §16) — findings report

[Copilot]

Per the issue's instruction ("just want a thorough report… no code changes"), this PR contains no source modifications. It records the forensic survey of src/ against the anti-pattern checklist in the issue, ranked by likely impact and triage cluster.

Headline findings

• No React.memo anywhere (grep -E 'React\.memo|memo\(' src → 0 hits). Combined with the next item, every parent re-render walks the entire JSON tree.
• Every Context provider's value={…} is a fresh object every render, with inline closures inside it. Even adding React.memo would be neutralised by the prop-bag spreads ({...props}) cascading down CollectionNode / ValueNodeWrapper.
• 3 confirmed timer leaks (missing setTimeout cleanups), independently fixable.

Real, high-impact (Cluster A — context refactor)

• src/contexts/TreeStateProvider.tsx:115-150 — inline value={{…}}, inline setCollapseState / setPreviouslyEditedElement / setTabDirection closures, and previouslyEdited.current / tabDirection.current read during render (concurrent-mode hazard).
• src/contexts/ThemeProvider.tsx:67-83 — inline value, compileStyles(theme, docRoot) runs every render (multi-pass merge), and docRoot.style.setProperty(...) is a side effect during render.
• src/JsonEditor.tsx:81-95 — defaults translations = {}, customNodeDefinitions = [], collapseClickZones = ['header', 'left'], etc. allocate fresh literals per render and feed useMemo deps that consequently never hit cache. (A prior repo memory claimed module-scoped EMPTY_* constants existed for these — verified false, downvoted.)
• src/JsonEditor.tsx:117-126 + src/hooks/useCommon.ts:41 — nodeData rebuilt every render and threaded through every child as a prop.
• src/JsonEditor.tsx:131-270 — onEdit/onDelete/onAdd/onMove/handleEdit not useCallback; flow into otherProps (line 351-403) which is itself a fresh object spread into the recursive <CollectionNode {...props} />.
• src/ValueNodeWrapper.tsx:160-164 — setState during render targeting a different component (setCurrentlyEditingElement from inside ValueNodeWrapper render). Forbidden under concurrent rendering.

Missing cleanups (Cluster B — timer leaks)

• src/contexts/TreeStateProvider.tsx:129 — setTimeout(() => setCollapseState(null), 2000) untracked, no cleanup, multiple in flight under rapid toggles.
• src/hooks/useCommon.ts:51-57 — showError's setTimeout(() => setError(null), errorMessageTimeout) is fire-and-forget; consecutive errors race.
• src/hooks/useCollapseTransition.ts:86-102 — the 5 ms setMaxHeight(0) timer is untracked, and the longer animation timer (timerId.current) has no unmount-time cleanup.

Test pin pattern (per issue): jest.useFakeTimers() + mount + trigger + unmount + expect(jest.getTimerCount()).toBe(0).

Real but lower-impact

• src/CollectionNode.tsx:293-298 — Object.entries(data).map(...) + sort every render; O(n log n) per parent re-render.
• src/CollectionNode.tsx:73 / src/ValueNodeWrapper.tsx:66 — useState(jsonStringify(data)) runs the serializer eagerly; should use lazy initialiser form.
• src/CollectionNode.tsx:316-345 — array children keyed by numeric key (== index). Causes per-node state (e.g. StringDisplay.isExpanded) to migrate to the wrong row on insert/delete/move. Author already mitigates this for cross-array moves via arr_${originalKey} but not for in-place renders.
• src/ValueNodeWrapper.tsx:92-110 — updateValue useCallback([onChange]) closes over value/nodeData.fullData; classic stale-closure vector for onChange.
• src/ValueNodeWrapper.tsx:128-134 — allDataTypes rebuilt every render; downstream allowedDataTypes useMemo keys on nodeData (new each render) so the cache never hits.

Already correct (worth a regression test, not a fix)

• src/ValueNodes.tsx:294-329 useKeyboardListener — listener add/remove balance via currentListener ref pattern is sound. Pin with a window.addEventListener spy.
• src/JsonEditor.tsx:110-115 — eslint-disable comment is technically wrong (setCurrentlyEditingElement is the rebuilt wrapper, not a stable React setter), but excluding it from deps is the correct behaviour.

Suggested triage / PR shape

1. Cluster A (context split + memoisation of prop bags + fix render-time setState) — coupled, ship together.
2. Cluster B (timer leaks) — independent, highest signal/effort. Ship separately.
3. Cluster C (React.memo pass) — only meaningful after A lands.
4. Cluster D (defaults, compileStyles memo, lazy initialisers, stable list keys) — small independent wins.

Out of scope, per issue: 10k-node wall-clock benchmark.

Memory hygiene

Two repo memories verified against current code and downvoted:

• JsonEditor uses module-scoped EMPTY_* constants… — defaults are inline literals; no such constants.
• EditingProvider.startEdit()… — EditingProvider.tsx does not exist; equivalent logic is in TreeStateProvider.updateCurrentlyEditingElement.

[CarlosNZ]

Applied to the wrong branch