From 1fc184d9a6ba6d0fca19b58ce14594259d0f0693 Mon Sep 17 00:00:00 2001 From: Luna Date: Fri, 5 Aug 2022 14:25:56 -0400 Subject: [PATCH] suspense boundary deleted --- .../src/ReactFiberCommitWork.new.js | 27 +- .../ReactFiberTracingMarkerComponent.new.js | 7 + .../src/ReactInternalTypes.js | 4 +- .../__tests__/ReactTransitionTracing-test.js | 420 +++++++++++++++++- 4 files changed, 445 insertions(+), 13 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 124667aa5eadb..734d2bfe0e625 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -2087,13 +2087,26 @@ function commitDeletionEffectsOnFiber( if (enableTransitionTracing) { // We need to mark this fiber's parents as deleted const instance: OffscreenInstance = deletedFiber.stateNode; - const markers = instance.pendingMarkers; - if (markers !== null) { - markers.forEach(marker => { - if (marker.pendingBoundaries.has(instance)) { - marker.pendingBoundaries.delete(instance); - } - }); + const transitions = instance.transitions; + if (transitions !== null) { + let name = null; + const parent = deletedFiber.return; + if ( + parent !== null && + parent.tag === SuspenseComponent && + parent.memoizedProps.unstable_name + ) { + name = parent.memoizedProps.unstable_name; + } + + recursivelyAbortParentMarkerTransitions( + deletedFiber, + nearestMountedAncestor, + { + type: 'suspense', + name, + }, + ); } } diff --git a/packages/react-reconciler/src/ReactFiberTracingMarkerComponent.new.js b/packages/react-reconciler/src/ReactFiberTracingMarkerComponent.new.js index 27b8d04492777..b17e70ed9bd50 100644 --- a/packages/react-reconciler/src/ReactFiberTracingMarkerComponent.new.js +++ b/packages/react-reconciler/src/ReactFiberTracingMarkerComponent.new.js @@ -169,6 +169,13 @@ function getFilteredDeletion(deletion: TransitionDeletion, endTime: number) { endTime, }; } + case 'suspense': { + return { + type: deletion.type, + name: deletion.name, + endTime, + }; + } default: { return null; } diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index 6a83f822c6a26..d5ed83d34a077 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -291,7 +291,7 @@ export type TransitionTracingCallbacks = { startTime: number, deletions: Array<{ type: string, - name?: string, + name?: string | null, endTime: number, }>, ) => void, @@ -313,7 +313,7 @@ export type TransitionTracingCallbacks = { startTime: number, deletions: Array<{ type: string, - name?: string, + name?: string | null, endTime: number, }>, ) => void, diff --git a/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js b/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js index 1089f0f7e0d0a..a6637b55977c9 100644 --- a/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js +++ b/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js @@ -1531,8 +1531,9 @@ describe('ReactInteractionTracing', () => { 'Loading...', 'Suspend [Sibling Text]', 'Sibling Loading...', - 'onMarkerIncomplete(transition one, marker one, 1000, [{endTime: 3000, name: marker one, type: marker}])', - 'onMarkerIncomplete(transition one, parent, 1000, [{endTime: 3000, name: marker one, type: marker}])', + 'onMarkerProgress(transition one, parent, 1000, 3000, [suspense sibling])', + 'onMarkerIncomplete(transition one, marker one, 1000, [{endTime: 3000, name: marker one, type: marker}, {endTime: 3000, name: suspense page, type: suspense}])', + 'onMarkerIncomplete(transition one, parent, 1000, [{endTime: 3000, name: marker one, type: marker}, {endTime: 3000, name: suspense page, type: suspense}])', ]); root.render(); @@ -1679,8 +1680,9 @@ describe('ReactInteractionTracing', () => { expect(Scheduler).toFlushAndYield([ 'Suspend [Page Two]', 'Loading Two...', - 'onMarkerIncomplete(transition, one, 1000, [{endTime: 3000, name: one, type: marker}])', - 'onMarkerIncomplete(transition, parent, 1000, [{endTime: 3000, name: one, type: marker}])', + 'onMarkerProgress(transition, parent, 1000, 3000, [suspense two])', + 'onMarkerIncomplete(transition, one, 1000, [{endTime: 3000, name: one, type: marker}, {endTime: 3000, name: suspense one, type: suspense}])', + 'onMarkerIncomplete(transition, parent, 1000, [{endTime: 3000, name: one, type: marker}, {endTime: 3000, name: suspense one, type: suspense}])', ]); await resolveText('Page Two'); @@ -1698,6 +1700,416 @@ describe('ReactInteractionTracing', () => { }); }); + // @gate enableTransitionTracing + it('Suspense boundary added by the transition is deleted', async () => { + const transitionCallbacks = { + onTransitionStart: (name, startTime) => { + Scheduler.unstable_yieldValue( + `onTransitionStart(${name}, ${startTime})`, + ); + }, + onTransitionProgress: (name, startTime, endTime, pending) => { + const suspenseNames = pending.map(p => p.name || '').join(', '); + Scheduler.unstable_yieldValue( + `onTransitionProgress(${name}, ${startTime}, ${endTime}, [${suspenseNames}])`, + ); + }, + onTransitionComplete: (name, startTime, endTime) => { + Scheduler.unstable_yieldValue( + `onTransitionComplete(${name}, ${startTime}, ${endTime})`, + ); + }, + onMarkerProgress: ( + transitioName, + markerName, + startTime, + currentTime, + pending, + ) => { + const suspenseNames = pending.map(p => p.name || '').join(', '); + Scheduler.unstable_yieldValue( + `onMarkerProgress(${transitioName}, ${markerName}, ${startTime}, ${currentTime}, [${suspenseNames}])`, + ); + }, + onMarkerIncomplete: ( + transitionName, + markerName, + startTime, + deletions, + ) => { + Scheduler.unstable_yieldValue( + `onMarkerIncomplete(${transitionName}, ${markerName}, ${startTime}, [${stringifyDeletions( + deletions, + )}])`, + ); + }, + onMarkerComplete: (transitioName, markerName, startTime, endTime) => { + Scheduler.unstable_yieldValue( + `onMarkerComplete(${transitioName}, ${markerName}, ${startTime}, ${endTime})`, + ); + }, + }; + + function App({navigate, deleteOne}) { + return ( +
+ {navigate ? ( + + + {!deleteOne ? ( + }> + + + }> + + + + + ) : null} + + + }> + + + + + ) : ( + + )} +
+ ); + } + const root = ReactNoop.createRoot({ + unstable_transitionCallbacks: transitionCallbacks, + }); + await act(async () => { + root.render(); + + ReactNoop.expire(1000); + await advanceTimers(1000); + expect(Scheduler).toFlushAndYield(['Page One']); + + startTransition( + () => root.render(), + { + name: 'transition', + }, + ); + ReactNoop.expire(1000); + await advanceTimers(1000); + expect(Scheduler).toFlushAndYield([ + 'Suspend [Page One]', + 'Suspend [Child]', + 'Loading Child...', + 'Loading One...', + 'Suspend [Page Two]', + 'Loading Two...', + 'onTransitionStart(transition, 1000)', + 'onMarkerProgress(transition, parent, 1000, 2000, [suspense one, suspense two])', + 'onMarkerProgress(transition, one, 1000, 2000, [suspense one])', + 'onMarkerProgress(transition, two, 1000, 2000, [suspense two])', + 'onTransitionProgress(transition, 1000, 2000, [suspense one, suspense two])', + ]); + + await resolveText('Page One'); + ReactNoop.expire(1000); + await advanceTimers(1000); + expect(Scheduler).toFlushAndYield([ + 'Page One', + 'Suspend [Child]', + 'Loading Child...', + 'onMarkerProgress(transition, parent, 1000, 3000, [suspense two, suspense child])', + 'onMarkerProgress(transition, one, 1000, 3000, [suspense child])', + 'onMarkerComplete(transition, page one, 1000, 3000)', + 'onTransitionProgress(transition, 1000, 3000, [suspense two, suspense child])', + ]); + + root.render(); + ReactNoop.expire(1000); + await advanceTimers(1000); + expect(Scheduler).toFlushAndYield([ + 'Suspend [Page Two]', + 'Loading Two...', + // "suspense one" has unsuspended so shouldn't be included + // tracing marker "page one" has completed so shouldn't be included + // all children of "suspense child" haven't yet been rendered so shouldn't be included + 'onMarkerProgress(transition, one, 1000, 4000, [])', + 'onMarkerProgress(transition, parent, 1000, 4000, [suspense two])', + 'onMarkerIncomplete(transition, one, 1000, [{endTime: 4000, name: suspense child, type: suspense}])', + 'onMarkerIncomplete(transition, parent, 1000, [{endTime: 4000, name: suspense child, type: suspense}])', + ]); + + await resolveText('Page Two'); + ReactNoop.expire(1000); + await advanceTimers(1000); + expect(Scheduler).toFlushAndYield([ + 'Page Two', + 'onMarkerProgress(transition, parent, 1000, 5000, [])', + 'onMarkerProgress(transition, two, 1000, 5000, [])', + 'onMarkerComplete(transition, two, 1000, 5000)', + 'onTransitionProgress(transition, 1000, 5000, [])', + ]); + }); + }); + + // @gate enableTransitionTracing + it('Suspense boundary not added by the transition is deleted ', async () => { + const transitionCallbacks = { + onTransitionStart: (name, startTime) => { + Scheduler.unstable_yieldValue( + `onTransitionStart(${name}, ${startTime})`, + ); + }, + onTransitionProgress: (name, startTime, endTime, pending) => { + const suspenseNames = pending.map(p => p.name || '').join(', '); + Scheduler.unstable_yieldValue( + `onTransitionProgress(${name}, ${startTime}, ${endTime}, [${suspenseNames}])`, + ); + }, + onTransitionComplete: (name, startTime, endTime) => { + Scheduler.unstable_yieldValue( + `onTransitionComplete(${name}, ${startTime}, ${endTime})`, + ); + }, + onMarkerProgress: ( + transitioName, + markerName, + startTime, + currentTime, + pending, + ) => { + const suspenseNames = pending.map(p => p.name || '').join(', '); + Scheduler.unstable_yieldValue( + `onMarkerProgress(${transitioName}, ${markerName}, ${startTime}, ${currentTime}, [${suspenseNames}])`, + ); + }, + onMarkerIncomplete: ( + transitionName, + markerName, + startTime, + deletions, + ) => { + Scheduler.unstable_yieldValue( + `onMarkerIncomplete(${transitionName}, ${markerName}, ${startTime}, [${stringifyDeletions( + deletions, + )}])`, + ); + }, + onMarkerComplete: (transitioName, markerName, startTime, endTime) => { + Scheduler.unstable_yieldValue( + `onMarkerComplete(${transitioName}, ${markerName}, ${startTime}, ${endTime})`, + ); + }, + }; + + function App({show}) { + return ( + + {show ? ( + + + + ) : null} + + + + + ); + } + + const root = ReactNoop.createRoot({ + unstable_transitionCallbacks: transitionCallbacks, + }); + await act(async () => { + startTransition(() => root.render(), { + name: 'transition', + }); + ReactNoop.expire(1000); + await advanceTimers(1000); + + expect(Scheduler).toFlushAndYield([ + 'Suspend [Child]', + 'onTransitionStart(transition, 0)', + 'onMarkerProgress(transition, parent, 0, 1000, [child])', + 'onTransitionProgress(transition, 0, 1000, [child])', + ]); + + root.render(); + ReactNoop.expire(1000); + await advanceTimers(1000); + // This appended child isn't part of the transition so we + // don't call any callback + expect(Scheduler).toFlushAndYield([ + 'Suspend [Appended child]', + 'Suspend [Child]', + ]); + + // This deleted child isn't part of the transition so we + // don't call any callbacks + root.render(); + ReactNoop.expire(1000); + await advanceTimers(1000); + expect(Scheduler).toFlushAndYield(['Suspend [Child]']); + + await resolveText('Child'); + ReactNoop.expire(1000); + await advanceTimers(1000); + + expect(Scheduler).toFlushAndYield([ + 'Child', + 'onMarkerProgress(transition, parent, 0, 4000, [])', + 'onMarkerComplete(transition, parent, 0, 4000)', + 'onTransitionProgress(transition, 0, 4000, [])', + 'onTransitionComplete(transition, 0, 4000)', + ]); + }); + }); + + // @gate enableTransitionTracing + it('marker incomplete gets called properly if child suspense marker is not part of it', async () => { + const transitionCallbacks = { + onTransitionStart: (name, startTime) => { + Scheduler.unstable_yieldValue( + `onTransitionStart(${name}, ${startTime})`, + ); + }, + onTransitionProgress: (name, startTime, endTime, pending) => { + const suspenseNames = pending.map(p => p.name || '').join(', '); + Scheduler.unstable_yieldValue( + `onTransitionProgress(${name}, ${startTime}, ${endTime}, [${suspenseNames}])`, + ); + }, + onTransitionComplete: (name, startTime, endTime) => { + Scheduler.unstable_yieldValue( + `onTransitionComplete(${name}, ${startTime}, ${endTime})`, + ); + }, + onMarkerProgress: ( + transitioName, + markerName, + startTime, + currentTime, + pending, + ) => { + const suspenseNames = pending.map(p => p.name || '').join(', '); + Scheduler.unstable_yieldValue( + `onMarkerProgress(${transitioName}, ${markerName}, ${startTime}, ${currentTime}, [${suspenseNames}])`, + ); + }, + onMarkerIncomplete: ( + transitionName, + markerName, + startTime, + deletions, + ) => { + Scheduler.unstable_yieldValue( + `onMarkerIncomplete(${transitionName}, ${markerName}, ${startTime}, [${stringifyDeletions( + deletions, + )}])`, + ); + }, + onMarkerComplete: (transitioName, markerName, startTime, endTime) => { + Scheduler.unstable_yieldValue( + `onMarkerComplete(${transitioName}, ${markerName}, ${startTime}, ${endTime})`, + ); + }, + }; + + function App({show, showSuspense}) { + return ( + + {show ? ( + + {showSuspense ? ( + + + + ) : null} + + ) : null} + + + + + ); + } + + const root = ReactNoop.createRoot({ + unstable_transitionCallbacks: transitionCallbacks, + }); + + await act(async () => { + startTransition( + () => root.render(), + { + name: 'transition one', + }, + ); + + ReactNoop.expire(1000); + await advanceTimers(1000); + }); + + expect(Scheduler).toHaveYielded([ + 'Suspend [Child]', + 'onTransitionStart(transition one, 0)', + 'onMarkerProgress(transition one, parent, 0, 1000, [child])', + 'onTransitionProgress(transition one, 0, 1000, [child])', + ]); + + await act(async () => { + startTransition( + () => root.render(), + { + name: 'transition two', + }, + ); + + ReactNoop.expire(1000); + await advanceTimers(1000); + }); + + expect(Scheduler).toHaveYielded([ + 'Suspend [Appended child]', + 'Suspend [Child]', + 'onTransitionStart(transition two, 1000)', + 'onMarkerProgress(transition two, appended child, 1000, 2000, [appended child])', + 'onTransitionProgress(transition two, 1000, 2000, [appended child])', + ]); + + await act(async () => { + root.render(); + ReactNoop.expire(1000); + await advanceTimers(1000); + }); + + expect(Scheduler).toHaveYielded([ + 'Suspend [Child]', + 'onMarkerProgress(transition two, appended child, 1000, 3000, [])', + 'onMarkerIncomplete(transition two, appended child, 1000, [{endTime: 3000, name: appended child, type: suspense}])', + ]); + + await act(async () => { + resolveText('Child'); + ReactNoop.expire(1000); + await advanceTimers(1000); + }); + + expect(Scheduler).toHaveYielded([ + 'Child', + 'onMarkerProgress(transition one, parent, 0, 4000, [])', + 'onMarkerComplete(transition one, parent, 0, 4000)', + 'onTransitionProgress(transition one, 0, 4000, [])', + 'onTransitionComplete(transition one, 0, 4000)', + ]); + }); + // @gate enableTransitionTracing it('warns when marker name changes', async () => { const transitionCallbacks = {