From bbf15c865e52c8289ca711151a550cca002a992b Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 4 Apr 2025 10:06:42 -0400 Subject: [PATCH 01/10] Add feature flag --- packages/shared/ReactFeatureFlags.js | 2 ++ packages/shared/forks/ReactFeatureFlags.native-fb.js | 1 + packages/shared/forks/ReactFeatureFlags.native-oss.js | 1 + packages/shared/forks/ReactFeatureFlags.test-renderer.js | 1 + .../shared/forks/ReactFeatureFlags.test-renderer.native-fb.js | 1 + packages/shared/forks/ReactFeatureFlags.test-renderer.www.js | 1 + packages/shared/forks/ReactFeatureFlags.www.js | 2 ++ 7 files changed, 9 insertions(+) diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 106460b7c08fd..ebd15d1ae138a 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -96,6 +96,8 @@ export const enableGestureTransition = __EXPERIMENTAL__; export const enableScrollEndPolyfill = __EXPERIMENTAL__; +export const enableSuspenseyImages = __EXPERIMENTAL__; + /** * Switches the Fabric API from doing layout in commit work instead of complete work. */ diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 0f34a6364fa6e..25a8e1f5115d1 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -82,6 +82,7 @@ export const enableThrottledScheduling = false; export const enableViewTransition = false; export const enableGestureTransition = false; export const enableScrollEndPolyfill = true; +export const enableSuspenseyImages = false; export const enableFragmentRefs = false; export const ownerStackLimit = 1e4; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 33d97eca70405..caf0f67c32b66 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -74,6 +74,7 @@ export const enableGestureTransition = false; export const enableFastAddPropertiesInDiffing = false; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; +export const enableSuspenseyImages = false; export const ownerStackLimit = 1e4; export const enableFragmentRefs = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 0162b75a7ae87..3b4916a22c8bd 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -73,6 +73,7 @@ export const enableGestureTransition = false; export const enableFastAddPropertiesInDiffing = true; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; +export const enableSuspenseyImages = false; export const ownerStackLimit = 1e4; export const enableFragmentRefs = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index 3809542ddfd9c..33ff8a8acc211 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -70,6 +70,7 @@ export const enableGestureTransition = false; export const enableFastAddPropertiesInDiffing = false; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; +export const enableSuspenseyImages = false; export const enableFragmentRefs = false; export const ownerStackLimit = 1e4; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 422c6ce471693..cde11ce03c4d1 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -84,6 +84,7 @@ export const enableGestureTransition = false; export const enableFastAddPropertiesInDiffing = false; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; +export const enableSuspenseyImages = false; export const enableFragmentRefs = false; export const ownerStackLimit = 1e4; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 23ac44b37c6a6..62da5ecf77e00 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -113,6 +113,8 @@ export const enableLazyPublicInstanceInFabric = false; export const enableGestureTransition = false; +export const enableSuspenseyImages = false; + export const ownerStackLimit = 1e4; // Flow magic to verify the exports of this file match the original version. From 97f003f68a50381753b1e065e7fcddf6443ec260 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 4 Apr 2025 10:39:25 -0400 Subject: [PATCH 02/10] Detect if an image may suspend on commit --- .../src/client/ReactFiberConfigDOM.js | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 8b7b552238caf..8c62343850508 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -103,6 +103,7 @@ import { disableLegacyMode, enableMoveBefore, disableCommentsAsDOMContainers, + enableSuspenseyImages, } from 'shared/ReactFeatureFlags'; import { HostComponent, @@ -145,6 +146,9 @@ export type Props = { is?: string, size?: number, multiple?: boolean, + src?: string, + loading?: 'eager' | 'lazy', + onLoad?: (event: any) => void, ... }; type RawProps = { @@ -4974,7 +4978,18 @@ export function isHostHoistableType( } export function maySuspendCommit(type: Type, props: Props): boolean { - return false; + if (!enableSuspenseyImages) { + return false; + } + // Suspensey images are the default, unless you opt-out of with either + // loading="lazy" or onLoad={...} which implies you're ok waiting. + return ( + type === 'img' && + props.src != null && + props.src !== '' && + props.onLoad == null && + props.loading !== 'lazy' + ); } export function mayResourceSuspendCommit(resource: Resource): boolean { @@ -4985,7 +5000,13 @@ export function mayResourceSuspendCommit(resource: Resource): boolean { } export function preloadInstance(type: Type, props: Props): boolean { - return true; + // We don't need to preload Suspensey images because the browser will + // load them early once we set the src. + // We indicate that all images are not yet loaded and if they're able + // to hit cache we let the decode() do that. Even if we did maintain + // our own cache to know this, it's not a guarantee that the browser + // keeps it in decoded memory. + return false; } export function preloadResource(resource: Resource): boolean { From 93eae2ab9f27809d0b0f70928dd348efcdaef785 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 4 Apr 2025 10:43:48 -0400 Subject: [PATCH 03/10] Add maySuspendCommitOnUpdate Config for optimizations Since we no longer diff all props in the render phase we might still get here when nothing has changed. So this adds a small optimization to avoid resuspending on every update. --- packages/react-art/src/ReactFiberConfigART.js | 4 ++++ .../src/client/ReactFiberConfigDOM.js | 16 ++++++++++++++-- .../src/ReactFiberConfigFabric.js | 8 ++++++++ .../src/ReactFiberConfigNative.js | 8 ++++++++ .../react-noop-renderer/src/createReactNoop.js | 12 ++++++++++++ .../src/ReactFiberCompleteWork.js | 18 ++++++++++++++---- .../ReactFiberHostContext-test.internal.js | 3 +++ .../src/forks/ReactFiberConfig.custom.js | 1 + .../src/ReactFiberConfigTestHost.js | 8 ++++++++ 9 files changed, 72 insertions(+), 6 deletions(-) diff --git a/packages/react-art/src/ReactFiberConfigART.js b/packages/react-art/src/ReactFiberConfigART.js index db4e8015df131..981b51d027736 100644 --- a/packages/react-art/src/ReactFiberConfigART.js +++ b/packages/react-art/src/ReactFiberConfigART.js @@ -596,6 +596,10 @@ export function maySuspendCommit(type, props) { return false; } +export function maySuspendCommitOnUpdate(type, oldProps, newProps) { + return false; +} + export function preloadInstance(type, props) { // Return true to indicate it's already loaded return true; diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 8c62343850508..07dac4a70db12 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -147,6 +147,7 @@ export type Props = { size?: number, multiple?: boolean, src?: string, + srcSet?: string, loading?: 'eager' | 'lazy', onLoad?: (event: any) => void, ... @@ -773,9 +774,9 @@ export function commitMount( // only need to assign one. And Safari just never triggers a new load event which means this technique // is already a noop regardless of which properties are assigned. We should revisit if browsers update // this heuristic in the future. - if ((newProps: any).src) { + if (newProps.src) { ((domElement: any): HTMLImageElement).src = (newProps: any).src; - } else if ((newProps: any).srcSet) { + } else if (newProps.srcSet) { ((domElement: any): HTMLImageElement).srcset = (newProps: any).srcSet; } return; @@ -4992,6 +4993,17 @@ export function maySuspendCommit(type: Type, props: Props): boolean { ); } +export function maySuspendCommitOnUpdate( + type: Type, + oldProps: Props, + newProps: Props, +): boolean { + return ( + maySuspendCommit(type, newProps) && + (newProps.src !== oldProps.src || newProps.srcSet !== oldProps.srcSet) + ); +} + export function mayResourceSuspendCommit(resource: Resource): boolean { return ( resource.type === 'stylesheet' && diff --git a/packages/react-native-renderer/src/ReactFiberConfigFabric.js b/packages/react-native-renderer/src/ReactFiberConfigFabric.js index c3b8ac4d35d14..d947deb790955 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigFabric.js +++ b/packages/react-native-renderer/src/ReactFiberConfigFabric.js @@ -577,6 +577,14 @@ export function maySuspendCommit(type: Type, props: Props): boolean { return false; } +export function maySuspendCommitOnUpdate( + type: Type, + oldProps: Props, + newProps: Props, +): boolean { + return false; +} + export function preloadInstance(type: Type, props: Props): boolean { return true; } diff --git a/packages/react-native-renderer/src/ReactFiberConfigNative.js b/packages/react-native-renderer/src/ReactFiberConfigNative.js index b022d82d4e6c7..a1e73b242d124 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigNative.js +++ b/packages/react-native-renderer/src/ReactFiberConfigNative.js @@ -735,6 +735,14 @@ export function maySuspendCommit(type: Type, props: Props): boolean { return false; } +export function maySuspendCommitOnUpdate( + type: Type, + oldProps: Props, + newProps: Props, +): boolean { + return false; +} + export function preloadInstance(type: Type, props: Props): boolean { // Return false to indicate it's already loaded return true; diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 57d86c6df5a9d..4acda7c170cd1 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -624,6 +624,18 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { return type === 'suspensey-thing' && typeof props.src === 'string'; }, + maySuspendCommitOnUpdate( + type: string, + oldProps: Props, + newProps: Props, + ): boolean { + // Asks whether it's possible for this combination of type and props + // to ever need to suspend. This is different from asking whether it's + // currently ready because even if it's ready now, it might get purged + // from the cache later. + return type === 'suspensey-thing' && typeof newProps.src === 'string'; + }, + mayResourceSuspendCommit(resource: mixed): boolean { throw new Error( 'Resources are not implemented for React Noop yet. This method should not be called', diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 663523dc952de..f349b72448ae3 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -116,6 +116,7 @@ import { preparePortalMount, prepareScopeUpdate, maySuspendCommit, + maySuspendCommitOnUpdate, mayResourceSuspendCommit, preloadInstance, preloadResource, @@ -547,10 +548,16 @@ function updateHostComponent( function preloadInstanceAndSuspendIfNeeded( workInProgress: Fiber, type: Type, - props: Props, + oldProps: null | Props, + newProps: Props, renderLanes: Lanes, ) { - if (!maySuspendCommit(type, props)) { + const maySuspend = + oldProps === null + ? maySuspendCommit(type, newProps) + : maySuspendCommitOnUpdate(type, oldProps, newProps); + + if (!maySuspend) { // If this flag was set previously, we can remove it. The flag // represents whether this particular set of props might ever need to // suspend. The safest thing to do is for maySuspendCommit to always @@ -571,7 +578,7 @@ function preloadInstanceAndSuspendIfNeeded( // preload the instance if necessary. Even if this is an urgent render there // could be benefits to preloading early. // @TODO we should probably do the preload in begin work - const isReady = preloadInstance(type, props); + const isReady = preloadInstance(type, newProps); if (!isReady) { if (shouldRemainOnPreviousScreen()) { workInProgress.flags |= ShouldSuspendCommit; @@ -1104,6 +1111,7 @@ function completeWork( preloadInstanceAndSuspendIfNeeded( workInProgress, type, + null, newProps, renderLanes, ); @@ -1137,10 +1145,10 @@ function completeWork( return null; } } else { + const oldProps = current.memoizedProps; // This is an Instance // We may have props to update on the Hoistable instance. if (supportsMutation) { - const oldProps = current.memoizedProps; if (oldProps !== newProps) { markUpdate(workInProgress); } @@ -1160,6 +1168,7 @@ function completeWork( preloadInstanceAndSuspendIfNeeded( workInProgress, type, + oldProps, newProps, renderLanes, ); @@ -1323,6 +1332,7 @@ function completeWork( preloadInstanceAndSuspendIfNeeded( workInProgress, workInProgress.type, + current === null ? null : current.memoizedProps, workInProgress.pendingProps, renderLanes, ); diff --git a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js index 7ceba3439f6ce..c5de3f5aebf80 100644 --- a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js @@ -97,6 +97,9 @@ describe('ReactFiberHostContext', () => { maySuspendCommit(type, props) { return false; }, + maySuspendCommitOnUpdate(type, oldProps, newProps) { + return false; + }, preloadInstance(type, props) { return true; }, diff --git a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js index 050b1650c3b69..de4eeef432716 100644 --- a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js @@ -88,6 +88,7 @@ export const shouldAttemptEagerTransition = export const detachDeletedInstance = $$$config.detachDeletedInstance; export const requestPostPaintCallback = $$$config.requestPostPaintCallback; export const maySuspendCommit = $$$config.maySuspendCommit; +export const maySuspendCommitOnUpdate = $$$config.maySuspendCommitOnUpdate; export const preloadInstance = $$$config.preloadInstance; export const startSuspendingCommit = $$$config.startSuspendingCommit; export const suspendInstance = $$$config.suspendInstance; diff --git a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js index c0ebf59601817..537f5ee78286c 100644 --- a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js +++ b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js @@ -537,6 +537,14 @@ export function maySuspendCommit(type: Type, props: Props): boolean { return false; } +export function maySuspendCommitOnUpdate( + type: Type, + oldProps: Props, + newProps: Props, +): boolean { + return false; +} + export function preloadInstance(type: Type, props: Props): boolean { // Return true to indicate it's already loaded return true; From bf17cc90016eb2155c304287c2b6c0708fb3a461 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 4 Apr 2025 11:18:38 -0400 Subject: [PATCH 04/10] Pass Instance to suspendInstance --- packages/react-art/src/ReactFiberConfigART.js | 2 +- .../react-dom-bindings/src/client/ReactFiberConfigDOM.js | 8 +++++--- .../react-native-renderer/src/ReactFiberConfigFabric.js | 6 +++++- .../react-native-renderer/src/ReactFiberConfigNative.js | 6 +++++- packages/react-noop-renderer/src/createReactNoop.js | 6 +++++- packages/react-reconciler/src/ReactFiberCommitWork.js | 6 ++++-- .../src/__tests__/ReactFiberHostContext-test.internal.js | 2 +- .../react-test-renderer/src/ReactFiberConfigTestHost.js | 6 +++++- 8 files changed, 31 insertions(+), 11 deletions(-) diff --git a/packages/react-art/src/ReactFiberConfigART.js b/packages/react-art/src/ReactFiberConfigART.js index 981b51d027736..dd0dd963ea5c5 100644 --- a/packages/react-art/src/ReactFiberConfigART.js +++ b/packages/react-art/src/ReactFiberConfigART.js @@ -607,7 +607,7 @@ export function preloadInstance(type, props) { export function startSuspendingCommit() {} -export function suspendInstance(type, props) {} +export function suspendInstance(instance, type, props) {} export function suspendOnActiveViewTransition(container) {} diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 07dac4a70db12..a973058d9289a 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -5055,9 +5055,11 @@ export function startSuspendingCommit(): void { }; } -export function suspendInstance(type: Type, props: Props): void { - return; -} +export function suspendInstance( + instance: Instance, + type: Type, + props: Props, +): void {} export function suspendResource( hoistableRoot: HoistableRoot, diff --git a/packages/react-native-renderer/src/ReactFiberConfigFabric.js b/packages/react-native-renderer/src/ReactFiberConfigFabric.js index d947deb790955..2bb59b2c76519 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigFabric.js +++ b/packages/react-native-renderer/src/ReactFiberConfigFabric.js @@ -591,7 +591,11 @@ export function preloadInstance(type: Type, props: Props): boolean { export function startSuspendingCommit(): void {} -export function suspendInstance(type: Type, props: Props): void {} +export function suspendInstance( + instance: Instance, + type: Type, + props: Props, +): void {} export function suspendOnActiveViewTransition(container: Container): void {} diff --git a/packages/react-native-renderer/src/ReactFiberConfigNative.js b/packages/react-native-renderer/src/ReactFiberConfigNative.js index a1e73b242d124..df6fc638e55cb 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigNative.js +++ b/packages/react-native-renderer/src/ReactFiberConfigNative.js @@ -750,7 +750,11 @@ export function preloadInstance(type: Type, props: Props): boolean { export function startSuspendingCommit(): void {} -export function suspendInstance(type: Type, props: Props): void {} +export function suspendInstance( + instance: Instance, + type: Type, + props: Props, +): void {} export function suspendOnActiveViewTransition(container: Container): void {} diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 4acda7c170cd1..8884d1b9a6416 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -320,7 +320,11 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { suspenseyCommitSubscription = null; } - function suspendInstance(type: string, props: Props): void { + function suspendInstance( + instance: Instance, + type: string, + props: Props, + ): void { const src = props.src; if (type === 'suspensey-thing' && typeof src === 'string') { // Attach a listener to the suspensey thing and create a subscription diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 3d6d05dbec9aa..1720122618e3a 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -4308,9 +4308,10 @@ function accumulateSuspenseyCommitOnFiber(fiber: Fiber) { fiber.memoizedProps, ); } else { + const instance = fiber.stateNode; const type = fiber.type; const props = fiber.memoizedProps; - suspendInstance(type, props); + suspendInstance(instance, type, props); } } break; @@ -4318,9 +4319,10 @@ function accumulateSuspenseyCommitOnFiber(fiber: Fiber) { case HostComponent: { recursivelyAccumulateSuspenseyCommit(fiber); if (fiber.flags & suspenseyCommitFlag) { + const instance = fiber.stateNode; const type = fiber.type; const props = fiber.memoizedProps; - suspendInstance(type, props); + suspendInstance(instance, type, props); } break; } diff --git a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js index c5de3f5aebf80..ba6013c873517 100644 --- a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js @@ -104,7 +104,7 @@ describe('ReactFiberHostContext', () => { return true; }, startSuspendingCommit() {}, - suspendInstance(type, props) {}, + suspendInstance(instance, type, props) {}, suspendOnActiveViewTransition(container) {}, waitForCommitToBeReady() { return null; diff --git a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js index 537f5ee78286c..e5e8742c46c6d 100644 --- a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js +++ b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js @@ -552,7 +552,11 @@ export function preloadInstance(type: Type, props: Props): boolean { export function startSuspendingCommit(): void {} -export function suspendInstance(type: Type, props: Props): void {} +export function suspendInstance( + instance: Instance, + type: Type, + props: Props, +): void {} export function suspendOnActiveViewTransition(container: Container): void {} From 9a72aedb8172947a5922953d3d75d8cd95e13e87 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 4 Apr 2025 11:36:00 -0400 Subject: [PATCH 05/10] Suspend the commit on loading and decoding the images Timeout at 500ms which is the same timeout we use for fonts in transitions but maybe we should wait less by default. --- .../src/client/ReactFiberConfigDOM.js | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index a973058d9289a..9ffd5a49444aa 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -5055,11 +5055,39 @@ export function startSuspendingCommit(): void { }; } +const SUSPENSEY_IMAGE_TIMEOUT = 500; + export function suspendInstance( instance: Instance, type: Type, props: Props, -): void {} +): void { + if (!enableSuspenseyImages) { + return; + } + if (suspendedState === null) { + throw new Error( + 'Internal React Error: suspendedState null when it was expected to exists. Please report this as a React bug.', + ); + } + const state = suspendedState; + if ( + // $FlowFixMe[prop-missing] + typeof instance.decode === 'function' && + typeof setTimeout === 'function' + ) { + // If this browser supports decode() API, we use it to suspend waiting on the image. + // The loading should have already started at this point, so it should be enough to + // just call decode() which should also wait for the data to finish loading. + state.count++; + const ping = onUnsuspend.bind(state); + Promise.race([ + // $FlowFixMe[prop-missing] + instance.decode(), + new Promise(resolve => setTimeout(resolve, SUSPENSEY_IMAGE_TIMEOUT)), + ]).then(ping, ping); + } +} export function suspendResource( hoistableRoot: HoistableRoot, From 495faf66c4980f3b6b8390181d6056cf68922c08 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 4 Apr 2025 11:50:09 -0400 Subject: [PATCH 06/10] Only suspend on images in Transitions, Retries, Gesture Transitions and Idle Meaning the blocking ones and Offscreen doesn't suspend on the images. We should allow an opt-in to suspend blocking too in the future. Resources still suspend blocking. --- .../src/ReactFiberCommitWork.js | 47 ++++++++++++------- .../src/ReactFiberCompleteWork.js | 22 +++++---- .../react-reconciler/src/ReactFiberLane.js | 8 ++++ .../src/ReactFiberWorkLoop.js | 2 +- 4 files changed, 53 insertions(+), 26 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 1720122618e3a..693dfee9ae4d1 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -18,7 +18,10 @@ import type { } from './ReactFiberConfig'; import type {Fiber, FiberRoot} from './ReactInternalTypes'; import type {Lanes} from './ReactFiberLane'; -import {includesOnlyViewTransitionEligibleLanes} from './ReactFiberLane'; +import { + includesOnlySuspenseyCommitEligibleLanes, + includesOnlyViewTransitionEligibleLanes, +} from './ReactFiberLane'; import type {SuspenseState, RetryQueue} from './ReactFiberSuspenseComponent'; import type {UpdateQueue} from './ReactFiberClassUpdateQueue'; import type {FunctionComponentUpdateQueue} from './ReactFiberHooks'; @@ -4280,25 +4283,31 @@ export function commitPassiveUnmountEffects(finishedWork: Fiber): void { // ViewTransitions so that we know to also visit those to collect appearing // pairs. let suspenseyCommitFlag = ShouldSuspendCommit; -export function accumulateSuspenseyCommit(finishedWork: Fiber): void { +export function accumulateSuspenseyCommit( + finishedWork: Fiber, + committedLanes: Lanes, +): void { resetAppearingViewTransitions(); - accumulateSuspenseyCommitOnFiber(finishedWork); + accumulateSuspenseyCommitOnFiber(finishedWork, committedLanes); } -function recursivelyAccumulateSuspenseyCommit(parentFiber: Fiber): void { +function recursivelyAccumulateSuspenseyCommit( + parentFiber: Fiber, + committedLanes: Lanes, +): void { if (parentFiber.subtreeFlags & suspenseyCommitFlag) { let child = parentFiber.child; while (child !== null) { - accumulateSuspenseyCommitOnFiber(child); + accumulateSuspenseyCommitOnFiber(child, committedLanes); child = child.sibling; } } } -function accumulateSuspenseyCommitOnFiber(fiber: Fiber) { +function accumulateSuspenseyCommitOnFiber(fiber: Fiber, committedLanes: Lanes) { switch (fiber.tag) { case HostHoistable: { - recursivelyAccumulateSuspenseyCommit(fiber); + recursivelyAccumulateSuspenseyCommit(fiber, committedLanes); if (fiber.flags & suspenseyCommitFlag) { if (fiber.memoizedState !== null) { suspendResource( @@ -4311,18 +4320,24 @@ function accumulateSuspenseyCommitOnFiber(fiber: Fiber) { const instance = fiber.stateNode; const type = fiber.type; const props = fiber.memoizedProps; - suspendInstance(instance, type, props); + // TODO: Allow sync lanes to suspend too with an opt-in. + if (includesOnlySuspenseyCommitEligibleLanes(committedLanes)) { + suspendInstance(instance, type, props); + } } } break; } case HostComponent: { - recursivelyAccumulateSuspenseyCommit(fiber); + recursivelyAccumulateSuspenseyCommit(fiber, committedLanes); if (fiber.flags & suspenseyCommitFlag) { const instance = fiber.stateNode; const type = fiber.type; const props = fiber.memoizedProps; - suspendInstance(instance, type, props); + // TODO: Allow sync lanes to suspend too with an opt-in. + if (includesOnlySuspenseyCommitEligibleLanes(committedLanes)) { + suspendInstance(instance, type, props); + } } break; } @@ -4333,10 +4348,10 @@ function accumulateSuspenseyCommitOnFiber(fiber: Fiber) { const container: Container = fiber.stateNode.containerInfo; currentHoistableRoot = getHoistableRoot(container); - recursivelyAccumulateSuspenseyCommit(fiber); + recursivelyAccumulateSuspenseyCommit(fiber, committedLanes); currentHoistableRoot = previousHoistableRoot; } else { - recursivelyAccumulateSuspenseyCommit(fiber); + recursivelyAccumulateSuspenseyCommit(fiber, committedLanes); } break; } @@ -4354,10 +4369,10 @@ function accumulateSuspenseyCommitOnFiber(fiber: Fiber) { // instances, even if they're in the current tree. const prevFlags = suspenseyCommitFlag; suspenseyCommitFlag = MaySuspendCommit; - recursivelyAccumulateSuspenseyCommit(fiber); + recursivelyAccumulateSuspenseyCommit(fiber, committedLanes); suspenseyCommitFlag = prevFlags; } else { - recursivelyAccumulateSuspenseyCommit(fiber); + recursivelyAccumulateSuspenseyCommit(fiber, committedLanes); } } break; @@ -4377,13 +4392,13 @@ function accumulateSuspenseyCommitOnFiber(fiber: Fiber) { trackAppearingViewTransition(name, state); } } - recursivelyAccumulateSuspenseyCommit(fiber); + recursivelyAccumulateSuspenseyCommit(fiber, committedLanes); break; } // Fallthrough } default: { - recursivelyAccumulateSuspenseyCommit(fiber); + recursivelyAccumulateSuspenseyCommit(fiber, committedLanes); } } } diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index f349b72448ae3..6e89ef1c28c16 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -168,6 +168,7 @@ import { includesSomeLane, mergeLanes, claimNextRetryLane, + includesOnlySuspenseyCommitEligibleLanes, } from './ReactFiberLane'; import {resetChildFibers} from './ReactChildFiber'; import {createScopeInstance} from './ReactFiberScope'; @@ -575,15 +576,18 @@ function preloadInstanceAndSuspendIfNeeded( // loaded yet. workInProgress.flags |= MaySuspendCommit; - // preload the instance if necessary. Even if this is an urgent render there - // could be benefits to preloading early. - // @TODO we should probably do the preload in begin work - const isReady = preloadInstance(type, newProps); - if (!isReady) { - if (shouldRemainOnPreviousScreen()) { - workInProgress.flags |= ShouldSuspendCommit; - } else { - suspendCommit(); + // TODO: Allow sync lanes to suspend too with an opt-in. + if (includesOnlySuspenseyCommitEligibleLanes(renderLanes)) { + // preload the instance if necessary. Even if this is an urgent render there + // could be benefits to preloading early. + // @TODO we should probably do the preload in begin work + const isReady = preloadInstance(type, newProps); + if (!isReady) { + if (shouldRemainOnPreviousScreen()) { + workInProgress.flags |= ShouldSuspendCommit; + } else { + suspendCommit(); + } } } } diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index eb65e69508cec..6548aeac3af38 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -637,6 +637,14 @@ export function includesOnlyViewTransitionEligibleLanes(lanes: Lanes): boolean { return (lanes & (TransitionLanes | RetryLanes | IdleLane)) === lanes; } +export function includesOnlySuspenseyCommitEligibleLanes( + lanes: Lanes, +): boolean { + return ( + (lanes & (TransitionLanes | RetryLanes | IdleLane | GestureLane)) === lanes + ); +} + export function includesBlockingLane(lanes: Lanes): boolean { const SyncDefaultLanes = InputContinuousHydrationLane | diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index e7d04d9f94ab9..6a54974d44489 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -1467,7 +1467,7 @@ function commitRootWhenReady( // transaction, so it track state in its own module scope. // This will also track any newly added or appearing ViewTransition // components for the purposes of forming pairs. - accumulateSuspenseyCommit(finishedWork); + accumulateSuspenseyCommit(finishedWork, lanes); if (isViewTransitionEligible || isGestureTransition) { // If we're stopping gestures we don't have to wait for any pending // view transition. We'll stop it when we commit. From fde8be85978926ed0cd3c70a313f2a6b89f65b5f Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 4 Apr 2025 12:07:15 -0400 Subject: [PATCH 07/10] Add fixture example --- fixtures/view-transition/src/components/Page.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fixtures/view-transition/src/components/Page.js b/fixtures/view-transition/src/components/Page.js index 3de87a949e5c7..d39b9bf45f9ca 100644 --- a/fixtures/view-transition/src/components/Page.js +++ b/fixtures/view-transition/src/components/Page.js @@ -41,6 +41,12 @@ function Component() { transitions['enter-slide-right'] + ' ' + transitions['exit-slide-left'] }>

