fix(core): stop useTexture's array/record forms looping forever - #3858
Open
DennisSmolek wants to merge 1 commit into
Open
fix(core): stop useTexture's array/record forms looping forever#3858DennisSmolek wants to merge 1 commit into
DennisSmolek wants to merge 1 commit into
Conversation
useTexture with an array or record argument re-ran its registry effect on every render, and the store.setState that effect performs re-renders every store subscriber, so the two fed each other until React bailed out with "Maximum update depth exceeded". The single-URL form was unaffected. Two independent instabilities had to go, and fixing either alone still loops: useLoader built its result array with .map() on every call, so the returned array had a new identity every render. That is the root cause and it is not useTexture-specific -- any consumer using the result as an effect dependency hits it. The array is now shallow-compared and its identity preserved while the contents are unchanged. Shallow-compared rather than keyed on the cache keys, because useLoader.clear() can hand back new objects for the same keys and that invalidation still has to propagate. useTexture keyed its memos and its registry effect on the raw `input` reference. Callers routinely pass an inline literal, so that reference is new every render too. Everything now keys off a value signature (which includes the record's keys -- two records can share URLs but map them to different names) and an identity-stable view of the input. Tests reproduce the reported failure exactly: the broad `useThree()` subscription lives in the same component as useTexture, because a sibling subscriber re-renders itself and not the useTexture caller, and without that the tests pass even on the broken build. Fixes #3849 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Fixes #3849.
useTexturewith an array or record argument re-ran its registry effect on every render, and thestore.setStatethat effect performs re-renders every store subscriber, so the two fed each other until React bailed out withMaximum update depth exceeded. The single-URL form was unaffected.Two instabilities, both load-bearing
Fixing either one alone still loops — I verified this by reverting each half independently.
1.
useLoaderreturned a new array every call..map()builds a fresh array each render, so the result could never be used as an effect dependency — by anyone. This is the root cause and it isn'tuseTexture-specific;useTexture's registry effect was just the first place it bit.The array is now shallow-compared and its identity preserved while the contents are unchanged. Shallow-compared rather than keyed on the cache keys — that alternative looks simpler but silently breaks invalidation, because
useLoader.clear()can hand back new objects for the same keys and that change still has to propagate. Comparing resolved values means a genuine change always yields a new identity and an incidental re-render never does.2.
useTexturekeyed its memos and effect on the rawinputreference. Callers routinely pass an inline literal, so that reference is new every render too. Everything now keys off a value signature plus an identity-stable view of the input.The signature includes the record's keys, not just its URLs — two records can share URLs while mapping them to different names, and those aren't interchangeable.
On the render-phase ref write in
useLoaderuseLoaderpreviously used no React hooks. It now uses oneuseRef. The write during render is idempotent — given equal contents it always settles on the same array — so it's safe under StrictMode's double render. Flagging it since it's the least conventional part of this change.Testing
Three regression tests, all confirmed failing on pre-fix source with the reported error:
Worth knowing if you edit these: the
useThree()call inside each Consumer is load-bearing. The loop only closes when the component callinguseTextureis itself a broad store subscriber — a sibling subscriber re-renders itself, not theuseTexturecaller. My first draft put it in a sibling and the tests passed on the broken build. The issue's own repro has it in the same component; that detail matters.pnpm test✅ 584 passed (was 581), 45 filespnpm typecheck/pnpm eslint/pnpm format✅🤖 Generated with Claude Code