Skip to content

Mirror of upstream PR #33188 #42

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Mirror of upstream PR #33188 #42

wants to merge 3 commits into from

Conversation

kushxg
Copy link
Owner

@kushxg kushxg commented May 13, 2025

Mirrored from facebook/react PR facebook#33188

sammy-SC and others added 3 commits May 12, 2025 17:39
This is a fix for a problem where React retains shadow nodes longer than
it needs to. The behaviour is shown in React Native test:
https://github.com/facebook/react-native/blob/main/packages/react-native/src/private/__tests__/utilities/__tests__/ShadowNodeReferenceCounter-itest.js#L169

# Problem
When React commits a new shadow tree, old shadow nodes are stored inside
`fiber.alternate.stateNode`. This is not cleared up until React clones
the node again. This may be problematic if mutation deletes a subtree,
in that case `fiber.alternate.stateNode` will retain entire subtree
until next update. In case of image nodes, this means retaining entire
images.

So when React goes from revision A: `<View><View /></View>` to revision
B: `<View />`, `fiber.alternate.stateNode` will be pointing to Shadow
Node that represents revision A..


![image](https://github.com/user-attachments/assets/076b677e-d152-4763-8c9d-4f923212b424)


# Fix
To fix this, this PR adds a new feature flag
`enableEagerAlternateStateNodeCleanup`. When enabled,
`alternate.stateNode` is proactively pointed towards finishedWork's
stateNode, releasing resources sooner.

I have verified this fixes the issue [demonstrated by React Native
tests](https://github.com/facebook/react-native/blob/main/packages/react-native/src/private/__tests__/utilities/__tests__/ShadowNodeReferenceCounter-itest.js#L169).
All existing React tests pass when the flag is enabled.
…ook#33028)

## Summary
Our builds generate files with a `.mjs` file extension. These are
currently filtered out by `ReactFlightWebpackPlugin` so I am updating it
to support this file extension.

This fixes facebook#33155

## How did you test this change?
I built the plugin with this change and used `yalc` to test it in my
project. I confirmed the expected files now show up in
`react-client-manifest.json`
When we're entangled with an async action lane we use that lane
instead of the currentEventTransitionLane. Conversely, if we start a new
async action lane we reuse the currentEventTransitionLane.

So they're basically supposed to be in sync but they're not if you resolve
the async action and then schedule new stuff in the same event. Then you
end up with two transitions in the same event with different lanes.

By stashing it like this we fix that but it also gives us an opportunity to
check just the currentEventTransitionLane to see if this event scheduled
any regular Transition updates or Async Transitions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants