Skip to content

Commit

Permalink
suspense boundary deleted
Browse files Browse the repository at this point in the history
  • Loading branch information
@lunaruan
lunaruan committed Aug 5, 2022
1 parent 5eb152f commit c7101a2
Show file tree
Hide file tree
Showing 3 changed files with 315 additions and 4 deletions.
35 changes: 35 additions & 0 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
}
}
}

Expand Down
7 changes: 7 additions & 0 deletions packages/react-reconciler/src/ReactFiberTracingMarkerComponent.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,13 @@ function getFilteredDeletion(deletion: TransitionDeletion, endTime: number) {
endTime,
};
}
case 'suspense': {
return {
type: deletion.type,
name: deletion.name,
endTime,
};
}
default: {
return null;
}
Expand Down
277 changes: 273 additions & 4 deletions packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(<App navigate={true} showMarker={true} />);
Expand Down Expand Up @@ -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');
Expand All @@ -1698,6 +1698,275 @@ 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 || '<null>').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 || '<null>').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 (
<div>
{navigate ? (
<React.unstable_TracingMarker name="parent">
<React.unstable_TracingMarker name="one">
{!deleteOne ? (
<Suspense
unstable_name="suspense one"
fallback={<Text text="Loading One..." />}>
<AsyncText text="Page One" />
<React.unstable_TracingMarker name="page one" />
<Suspense
unstable_name="suspense child"
fallback={<Text text="Loading Child..." />}>
<React.unstable_TracingMarker name="child" />
<AsyncText text="Child" />
</Suspense>
</Suspense>
) : null}
</React.unstable_TracingMarker>
<React.unstable_TracingMarker name="two">
<Suspense
unstable_name="suspense two"
fallback={<Text text="Loading Two..." />}>
<AsyncText text="Page Two" />
</Suspense>
</React.unstable_TracingMarker>
</React.unstable_TracingMarker>
) : (
<Text text="Page One" />
)}
</div>
);
}
const root = ReactNoop.createRoot({
unstable_transitionCallbacks: transitionCallbacks,
});
await act(async () => {
root.render(<App navigate={false} deleteOne={false} />);

ReactNoop.expire(1000);
await advanceTimers(1000);
expect(Scheduler).toFlushAndYield(['Page One']);

startTransition(
() => root.render(<App navigate={true} deleteOne={false} />),
{
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(<App navigate={true} deleteOne={true} />);
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, [])',
]);
});
});

// @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 || '<null>').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 || '<null>').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 (
<React.unstable_TracingMarker name="parent">
{show ? (
<Suspense unstable_name="appended child">
<AsyncText text="Appended child" />
</Suspense>
) : null}
<Suspense unstable_name="child">
<AsyncText text="Child" />
</Suspense>
</React.unstable_TracingMarker>
);
}

const root = ReactNoop.createRoot({
unstable_transitionCallbacks: transitionCallbacks,
});
await act(async () => {
startTransition(() => root.render(<App show={false} />), {
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(<App show={true} />);
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(<App show={false} />);
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 = {
Expand Down

0 comments on commit c7101a2

Please sign in to comment.