Add eager alternate.stateNode cleanup #33161

sammy-SC · 2025-05-09T15:02:00Z

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..

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.
All existing React tests pass when the flag is enabled.

react-sizebot · 2025-05-09T15:05:28Z

Comparing: 21fdf30...2f38d6a

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.js	=	6.68 kB	6.68 kB	=	1.83 kB	1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js	=	528.09 kB	528.09 kB	=	93.17 kB	93.17 kB
oss-experimental/react-dom/cjs/react-dom.production.js	=	6.69 kB	6.69 kB	+0.05%	1.83 kB	1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js	=	646.40 kB	646.40 kB	=	113.71 kB	113.71 kB
facebook-www/ReactDOM-prod.classic.js	=	674.15 kB	674.15 kB	=	118.41 kB	118.41 kB
facebook-www/ReactDOM-prod.modern.js	=	664.43 kB	664.43 kB	=	116.81 kB	116.81 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 2f38d6a

sebmarkbage · 2025-05-09T20:35:49Z

packages/react-reconciler/src/ReactFiberCommitWork.js

@@ -2171,6 +2172,11 @@ function commitMutationEffectsOnFiber(
          }
        }
      }
+      if (enableEagerAlternateStateNodeCleanup) {


This should be in the else branch of if (supportsMutation) and it should check if (supportsPersistence) since this should only apply to the persistent mode which is the only mode that clones the state nodes.

The flag is only on in RN but it's still on for Paper unnecessarily. You're better off turning the flag on everywhere else because that flushes out any issues where it would have issues if it turned on in the other renders. Like not gating it properly on supportsPersistence.

let me do that. My thinking was that it is a no-op in supportsMutation mode as stateNode will be the same between finishedWork and alternate.

thanks for pointing out the supportsPersistence. I will also add a comment explaining why this is needed.

yungsters · 2025-05-12T13:22:43Z

packages/react-reconciler/src/ReactFiberCommitWork.js

+            // prevents shadow node from staying in memory longer than it
+            // needs to. The correct behaviour of this is checked by test in
+            // React Native: ShadowNodeReferenceCounter-itest.js#L150
+            finishedWork.alternate.stateNode = finishedWork.stateNode;


Maybe @sebmarkbage or @rickhanlonii…

What are the correctness trade offs between setting this to finishedWork.stateNode versus null?

setting it to null seems wrong because this is a shared value between the current and alternate fibers, and I don't think we do that for DOM, but maybe @sebmarkbage will say I'm wrong and it's fine. What's the benefit? It doesn't seem like it would allow releasing any additional memory.

packages/react-reconciler/src/ReactFiberCommitWork.js

rickhanlonii · 2025-05-12T15:10:26Z

packages/shared/ReactFeatureFlags.js

@@ -141,6 +141,8 @@ export const enablePersistedModeClonedFlag = false;

 export const enableShallowPropDiffing = false;

+export const enableEagerAlternateStateNodeCleanup = false;


This can be true here, as well as the other non-RN files. Since this only impacts RN, we don't need to set it to false. Makes it easier to roll out - and if it causes a test to fail somehow (it shouldn't) we'll catch it now instead of when trying to remove the flag

packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js

rickhanlonii

LGTM, just a couple nits

yungsters · 2025-05-12T16:33:40Z

packages/react-reconciler/src/ReactFiberCommitWork.js

