fix: re-create disconnected style resource instances in acquireResource#36398
Open
sleitor wants to merge 1 commit intofacebook:mainfrom
Open
fix: re-create disconnected style resource instances in acquireResource#36398sleitor wants to merge 1 commit intofacebook:mainfrom
sleitor wants to merge 1 commit intofacebook:mainfrom
Conversation
When a <style precedence> resource is rendered inside a createPortal container and that container is later removed from the DOM, React kept a stale reference to the now-disconnected <style> element in the hoistable-styles cache. On subsequent renders (e.g. when the same href renders in the main tree), acquireResource would skip the creation path because resource.instance was non-null, and return the detached element. The style was then permanently lost for the rest of the session. Fix: before checking resource.instance === null, also reset it to null if the cached element is no longer connected to any document. This causes acquireResource to re-create and re-insert the style element in the correct hoistable root (the document head). Fixes: facebook#36373
|
Comparing: 9635257...980caf0 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
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
2 participants
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
When a
<style href="..." precedence="...">resource is rendered inside acreatePortalcontainer and that container is later removed from the DOM, React retains a stale reference to the now-disconnected<style>element in the hoistable-styles resource cache.On the next render of the same
href(e.g. the component also renders in the main tree),acquireResourceskips the creation branch becauseresource.instanceis non-null, and returns the detached element. The<style>is gone from the document and styles are permanently lost for the rest of the session.Root cause
ReactFiberConfigDOM.js#acquireResourcechecksif (resource.instance === null)before creating a new element. When the portal container is removed,resource.instanceis still set to the element that lived inside it — it is now disconnected (isConnected: false) but the cache never invalidates it.Fix
Before the
resource.instance === nullguard, check whether the cached instance is still connected to the document:This causes the code to fall through to the creation path, which re-creates the
<style>element and inserts it in the correct hoistable root (the document<head>).How did you test this change?
Added a regression test in
ReactDOMFloat-test.js:<style href="x" precedence="custom">inside both a portal container and the main tree.<head>.All existing Style Resource tests still pass (9 total, +1 new).
Fixes #36373