From aba370f1e45d21f19f33c04c33fc99fb3d0109e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Fri, 22 Nov 2024 13:24:29 -0500 Subject: [PATCH] Add moveBefore Experiment (#31596) A long standing issue for React has been that if you reorder stateful nodes, they may lose their state and reload. The thing moving loses its state. There's no way to solve this in general where two stateful nodes swap. The [`moveBefore()` proposal](https://chromestatus.com/feature/5135990159835136?gate=5177450351558656) has now moved to [intent-to-ship](https://groups.google.com/a/chromium.org/g/blink-dev/c/YE_xLH6MkRs/m/_7CD0NYMAAAJ). This function is kind of like `insertBefore` but preserves state. There's [a demo here](https://state-preserving-atomic-move.glitch.me/). Ideally we'd port this demo to a fixture so we can try it. Currently this flag is always off - even in experimental. That's because this is still behind a Chrome flag so it's a little early to turn it on even in experimental. So you need a custom build. It's on in RN but only because it doesn't apply there which makes it easier to tell that it's safe to ship once it's on everywhere else. The other reason it's still off is because there's currently a semantic breaking change. `moveBefore()` errors if both nodes are disconnected. That happens if we're inside a completely disconnected React root. That's not usually how you should use React because it means effects can't read layout etc. However, it is currently supported. To handle this we'd have to try/catch the `moveBefore` to handle this case but we hope this semantic will change before it ships. Before we turn this on in experimental we either have to wait for the implementation to not error in the disconnected-disconnected case in Chrome or we'd have to add try/catch. --- .../src/client/ReactFiberConfigDOM.js | 22 +++++++++++++++++-- packages/shared/ReactFeatureFlags.js | 3 +++ .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + ...actFeatureFlags.test-renderer.native-fb.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 1 + 8 files changed, 29 insertions(+), 2 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 3315ce92bbc4f..bfa9adb48d737 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -93,6 +93,7 @@ import { enableTrustedTypesIntegration, enableAsyncActions, disableLegacyMode, + enableMoveBefore, } from 'shared/ReactFeatureFlags'; import { HostComponent, @@ -525,6 +526,7 @@ export function appendInitialChild( parentInstance: Instance, child: Instance | TextInstance, ): void { + // Note: This should not use moveBefore() because initial are appended while disconnected. parentInstance.appendChild(child); } @@ -757,11 +759,22 @@ export function commitTextUpdate( textInstance.nodeValue = newText; } +const supportsMoveBefore = + // $FlowFixMe[prop-missing]: We're doing the feature detection here. + enableMoveBefore && + typeof window !== 'undefined' && + typeof window.Node.prototype.moveBefore === 'function'; + export function appendChild( parentInstance: Instance, child: Instance | TextInstance, ): void { - parentInstance.appendChild(child); + if (supportsMoveBefore) { + // $FlowFixMe[prop-missing]: We've checked this with supportsMoveBefore. + parentInstance.moveBefore(child, null); + } else { + parentInstance.appendChild(child); + } } export function appendChildToContainer( @@ -799,7 +812,12 @@ export function insertBefore( child: Instance | TextInstance, beforeChild: Instance | TextInstance | SuspenseInstance, ): void { - parentInstance.insertBefore(child, beforeChild); + if (supportsMoveBefore) { + // $FlowFixMe[prop-missing]: We've checked this with supportsMoveBefore. + parentInstance.moveBefore(child, beforeChild); + } else { + parentInstance.insertBefore(child, beforeChild); + } } export function insertInContainerBefore( diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 997e9461059fa..e82bce7b50cbe 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -210,6 +210,9 @@ export const disableIEWorkarounds = true; // request for certain browsers. export const enableFilterEmptyStringAttributesDOM = true; +// Enable the moveBefore() alternative to insertBefore(). This preserves states of moves. +export const enableMoveBefore = false; + // Disabled caching behavior of `react/cache` in client runtimes. export const disableClientCache = true; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 748aa03a9d7ae..defa7cd330d7f 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -55,6 +55,7 @@ export const enableDebugTracing = false; export const enableDeferRootSchedulingToMicrotask = true; export const enableDO_NOT_USE_disableStrictPassiveEffect = false; export const enableFilterEmptyStringAttributesDOM = true; +export const enableMoveBefore = true; export const enableFizzExternalRuntime = true; export const enableFlightReadableStream = true; export const enableGetInspectorDataForInstanceInProduction = true; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 9f48f3877f81d..16c156f41c5ac 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -44,6 +44,7 @@ export const enableDeferRootSchedulingToMicrotask = true; export const enableDO_NOT_USE_disableStrictPassiveEffect = false; export const enableFabricCompleteRootInCommitPhase = false; export const enableFilterEmptyStringAttributesDOM = true; +export const enableMoveBefore = true; export const enableFizzExternalRuntime = true; export const enableFlightReadableStream = true; export const enableGetInspectorDataForInstanceInProduction = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index fbb95e0829018..01462919600d2 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -45,6 +45,7 @@ export const favorSafetyOverHydrationPerf = true; export const enableComponentStackLocations = true; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = true; +export const enableMoveBefore = false; export const enableGetInspectorDataForInstanceInProduction = false; export const enableFabricCompleteRootInCommitPhase = false; export const enableHiddenSubtreeInsertionEffectCleanup = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index 8bc77e4bfe375..51d9b90d4f7be 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -35,6 +35,7 @@ export const enableDebugTracing = false; export const enableDeferRootSchedulingToMicrotask = true; export const enableDO_NOT_USE_disableStrictPassiveEffect = false; export const enableFilterEmptyStringAttributesDOM = true; +export const enableMoveBefore = false; export const enableFizzExternalRuntime = true; export const enableFlightReadableStream = true; export const enableGetInspectorDataForInstanceInProduction = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index e28627760a756..272886cc58d07 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -47,6 +47,7 @@ export const favorSafetyOverHydrationPerf = true; export const enableComponentStackLocations = true; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = true; +export const enableMoveBefore = false; export const enableGetInspectorDataForInstanceInProduction = false; export const enableRenderableContext = false; export const enableFabricCompleteRootInCommitPhase = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 67d1ad74c0494..8ddbf3dd7c519 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -57,6 +57,7 @@ export const enableCPUSuspense = true; export const enableUseMemoCacheHook = true; export const enableUseEffectEventHook = true; export const enableFilterEmptyStringAttributesDOM = true; +export const enableMoveBefore = false; export const enableAsyncActions = true; export const disableInputAttributeSyncing = false; export const enableLegacyFBSupport = true;