+      } else {
+        if (enableEagerAlternateStateNodeCleanup) {
+          if (supportsPersistence) {


I think what you have here is fine, but I think what @rickhanlonii meant was this.

Suggested change

} else {

if (enableEagerAlternateStateNodeCleanup) {

if (supportsPersistence) {

} else if (supportsPersistence) {

if (enableEagerAlternateStateNodeCleanup) {

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. DiffTrain build for [5d04d73](5d04d73)

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. DiffTrain build for [5d04d73](facebook@5d04d73)

Summary: Enables the `enableEagerAlternateStateNodeCleanup` feature flag in the open source React Native renderers that are currently targeting React 19.1, by manually patching them in the React Native repository. This feature flag has been found to significantly improve memory management of parent alternate fibers in persistent modes (i.e. Fabric), and we want this to be available to open source users of React Native before the next scheduled public version release of React. For more details about the fix, see: facebook/react#33161 Changelog: [General][Changed] - Reduces memory usage by improving memory management of parent alternate fibers. Differential Revision: D76073900

…#51856) Summary: Pull Request resolved: #51856 Enables the `enableEagerAlternateStateNodeCleanup` feature flag in the open source React Native renderers that are currently targeting React 19.1, by manually patching them in the React Native repository. This feature flag has been found to significantly improve memory management of parent alternate fibers in persistent modes (i.e. Fabric), and we want this to be available to open source users of React Native before the next scheduled public version release of React. For more details about the fix, see: facebook/react#33161 Changelog: [General][Changed] - Reduces memory usage, by improving memory management of parent alternate fibers. (Previously, a parent fiber might retain memory associated with shadow nodes from a previous commit.) Reviewed By: rickhanlonii Differential Revision: D76073900 fbshipit-source-id: 6779ea0862d4a1e25354b12ef3d1363dc12d26cc

add eager statenode cleanup

a721b37

sammy-SC requested review from yungsters and jackpope May 9, 2025 15:02

facebook-github-bot added the CLA Signed label May 9, 2025

sammy-SC self-assigned this May 9, 2025

sammy-SC requested a review from kassens May 9, 2025 15:48

sebmarkbage reviewed May 9, 2025

View reviewed changes

kushxg mentioned this pull request May 12, 2025

Mirror of upstream PR #33161 kushxg/react-mirror#6

Open

sammy-SC added 3 commits May 12, 2025 10:33

add comment + scope the change to Fabric only

1a34807

format comment

ee701ea

run yarn prettier-all

d281a97

yungsters reviewed May 12, 2025

View reviewed changes

rickhanlonii reviewed May 12, 2025

View reviewed changes

packages/react-reconciler/src/ReactFiberCommitWork.js Outdated Show resolved Hide resolved

rickhanlonii reviewed May 12, 2025

View reviewed changes

packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js Show resolved Hide resolved

rickhanlonii approved these changes May 12, 2025

View reviewed changes

address commits from Rick

2f38d6a

yungsters reviewed May 12, 2025

View reviewed changes

sammy-SC merged commit 5d04d73 into facebook:main May 12, 2025
239 checks passed

sammy-SC deleted the sammy-sc/add-eager-stateNode-cleanup branch May 12, 2025 16:39

vercel-release-bot mentioned this pull request May 21, 2025

Upgrade React from 197d6a04-20250424 to 23884812-20250520 vercel/next.js#79462

Closed

vercel-release-bot mentioned this pull request May 21, 2025

Upgrade React from c4676e72-20250520 to 23884812-20250520 vercel/next.js#79467

Closed

yungsters mentioned this pull request Jun 5, 2025

RN: Patch enableEagerAlternateStateNodeCleanup into Renderers (OSS) facebook/react-native#51856

Closed

eps1lon mentioned this pull request Jun 7, 2025

Upgrade React from 197d6a04-20250424 to latest vercel/next.js#79477

Draft

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add eager alternate.stateNode cleanup #33161

Add eager alternate.stateNode cleanup #33161

Uh oh!

sammy-SC commented May 9, 2025 •

edited

Loading

Uh oh!

react-sizebot commented May 9, 2025 •

edited

Loading

Uh oh!

sebmarkbage May 9, 2025

Uh oh!

sammy-SC May 12, 2025

Uh oh!

yungsters May 12, 2025

Uh oh!

rickhanlonii May 12, 2025

Uh oh!

Uh oh!

rickhanlonii May 12, 2025

Uh oh!

Uh oh!

rickhanlonii left a comment

Uh oh!

yungsters May 12, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

		@@ -141,6 +141,8 @@ export const enablePersistedModeClonedFlag = false;

		export const enableShallowPropDiffing = false;

		export const enableEagerAlternateStateNodeCleanup = false;

Add eager alternate.stateNode cleanup #33161

Add eager alternate.stateNode cleanup #33161

Uh oh!

Conversation

sammy-SC commented May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Uh oh!

react-sizebot commented May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Critical size changes

Significant size changes

Uh oh!

sebmarkbage May 9, 2025

Choose a reason for hiding this comment

Uh oh!

sammy-SC May 12, 2025

Choose a reason for hiding this comment

Uh oh!

yungsters May 12, 2025

Choose a reason for hiding this comment

Uh oh!

rickhanlonii May 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rickhanlonii May 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rickhanlonii left a comment

Choose a reason for hiding this comment

Uh oh!

yungsters May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sammy-SC commented May 9, 2025 •

edited

Loading

react-sizebot commented May 9, 2025 •

edited

Loading

yungsters May 12, 2025 •

edited

Loading