fix: prevent infinite re-renders in Renderer component #76
Conversation
Root causes fixed: 1. DataProvider.get callback depended on data state, causing cascading memo invalidation through all consumers. Now uses a ref to read current data, keeping the callback identity stable. 2. ActionProvider.execute depended on data/handlers/set/navigate directly, so every data change recreated the callback and invalidated the context value. Now uses refs for frequently-changing values. 3. createRenderer created a new actionHandlers object on every render, causing ActionProvider to receive new props each cycle. Now memoized with useMemo. 4. DataProvider setData in the sync useEffect could produce a new object reference even when values hadn't changed. Added bail-out check. 5. Added useEffect to sync ActionProvider handlers when props change. Closes vercel-labs#53
|
Someone is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df503e26b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Only update if the handlers object reference actually changed and has entries | ||
| if (initialHandlers !== initialHandlersRef.current) { | ||
| initialHandlersRef.current = initialHandlers; | ||
| if (Object.keys(initialHandlers).length > 0) { |
There was a problem hiding this comment.
The new synchronization effect only updates state when initialHandlers has keys, so if the parent later passes {}/undefined (for example, createRenderer toggling onAction off), the previous handler map is retained indefinitely. In that state, execute still sees actions as registered and can continue running downstream onSuccess/onError behavior, so disabling handlers no longer actually disables action execution. The sync path should also clear handlers when the prop becomes empty.
Useful? React with 👍 / 👎.
1 participant
Summary
Fixes #53 — Renderer component causes infinite re-renders when given a static JSON tree in non-streaming scenarios.
Root Causes
The infinite re-render loop was caused by multiple interacting issues in the context providers:
1. Unstable
getcallback inDataProvideruseCallbackforgetdepended ondatastate, so every data change created a newget→ new context value → all consumers re-rendered. Fixed by using a ref to read current data, keeping callback identity stable.2. Unstable
executecallback inActionProviderexecutedepended ondata,handlers,set, andnavigatedirectly. Any data change recreatedexecute→ new context value → consumers re-render → potential cascading updates. Fixed by reading frequently-changing values from refs.3. Inline
actionHandlersobject increateRendererA new
{ __default__: ... }object was created on every render ofCatalogRenderer, causingActionProviderto receive new props each cycle. Fixed by memoizing withuseMemoand using a ref for theonActioncallback.4. Missing bail-out in
DataProvidersync effectsetData((prev) => ({ ...prev, ...initialData }))always created a new object reference even when values hadn't changed, triggering unnecessary re-renders. Added a JSON equality check to bail out when the merged result is identical.Changes
packages/react/src/contexts/data.tsx— Stabilizedgetcallback with ref; added bail-out in sync effectpackages/react/src/contexts/actions.tsx— Stabilizedexecutecallback with refs; added handler sync effectpackages/react/src/renderer.tsx— MemoizedactionHandlersincreateRendererpackages/react/src/contexts/rerender.test.tsx— Added 4 regression testsTests
All 186 tests pass (11 test files), including 4 new regression tests that verify:
getcallback identity remains stable across data changes