From ee037cb3eb6c6ef873ed98044e67084d423c204a 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 | 35 +++ .../ReactFiberTracingMarkerComponent.new.js | 7 + .../__tests__/ReactTransitionTracing-test.js | 275 +++++++++++++++++- 3 files changed, 313 insertions(+), 4 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 0aa2fc71e2b24..bc0fe10c00b28 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -1998,6 +1998,33 @@ function commitDeletionEffectsOnFiber( const markers = instance.pendingMarkers; if (markers !== null) { markers.forEach(marker => { + if (marker.deletions === null) { + marker.deletions = []; + + if (marker.name !== null) { + addMarkerIncompleteCallbackToPendingTransition( + marker.name, + instance.transitions, + marker.deletions, + ); + } + } + + let name = null; + const parent = deletedFiber.return; + if ( + parent !== null && + parent.tag === SuspenseComponent && + parent.memoizedProps.unstable_name + ) { + name = parent.memoizedProps.unstable_name; + } + marker.deletions.push({ + type: 'suspense', + name, + transitions: instance.transitions, + }); + if (marker.pendingBoundaries.has(instance)) { marker.pendingBoundaries.delete(instance); } @@ -3057,6 +3084,14 @@ function commitOffscreenPassiveMountEffects( } commitTransitionProgress(finishedWork); + + if (!isHidden) { + instance.transitions = null; + instance.pendingMarkers = null; + instance.deletions = null; + instance.parents = null; + instance.name = null; + } } } diff --git a/packages/react-reconciler/src/ReactFiberTracingMarkerComponent.new.js b/packages/react-reconciler/src/ReactFiberTracingMarkerComponent.new.js index 2e811c9f1f6b2..ef7656e08fab2 100644 --- a/packages/react-reconciler/src/ReactFiberTracingMarkerComponent.new.js +++ b/packages/react-reconciler/src/ReactFiberTracingMarkerComponent.new.js @@ -170,6 +170,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/__tests__/ReactTransitionTracing-test.js b/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js index 1089f0f7e0d0a..d6ce4dec8d3b6 100644 --- a/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js +++ b/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js @@ -1531,8 +1531,8 @@ 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}])', + '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 +1679,8 @@ 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}])', + '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 +1698,273 @@ describe('ReactInteractionTracing', () => { }); }); + 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 + 'onMarkerIncomplete(transition, parent, 1000, [{endTime: 4000, name: suspense child, type: suspense}])', + 'onMarkerIncomplete(transition, one, 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, [])', + ]); + }); + }); + + 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('warns when marker name changes', async () => { const transitionCallbacks = {