Fix freeze from useBoxMetrics layout listener recursion#966
Open
msfeldstein wants to merge 3 commits into
Open
Fix freeze from useBoxMetrics layout listener recursion#966msfeldstein wants to merge 3 commits into
msfeldstein wants to merge 3 commits into
Conversation
emitLayoutListeners runs from React's resetAfterCommit host callback. Invoking listeners synchronously lets hooks (e.g. useBoxMetrics) schedule React state while React is still committing, which can recurse until React trips its nested update guard and freezes the CLI. Coalesce pending listeners and flush them in a microtask after the commit finishes, skipping any listener removed before the flush. Co-authored-by: Cursor <cursoragent@cursor.com>
A useBoxMetrics-driven box whose rendered height depends on its measured height creates a layout feedback loop. Without deferring layout listener callbacks, this recurses synchronously inside React's commit and crashes with "Maximum update depth exceeded"; with the fix it converges. Co-authored-by: Cursor <cursoragent@cursor.com>
The previous "runs after commit stack" test only pinned the microtask mechanism, not the property we care about. Replace it with an integration test that renders a layout-feedback component and asserts render does not throw "Maximum update depth exceeded" — this fails without the fix and is agnostic to how the deferral is implemented. Keep the removed-listener test, which guards the stale-listener edge case the deferral introduces. Co-authored-by: Cursor <cursoragent@cursor.com>
5dd5884 to
6ac2c9e
Compare
Collaborator
|
I don't think this is fixed yet.
|
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
resetAfterCommitstack so layout listeners (e.g.useBoxMetrics) can't synchronously schedule React state while React is still committing.Problem
emitLayoutListenersruns from Ink'sresetAfterCommithost callback. It invokes listeners synchronously, so a listener that schedules state (useBoxMetricscallssetMetrics/setHasMeasured) updates React mid-commit. When the rendered layout depends on the measured layout, each commit re-measures and re-commits, recursing until React trips its nested-update guard and throwsMaximum update depth exceeded, freezing the CLI.Fix
Buffer listeners into a pending set and flush them once in a
queueMicrotaskafter the commit unwinds. The flush re-checks subscription so a listener removed in the window between commit and flush (e.g. an unmounting component) does not fire.Test plan
npx ava test/dom-layout-listeners.tsx— new regression test renders a layout-feedback box and asserts render does not throw. Verified it fails without the fix (Maximum update depth exceeded) and passes with it.npx ava test/use-box-metrics.tsxnpx tsc --noEmitnpx prettier --checknode --import=tsx examples/layout-listener-recursion/index.tscrashes without the fix, converges with it.Made with Cursor