Skip to content

Reconcile element types of lazy component yielding the same type #20357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 1, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 64 additions & 50 deletions packages/react-reconciler/src/ReactChildFiber.new.js
Original file line number Diff line number Diff line change
@@ -246,6 +246,12 @@ function warnOnFunctionType(returnFiber: Fiber) {
}
}

function resolveLazy(lazyType) {
const payload = lazyType._payload;
const init = lazyType._init;
return init(payload);
}

// This wrapper function exists because I expect to clone the code in each path
// to be able to optimize each path individually by branching early. This needs
// a compiler or we can do it manually. Helpers that don't need this branching
@@ -383,11 +389,32 @@ function ChildReconciler(shouldTrackSideEffects) {
element: ReactElement,
lanes: Lanes,
): Fiber {
const elementType = element.type;
if (elementType === REACT_FRAGMENT_TYPE) {
return updateFragment(
returnFiber,
current,
element.props.children,
lanes,
element.key,
);
}
if (current !== null) {
if (
current.elementType === element.type ||
current.elementType === elementType ||
// Keep this check inline so it only runs on the false path:
(__DEV__ ? isCompatibleFamilyForHotReloading(current, element) : false)
(__DEV__
? isCompatibleFamilyForHotReloading(current, element)
: false) ||
// Lazy types should reconcile their resolved type.
// We need to do this after the Hot Reloading check above,
// because hot reloading has different semantics than prod because
// it doesn't resuspend. So we can't let the call below suspend.
(enableLazyElements &&
typeof elementType === 'object' &&
elementType !== null &&
elementType.$$typeof === REACT_LAZY_TYPE &&
resolveLazy(elementType) === current.type)
) {
// Move based on index
const existing = useFiber(current, element.props);
@@ -551,15 +578,6 @@ function ChildReconciler(shouldTrackSideEffects) {
switch (newChild.$$typeof) {
case REACT_ELEMENT_TYPE: {
if (newChild.key === key) {
if (newChild.type === REACT_FRAGMENT_TYPE) {
return updateFragment(
returnFiber,
oldFiber,
newChild.props.children,
lanes,
key,
);
}
return updateElement(returnFiber, oldFiber, newChild, lanes);
} else {
return null;
@@ -622,15 +640,6 @@ function ChildReconciler(shouldTrackSideEffects) {
existingChildren.get(
newChild.key === null ? newIdx : newChild.key,
) || null;
if (newChild.type === REACT_FRAGMENT_TYPE) {
return updateFragment(
returnFiber,
matchedFiber,
newChild.props.children,
lanes,
newChild.key,
);
}
return updateElement(returnFiber, matchedFiber, newChild, lanes);
}
case REACT_PORTAL_TYPE: {
@@ -1101,39 +1110,44 @@ function ChildReconciler(shouldTrackSideEffects) {
// TODO: If key === null and child.key === null, then this only applies to
// the first item in the list.
if (child.key === key) {
switch (child.tag) {
case Fragment: {
if (element.type === REACT_FRAGMENT_TYPE) {
deleteRemainingChildren(returnFiber, child.sibling);
const existing = useFiber(child, element.props.children);
existing.return = returnFiber;
if (__DEV__) {
existing._debugSource = element._source;
existing._debugOwner = element._owner;
}
return existing;
const elementType = element.type;
if (elementType === REACT_FRAGMENT_TYPE) {
if (child.tag === Fragment) {
deleteRemainingChildren(returnFiber, child.sibling);
const existing = useFiber(child, element.props.children);
existing.return = returnFiber;
if (__DEV__) {
existing._debugSource = element._source;
existing._debugOwner = element._owner;
}
break;
return existing;
}
default: {
if (
child.elementType === element.type ||
// Keep this check inline so it only runs on the false path:
(__DEV__
? isCompatibleFamilyForHotReloading(child, element)
: false)
) {
deleteRemainingChildren(returnFiber, child.sibling);
const existing = useFiber(child, element.props);
existing.ref = coerceRef(returnFiber, child, element);
existing.return = returnFiber;
if (__DEV__) {
existing._debugSource = element._source;
existing._debugOwner = element._owner;
}
return existing;
} else {
if (
child.elementType === elementType ||
// Keep this check inline so it only runs on the false path:
(__DEV__
? isCompatibleFamilyForHotReloading(child, element)
: false) ||
// Lazy types should reconcile their resolved type.
// We need to do this after the Hot Reloading check above,
// because hot reloading has different semantics than prod because
// it doesn't resuspend. So we can't let the call below suspend.
(enableLazyElements &&
typeof elementType === 'object' &&
elementType !== null &&
elementType.$$typeof === REACT_LAZY_TYPE &&
resolveLazy(elementType) === child.type)
) {
deleteRemainingChildren(returnFiber, child.sibling);
const existing = useFiber(child, element.props);
existing.ref = coerceRef(returnFiber, child, element);
existing.return = returnFiber;
if (__DEV__) {
existing._debugSource = element._source;
existing._debugOwner = element._owner;
}
break;
return existing;
}
}
// Didn't match.
113 changes: 63 additions & 50 deletions packages/react-reconciler/src/ReactChildFiber.old.js
Original file line number Diff line number Diff line change
@@ -246,6 +246,12 @@ function warnOnFunctionType(returnFiber: Fiber) {
}
}

function resolveLazy(lazyType) {
const payload = lazyType._payload;
const init = lazyType._init;
return init(payload);
}

// This wrapper function exists because I expect to clone the code in each path
// to be able to optimize each path individually by branching early. This needs
// a compiler or we can do it manually. Helpers that don't need this branching
@@ -383,11 +389,32 @@ function ChildReconciler(shouldTrackSideEffects) {
element: ReactElement,
lanes: Lanes,
): Fiber {
const elementType = element.type;
if (elementType === REACT_FRAGMENT_TYPE) {
return updateFragment(
returnFiber,
current,
element.props.children,
lanes,
element.key,
);
}
if (current !== null) {
if (
current.elementType === element.type ||
current.elementType === elementType ||
// Keep this check inline so it only runs on the false path:
(__DEV__ ? isCompatibleFamilyForHotReloading(current, element) : false)
(__DEV__
? isCompatibleFamilyForHotReloading(current, element)
: false) ||
// Lazy types should reconcile their resolved type.
// We need to do this after the Hot Reloading check above,
// because hot reloading has different semantics than prod because
// it doesn't resuspend. So we can't let the call below suspend.
(enableLazyElements &&
typeof elementType === 'object' &&
elementType !== null &&
elementType.$$typeof === REACT_LAZY_TYPE &&
resolveLazy(elementType) === current.type)
) {
// Move based on index
const existing = useFiber(current, element.props);
@@ -551,15 +578,6 @@ function ChildReconciler(shouldTrackSideEffects) {
switch (newChild.$$typeof) {
case REACT_ELEMENT_TYPE: {
if (newChild.key === key) {
if (newChild.type === REACT_FRAGMENT_TYPE) {
return updateFragment(
returnFiber,
oldFiber,
newChild.props.children,
lanes,
key,
);
}
return updateElement(returnFiber, oldFiber, newChild, lanes);
} else {
return null;
@@ -622,15 +640,6 @@ function ChildReconciler(shouldTrackSideEffects) {
existingChildren.get(
newChild.key === null ? newIdx : newChild.key,
) || null;
if (newChild.type === REACT_FRAGMENT_TYPE) {
return updateFragment(
returnFiber,
matchedFiber,
newChild.props.children,
lanes,
newChild.key,
);
}
return updateElement(returnFiber, matchedFiber, newChild, lanes);
}
case REACT_PORTAL_TYPE: {
@@ -1101,39 +1110,43 @@ function ChildReconciler(shouldTrackSideEffects) {
// TODO: If key === null and child.key === null, then this only applies to
// the first item in the list.
if (child.key === key) {
switch (child.tag) {
case Fragment: {
if (element.type === REACT_FRAGMENT_TYPE) {
deleteRemainingChildren(returnFiber, child.sibling);
const existing = useFiber(child, element.props.children);
existing.return = returnFiber;
if (__DEV__) {
existing._debugSource = element._source;
existing._debugOwner = element._owner;
}
return existing;
const elementType = element.type;
if (elementType === REACT_FRAGMENT_TYPE) {
if (child.tag === Fragment) {
deleteRemainingChildren(returnFiber, child.sibling);
const existing = useFiber(child, element.props.children);
existing.return = returnFiber;
if (__DEV__) {
existing._debugSource = element._source;
existing._debugOwner = element._owner;
}
break;
return existing;
}
default: {
if (
child.elementType === element.type ||
// Keep this check inline so it only runs on the false path:
(__DEV__
? isCompatibleFamilyForHotReloading(child, element)
: false)
) {
deleteRemainingChildren(returnFiber, child.sibling);
const existing = useFiber(child, element.props);
existing.ref = coerceRef(returnFiber, child, element);
existing.return = returnFiber;
if (__DEV__) {
existing._debugSource = element._source;
existing._debugOwner = element._owner;
}
return existing;
} else {
if (
child.elementType === elementType ||
(__DEV__
? isCompatibleFamilyForHotReloading(child, element)
: false) ||
// Lazy types should reconcile their resolved type.
// We need to do this after the Hot Reloading check above,
// because hot reloading has different semantics than prod because
// it doesn't resuspend. So we can't let the call below suspend.
(enableLazyElements &&
typeof elementType === 'object' &&
elementType !== null &&
elementType.$$typeof === REACT_LAZY_TYPE &&
resolveLazy(elementType) === child.type)
) {
deleteRemainingChildren(returnFiber, child.sibling);
const existing = useFiber(child, element.props);
existing.ref = coerceRef(returnFiber, child, element);
existing.return = returnFiber;
if (__DEV__) {
existing._debugSource = element._source;
existing._debugOwner = element._owner;
}
break;
return existing;
}
}
// Didn't match.
10 changes: 9 additions & 1 deletion packages/react-reconciler/src/ReactFiberThrow.new.js
Original file line number Diff line number Diff line change
@@ -256,7 +256,15 @@ function throwException(
// Note: It doesn't matter whether the component that suspended was
// inside a blocking mode tree. If the Suspense is outside of it, we
// should *not* suspend the commit.
if ((workInProgress.mode & BlockingMode) === NoMode) {
//
// If the suspense boundary suspended itself suspended, we don't have to
// do this trick because nothing was partially started. We can just
// directly do a second pass over the fallback in this render and
// pretend we meant to render that directly.
if (
(workInProgress.mode & BlockingMode) === NoMode &&
workInProgress !== returnFiber
Copy link
Collaborator Author

@sebmarkbage sebmarkbage Dec 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acdlite 👆

Fix infinite loop in legacy mode

In legacy mode we typically commit the suspending fiber and then rerender
the nearest boundary to render the fallback in a separate commit.

We can't do that when the boundary itself suspends because when we try to
do the second pass, it'll suspend again and infinite loop.

Interestingly the legacy semantics are not needed in this case because
they exist to let an existing partial render fully commit its partial state.

In this case there's no partial state, so we can just render the fallback
immediately instead.

) {
workInProgress.flags |= DidCapture;
sourceFiber.flags |= ForceUpdateForLegacySuspense;

10 changes: 9 additions & 1 deletion packages/react-reconciler/src/ReactFiberThrow.old.js
Original file line number Diff line number Diff line change
@@ -256,7 +256,15 @@ function throwException(
// Note: It doesn't matter whether the component that suspended was
// inside a blocking mode tree. If the Suspense is outside of it, we
// should *not* suspend the commit.
if ((workInProgress.mode & BlockingMode) === NoMode) {
//
// If the suspense boundary suspended itself suspended, we don't have to
// do this trick because nothing was partially started. We can just
// directly do a second pass over the fallback in this render and
// pretend we meant to render that directly.
if (
(workInProgress.mode & BlockingMode) === NoMode &&
workInProgress !== returnFiber
) {
workInProgress.flags |= DidCapture;
sourceFiber.flags |= ForceUpdateForLegacySuspense;

262 changes: 262 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js
Original file line number Diff line number Diff line change
@@ -1268,6 +1268,192 @@ describe('ReactLazy', () => {
expect(componentStackMessage).toContain('in Lazy');
});

// @gate enableLazyElements
it('mount and reorder lazy types', async () => {
class Child extends React.Component {
componentDidMount() {
Scheduler.unstable_yieldValue('Did mount: ' + this.props.label);
}
componentDidUpdate() {
Scheduler.unstable_yieldValue('Did update: ' + this.props.label);
}
render() {
return <Text text={this.props.label} />;
}
}

function ChildA({lowerCase}) {
return <Child label={lowerCase ? 'a' : 'A'} />;
}

function ChildB({lowerCase}) {
return <Child label={lowerCase ? 'b' : 'B'} />;
}

const LazyChildA = lazy(() => {
Scheduler.unstable_yieldValue('Init A');
return fakeImport(ChildA);
});
const LazyChildB = lazy(() => {
Scheduler.unstable_yieldValue('Init B');
return fakeImport(ChildB);
});
const LazyChildA2 = lazy(() => {
Scheduler.unstable_yieldValue('Init A2');
return fakeImport(ChildA);
});
let resolveB2;
const LazyChildB2 = lazy(() => {
Scheduler.unstable_yieldValue('Init B2');
return new Promise(r => {
resolveB2 = r;
});
});

function Parent({swap}) {
return (
<Suspense fallback={<Text text="Outer..." />}>
<Suspense fallback={<Text text="Loading..." />}>
{swap
? [
<LazyChildB2 key="B" lowerCase={true} />,
<LazyChildA2 key="A" lowerCase={true} />,
]
: [<LazyChildA key="A" />, <LazyChildB key="B" />]}
</Suspense>
</Suspense>
);
}

const root = ReactTestRenderer.create(<Parent swap={false} />, {
unstable_isConcurrent: true,
});

expect(Scheduler).toFlushAndYield(['Init A', 'Init B', 'Loading...']);
expect(root).not.toMatchRenderedOutput('AB');

await LazyChildA;
await LazyChildB;

expect(Scheduler).toFlushAndYield([
'A',
'B',
'Did mount: A',
'Did mount: B',
]);
expect(root).toMatchRenderedOutput('AB');

// Swap the position of A and B
root.update(<Parent swap={true} />);
expect(Scheduler).toFlushAndYield(['Init B2', 'Loading...']);
jest.runAllTimers();

// The suspense boundary should've triggered now.
expect(root).toMatchRenderedOutput('Loading...');
await resolveB2({default: ChildB});

// We need to flush to trigger the second one to load.
expect(Scheduler).toFlushAndYield(['Init A2']);
await LazyChildA2;

expect(Scheduler).toFlushAndYield([
'b',
'a',
'Did update: b',
'Did update: a',
]);
expect(root).toMatchRenderedOutput('ba');
});

// @gate enableLazyElements
it('mount and reorder lazy types (legacy mode)', async () => {
class Child extends React.Component {
componentDidMount() {
Scheduler.unstable_yieldValue('Did mount: ' + this.props.label);
}
componentDidUpdate() {
Scheduler.unstable_yieldValue('Did update: ' + this.props.label);
}
render() {
return <Text text={this.props.label} />;
}
}

function ChildA({lowerCase}) {
return <Child label={lowerCase ? 'a' : 'A'} />;
}

function ChildB({lowerCase}) {
return <Child label={lowerCase ? 'b' : 'B'} />;
}

const LazyChildA = lazy(() => {
Scheduler.unstable_yieldValue('Init A');
return fakeImport(ChildA);
});
const LazyChildB = lazy(() => {
Scheduler.unstable_yieldValue('Init B');
return fakeImport(ChildB);
});
const LazyChildA2 = lazy(() => {
Scheduler.unstable_yieldValue('Init A2');
return fakeImport(ChildA);
});
const LazyChildB2 = lazy(() => {
Scheduler.unstable_yieldValue('Init B2');
return fakeImport(ChildB);
});

function Parent({swap}) {
return (
<Suspense fallback={<Text text="Outer..." />}>
<Suspense fallback={<Text text="Loading..." />}>
{swap
? [
<LazyChildB2 key="B" lowerCase={true} />,
<LazyChildA2 key="A" lowerCase={true} />,
]
: [<LazyChildA key="A" />, <LazyChildB key="B" />]}
</Suspense>
</Suspense>
);
}

const root = ReactTestRenderer.create(<Parent swap={false} />, {
unstable_isConcurrent: false,
});

expect(Scheduler).toHaveYielded(['Init A', 'Init B', 'Loading...']);
expect(root).not.toMatchRenderedOutput('AB');

await LazyChildA;
await LazyChildB;

expect(Scheduler).toFlushAndYield([
'A',
'B',
'Did mount: A',
'Did mount: B',
]);
expect(root).toMatchRenderedOutput('AB');

// Swap the position of A and B
root.update(<Parent swap={true} />);
expect(Scheduler).toHaveYielded(['Init B2', 'Loading...']);
await LazyChildB2;
// We need to flush to trigger the second one to load.
expect(Scheduler).toFlushAndYield(['Init A2']);
await LazyChildA2;

expect(Scheduler).toFlushAndYield([
'b',
'a',
'Did update: b',
'Did update: a',
]);
expect(root).toMatchRenderedOutput('ba');
});

// @gate enableLazyElements
it('mount and reorder lazy elements', async () => {
class Child extends React.Component {
@@ -1343,4 +1529,80 @@ describe('ReactLazy', () => {
]);
expect(root).toMatchRenderedOutput('ba');
});

// @gate enableLazyElements
it('mount and reorder lazy elements (legacy mode)', async () => {
class Child extends React.Component {
componentDidMount() {
Scheduler.unstable_yieldValue('Did mount: ' + this.props.label);
}
componentDidUpdate() {
Scheduler.unstable_yieldValue('Did update: ' + this.props.label);
}
render() {
return <Text text={this.props.label} />;
}
}

const lazyChildA = lazy(() => {
Scheduler.unstable_yieldValue('Init A');
return fakeImport(<Child key="A" label="A" />);
});
const lazyChildB = lazy(() => {
Scheduler.unstable_yieldValue('Init B');
return fakeImport(<Child key="B" label="B" />);
});
const lazyChildA2 = lazy(() => {
Scheduler.unstable_yieldValue('Init A2');
return fakeImport(<Child key="A" label="a" />);
});
const lazyChildB2 = lazy(() => {
Scheduler.unstable_yieldValue('Init B2');
return fakeImport(<Child key="B" label="b" />);
});

function Parent({swap}) {
return (
<Suspense fallback={<Text text="Loading..." />}>
{swap ? [lazyChildB2, lazyChildA2] : [lazyChildA, lazyChildB]}
</Suspense>
);
}

const root = ReactTestRenderer.create(<Parent swap={false} />, {
unstable_isConcurrent: false,
});

expect(Scheduler).toHaveYielded(['Init A', 'Loading...']);
expect(root).not.toMatchRenderedOutput('AB');

await lazyChildA;
// We need to flush to trigger the B to load.
expect(Scheduler).toFlushAndYield(['Init B']);
await lazyChildB;

expect(Scheduler).toFlushAndYield([
'A',
'B',
'Did mount: A',
'Did mount: B',
]);
expect(root).toMatchRenderedOutput('AB');

// Swap the position of A and B
root.update(<Parent swap={true} />);
expect(Scheduler).toHaveYielded(['Init B2', 'Loading...']);
await lazyChildB2;
// We need to flush to trigger the second one to load.
expect(Scheduler).toFlushAndYield(['Init A2']);
await lazyChildA2;

expect(Scheduler).toFlushAndYield([
'b',
'a',
'Did update: b',
'Did update: a',
]);
expect(root).toMatchRenderedOutput('ba');
});
});