Slide In from Left, Slide Out to Right

+

+ +

); } From 4b430b6cbbbb88881821e67b68ec8300cc80703d Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 4 Apr 2025 12:16:04 -0400 Subject: [PATCH 08/10] Update performance track message --- packages/react-reconciler/src/ReactFiberPerformanceTrack.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberPerformanceTrack.js b/packages/react-reconciler/src/ReactFiberPerformanceTrack.js index 9a59997521871..042c3fd927eff 100644 --- a/packages/react-reconciler/src/ReactFiberPerformanceTrack.js +++ b/packages/react-reconciler/src/ReactFiberPerformanceTrack.js @@ -645,9 +645,9 @@ export function logSuspendedCommitPhase( reusableLaneDevToolDetails.color = 'secondary-light'; reusableLaneOptions.start = startTime; reusableLaneOptions.end = endTime; - // TODO: Make this conditionally "Suspended on Images" or both when we add Suspensey Images. + // TODO: Include the exact reason and URLs of what resources suspended. // TODO: This might also be Suspended while waiting on a View Transition. - performance.measure('Suspended on CSS', reusableLaneOptions); + performance.measure('Suspended on CSS or Images', reusableLaneOptions); } } From 1127ba89c95d3b321b63d489b6ce4ba969654076 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 4 Apr 2025 12:55:52 -0400 Subject: [PATCH 09/10] Add Config for opting in to sync updates Since we have tests for this in noop. --- packages/react-art/src/ReactFiberConfigART.js | 4 ++++ .../src/client/ReactFiberConfigDOM.js | 8 ++++++++ .../src/ReactFiberConfigFabric.js | 7 +++++++ .../src/ReactFiberConfigNative.js | 7 +++++++ packages/react-noop-renderer/src/createReactNoop.js | 10 +++++++++- packages/react-reconciler/src/ReactFiberCommitWork.js | 11 +++++++++-- .../react-reconciler/src/ReactFiberCompleteWork.js | 7 +++++-- .../__tests__/ReactFiberHostContext-test.internal.js | 3 +++ .../src/forks/ReactFiberConfig.custom.js | 2 ++ .../src/ReactFiberConfigTestHost.js | 7 +++++++ 10 files changed, 61 insertions(+), 5 deletions(-) diff --git a/packages/react-art/src/ReactFiberConfigART.js b/packages/react-art/src/ReactFiberConfigART.js index dd0dd963ea5c5..be0f1210974de 100644 --- a/packages/react-art/src/ReactFiberConfigART.js +++ b/packages/react-art/src/ReactFiberConfigART.js @@ -600,6 +600,10 @@ export function maySuspendCommitOnUpdate(type, oldProps, newProps) { return false; } +export function maySuspendCommitInSyncRender(type, props) { + return false; +} + export function preloadInstance(type, props) { // Return true to indicate it's already loaded return true; diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 9ffd5a49444aa..2b696e9deca1a 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -5004,6 +5004,14 @@ export function maySuspendCommitOnUpdate( ); } +export function maySuspendCommitInSyncRender( + type: Type, + props: Props, +): boolean { + // TODO: Allow sync lanes to suspend too with an opt-in. + return false; +} + export function mayResourceSuspendCommit(resource: Resource): boolean { return ( resource.type === 'stylesheet' && diff --git a/packages/react-native-renderer/src/ReactFiberConfigFabric.js b/packages/react-native-renderer/src/ReactFiberConfigFabric.js index 2bb59b2c76519..0ea3d909318c1 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigFabric.js +++ b/packages/react-native-renderer/src/ReactFiberConfigFabric.js @@ -585,6 +585,13 @@ export function maySuspendCommitOnUpdate( return false; } +export function maySuspendCommitInSyncRender( + type: Type, + props: Props, +): boolean { + return false; +} + export function preloadInstance(type: Type, props: Props): boolean { return true; } diff --git a/packages/react-native-renderer/src/ReactFiberConfigNative.js b/packages/react-native-renderer/src/ReactFiberConfigNative.js index df6fc638e55cb..91e9c496b5b82 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigNative.js +++ b/packages/react-native-renderer/src/ReactFiberConfigNative.js @@ -743,6 +743,13 @@ export function maySuspendCommitOnUpdate( return false; } +export function maySuspendCommitInSyncRender( + type: Type, + props: Props, +): boolean { + return false; +} + export function preloadInstance(type: Type, props: Props): boolean { // Return false to indicate it's already loaded return true; diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 8884d1b9a6416..d4eed084b756b 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -637,7 +637,15 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { // to ever need to suspend. This is different from asking whether it's // currently ready because even if it's ready now, it might get purged // from the cache later. - return type === 'suspensey-thing' && typeof newProps.src === 'string'; + return ( + type === 'suspensey-thing' && + typeof newProps.src === 'string' && + newProps.src !== oldProps.src + ); + }, + + maySuspendCommitInSyncRender(type: string, props: Props): boolean { + return true; }, mayResourceSuspendCommit(resource: mixed): boolean { diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 693dfee9ae4d1..01a6a30f3116d 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -163,6 +163,7 @@ import { mountHoistable, unmountHoistable, prepareToCommitHoistables, + maySuspendCommitInSyncRender, suspendInstance, suspendResource, resetFormInstance, @@ -4321,7 +4322,10 @@ function accumulateSuspenseyCommitOnFiber(fiber: Fiber, committedLanes: Lanes) { const type = fiber.type; const props = fiber.memoizedProps; // TODO: Allow sync lanes to suspend too with an opt-in. - if (includesOnlySuspenseyCommitEligibleLanes(committedLanes)) { + if ( + includesOnlySuspenseyCommitEligibleLanes(committedLanes) || + maySuspendCommitInSyncRender(type, props) + ) { suspendInstance(instance, type, props); } } @@ -4335,7 +4339,10 @@ function accumulateSuspenseyCommitOnFiber(fiber: Fiber, committedLanes: Lanes) { const type = fiber.type; const props = fiber.memoizedProps; // TODO: Allow sync lanes to suspend too with an opt-in. - if (includesOnlySuspenseyCommitEligibleLanes(committedLanes)) { + if ( + includesOnlySuspenseyCommitEligibleLanes(committedLanes) || + maySuspendCommitInSyncRender(type, props) + ) { suspendInstance(instance, type, props); } } diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 6e89ef1c28c16..846646c89f871 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -117,6 +117,7 @@ import { prepareScopeUpdate, maySuspendCommit, maySuspendCommitOnUpdate, + maySuspendCommitInSyncRender, mayResourceSuspendCommit, preloadInstance, preloadResource, @@ -576,8 +577,10 @@ function preloadInstanceAndSuspendIfNeeded( // loaded yet. workInProgress.flags |= MaySuspendCommit; - // TODO: Allow sync lanes to suspend too with an opt-in. - if (includesOnlySuspenseyCommitEligibleLanes(renderLanes)) { + if ( + includesOnlySuspenseyCommitEligibleLanes(renderLanes) || + maySuspendCommitInSyncRender(type, newProps) + ) { // preload the instance if necessary. Even if this is an urgent render there // could be benefits to preloading early. // @TODO we should probably do the preload in begin work diff --git a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js index ba6013c873517..b62f58431059e 100644 --- a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js @@ -100,6 +100,9 @@ describe('ReactFiberHostContext', () => { maySuspendCommitOnUpdate(type, oldProps, newProps) { return false; }, + maySuspendCommitInSyncRender(type, props) { + return false; + }, preloadInstance(type, props) { return true; }, diff --git a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js index de4eeef432716..f890a78a80e71 100644 --- a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js @@ -89,6 +89,8 @@ export const detachDeletedInstance = $$$config.detachDeletedInstance; export const requestPostPaintCallback = $$$config.requestPostPaintCallback; export const maySuspendCommit = $$$config.maySuspendCommit; export const maySuspendCommitOnUpdate = $$$config.maySuspendCommitOnUpdate; +export const maySuspendCommitInSyncRender = + $$$config.maySuspendCommitInSyncRender; export const preloadInstance = $$$config.preloadInstance; export const startSuspendingCommit = $$$config.startSuspendingCommit; export const suspendInstance = $$$config.suspendInstance; diff --git a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js index e5e8742c46c6d..3bc980bd638f8 100644 --- a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js +++ b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js @@ -545,6 +545,13 @@ export function maySuspendCommitOnUpdate( return false; } +export function maySuspendCommitInSyncRender( + type: Type, + props: Props, +): boolean { + return false; +} + export function preloadInstance(type: Type, props: Props): boolean { // Return true to indicate it's already loaded return true; From 21eed10cdde48aa79cba4403ce83110de90cdd66 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 4 Apr 2025 14:46:17 -0400 Subject: [PATCH 10/10] Avoid triggering Suspense fallback if the DOM node indicates it is complete We will still visit this in the commit phase to decode() it in case it's not decoded already. --- .../src/client/ReactFiberConfigDOM.js | 15 +++++++++------ .../src/ReactFiberConfigFabric.js | 6 +++++- .../src/ReactFiberConfigNative.js | 6 +++++- .../react-noop-renderer/src/createReactNoop.js | 2 +- .../src/ReactFiberCompleteWork.js | 7 ++++++- .../react-reconciler/src/ReactFiberWorkLoop.js | 2 +- .../ReactFiberHostContext-test.internal.js | 2 +- .../src/ReactFiberConfigTestHost.js | 6 +++++- 8 files changed, 33 insertions(+), 13 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 2b696e9deca1a..23056953aeb1d 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -5019,14 +5019,17 @@ export function mayResourceSuspendCommit(resource: Resource): boolean { ); } -export function preloadInstance(type: Type, props: Props): boolean { +export function preloadInstance( + instance: Instance, + type: Type, + props: Props, +): boolean { // We don't need to preload Suspensey images because the browser will // load them early once we set the src. - // We indicate that all images are not yet loaded and if they're able - // to hit cache we let the decode() do that. Even if we did maintain - // our own cache to know this, it's not a guarantee that the browser - // keeps it in decoded memory. - return false; + // If we return true here, we'll still get a suspendInstance call in the + // pre-commit phase to determine if we still need to decode the image or + // if was dropped from cache. This just avoids rendering Suspense fallback. + return !!(instance: any).complete; } export function preloadResource(resource: Resource): boolean { diff --git a/packages/react-native-renderer/src/ReactFiberConfigFabric.js b/packages/react-native-renderer/src/ReactFiberConfigFabric.js index 0ea3d909318c1..d0ddc5f7f0dc0 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigFabric.js +++ b/packages/react-native-renderer/src/ReactFiberConfigFabric.js @@ -592,7 +592,11 @@ export function maySuspendCommitInSyncRender( return false; } -export function preloadInstance(type: Type, props: Props): boolean { +export function preloadInstance( + instance: Instance, + type: Type, + props: Props, +): boolean { return true; } diff --git a/packages/react-native-renderer/src/ReactFiberConfigNative.js b/packages/react-native-renderer/src/ReactFiberConfigNative.js index 91e9c496b5b82..6ff7eb3b91a3b 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigNative.js +++ b/packages/react-native-renderer/src/ReactFiberConfigNative.js @@ -750,7 +750,11 @@ export function maySuspendCommitInSyncRender( return false; } -export function preloadInstance(type: Type, props: Props): boolean { +export function preloadInstance( + instance: Instance, + type: Type, + props: Props, +): boolean { // Return false to indicate it's already loaded return true; } diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index d4eed084b756b..fc3c2fd3cde24 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -654,7 +654,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { ); }, - preloadInstance(type: string, props: Props): boolean { + preloadInstance(instance: Instance, type: string, props: Props): boolean { if (type !== 'suspensey-thing' || typeof props.src !== 'string') { throw new Error('Attempted to preload unexpected instance: ' + type); } diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 846646c89f871..037c1d3bef42d 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -584,13 +584,18 @@ function preloadInstanceAndSuspendIfNeeded( // preload the instance if necessary. Even if this is an urgent render there // could be benefits to preloading early. // @TODO we should probably do the preload in begin work - const isReady = preloadInstance(type, newProps); + const isReady = preloadInstance(workInProgress.stateNode, type, newProps); if (!isReady) { if (shouldRemainOnPreviousScreen()) { workInProgress.flags |= ShouldSuspendCommit; } else { suspendCommit(); } + } else { + // Even if we're ready we suspend the commit and check again in the pre-commit + // phase if we need to suspend anyway. Such as if it's delayed on decoding or + // if it was dropped from the cache while rendering due to pressure. + workInProgress.flags |= ShouldSuspendCommit; } } } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 6a54974d44489..ca1c59e85b782 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -2638,7 +2638,7 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { const props = hostFiber.pendingProps; const isReady = resource ? preloadResource(resource) - : preloadInstance(type, props); + : preloadInstance(hostFiber.stateNode, type, props); if (isReady) { // The data resolved. Resume the work loop as if nothing // suspended. Unlike when a user component suspends, we don't diff --git a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js index b62f58431059e..2351f7ed01b8f 100644 --- a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js @@ -103,7 +103,7 @@ describe('ReactFiberHostContext', () => { maySuspendCommitInSyncRender(type, props) { return false; }, - preloadInstance(type, props) { + preloadInstance(instance, type, props) { return true; }, startSuspendingCommit() {}, diff --git a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js index 3bc980bd638f8..05fa9dffa3608 100644 --- a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js +++ b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js @@ -552,7 +552,11 @@ export function maySuspendCommitInSyncRender( return false; } -export function preloadInstance(type: Type, props: Props): boolean { +export function preloadInstance( + instance: Instance, + type: Type, + props: Props, +): boolean { // Return true to indicate it's already loaded return true; }