Skip to content

Commit

Permalink
Add moveBefore Experiment (facebook#31596)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sebmarkbage authored Nov 22, 2024
1 parent 1345c37 commit aba370f
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 2 deletions.
22 changes: 20 additions & 2 deletions packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ import {
enableTrustedTypesIntegration,
enableAsyncActions,
disableLegacyMode,
enableMoveBefore,
} from 'shared/ReactFeatureFlags';
import {
HostComponent,
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
3 changes: 3 additions & 0 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit aba370f

Please sign in to comment.