fix: stabilize register/unregister callbacks without React Compiler#3
fix: stabilize register/unregister callbacks without React Compiler#3seanryy wants to merge 1 commit into
Conversation
registerSeries/unregisterSeries (chart-context.tsx) and registerVariant/ unregisterVariant (polar-context.tsx) are plain closures recreated every render, and area/bar/pie/radar list them in their useEffect deps. With React Compiler that's fine — it memoizes them — but in a setup without babel-plugin-react-compiler every render re-fires the effect, and the unregister/register setState pair triggers another render, looping until React throws "Maximum update depth exceeded". Wrap the four callbacks in useCallback with empty deps — each only calls its setState updater, which React guarantees stable — so the components work with or without the compiler. Regenerated r/ payloads.
…t Compiler Companion to Boring-Software-Inc#3 (which stabilizes the register/unregister callbacks): Sparkline rebuilds rows/config on every render, and rows identity feeds useRevision's adjust-state-during-render — which never stabilizes when the array is fresh each pass. Memoize both against data/color. Registry JSON regenerated via npm run build.
…t Compiler Companion to Boring-Software-Inc#3 (which stabilizes the register/unregister callbacks): Sparkline rebuilds rows/config on every render, and rows identity feeds useRevision's adjust-state-during-render — which never stabilizes when the array is fresh each pass. Memoize both against data/color. Registry JSON regenerated via npm run build.
DloomPlz
left a comment
DloomPlz
left a comment
There was a problem hiding this comment.
Independently hit this exact bug adopting the kit in a production Next.js app (no React Compiler): mounting any chart entered the unregister/register effect loop and hung the tab. We applied the same fix pattern — useCallback with empty deps on all four register/unregister callbacks — and have been running it in production since; charts behave identically under the compiler-less runtime, and compiler-enabled projects are unaffected (this is what the compiler emits anyway).
Verified the diff covers both the cartesian (registerSeries/unregisterSeries) and polar (registerVariant/unregisterVariant) controllers, which is the complete set of effect-dep callbacks. One companion gap this PR doesn't cover (by design): Sparkline's derived rows/config rebuild per render and feed useRevision's adjust-state-during-render — #4 handles that piece so the two merge cleanly together and fully close #2.
3 participants
Fixes #2.
registerSeries/unregisterSeries(chart-context.tsx) andregisterVariant/unregisterVariant(polar-context.tsx) were plain closures recreated every render, whilearea.tsx/bar.tsx/pie.tsx/radar.tsxlist them inuseEffectdeps. Without React Compiler the effect re-fires every render and the unregister/registersetStatepair loops until React throws "Maximum update depth exceeded" — see #2 for the full repro (plain Vite or Astro +@astrojs/react, nobabel-plugin-react-compiler).Changes:
useCallbackwith empty deps. Each only calls itssetStateupdater, which React guarantees stable, so[]is safe.r/payloads withnpm run build(onlyr/core.jsonchanged).No behavior change for compiler-enabled consumers —
useCallbackunder React Compiler is a no-op memo-wise. We've been running this exact patch in a vendored copy in production (Astro 5 + React 19, no compiler) without issues.