Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fixes a bug where a lazy component does not cache the result of
its constructor.
  • Loading branch information
Andrew Clark authored and linjiajian999 committed Oct 22, 2018
1 parent 1616d90 commit 9fa5d93
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 15 deletions.
21 changes: 14 additions & 7 deletions packages/react-reconciler/src/ReactFiberLazyComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,27 @@
* @flow
*/

import type {LazyComponent} from 'shared/ReactLazyComponent';
import type {LazyComponent, Thenable} from 'shared/ReactLazyComponent';

import {Resolved, Rejected, Pending} from 'shared/ReactLazyComponent';
import warning from 'shared/warning';

export function readLazyComponentType<T>(lazyComponent: LazyComponent<T>): T {
const status = lazyComponent._status;
const result = lazyComponent._result;
switch (status) {
case Resolved:
const Component: T = lazyComponent._result;
case Resolved: {
const Component: T = result;
return Component;
case Rejected:
throw lazyComponent._result;
case Pending:
throw lazyComponent;
}
case Rejected: {
const error: mixed = result;
throw error;
}
case Pending: {
const thenable: Thenable<T, mixed> = result;
throw thenable;
}
default: {
lazyComponent._status = Pending;
const ctor = lazyComponent._ctor;
Expand Down Expand Up @@ -52,6 +58,7 @@ export function readLazyComponentType<T>(lazyComponent: LazyComponent<T>): T {
}
},
);
lazyComponent._result = thenable;
throw thenable;
}
}
Expand Down
60 changes: 52 additions & 8 deletions packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ describe('ReactLazy', () => {
return props.text;
}

function delay(ms) {
return new Promise(resolve => setTimeout(() => resolve(), ms));
}

async function fakeImport(result) {
return {default: result};
}
Expand All @@ -40,7 +44,7 @@ describe('ReactLazy', () => {
expect(root).toFlushAndYield(['Loading...']);
expect(root).toMatchRenderedOutput(null);

await LazyText;
await Promise.resolve();

expect(root).toFlushAndYield(['Hi']);
expect(root).toMatchRenderedOutput('Hi');
Expand All @@ -55,6 +59,47 @@ describe('ReactLazy', () => {
expect(root).toMatchRenderedOutput('Hi again');
});

it('multiple lazy components', async () => {
function Foo() {
return <Text text="Foo" />;
}

function Bar() {
return <Text text="Bar" />;
}

const promiseForFoo = delay(1000).then(() => fakeImport(Foo));
const promiseForBar = delay(2000).then(() => fakeImport(Bar));

const LazyFoo = lazy(() => promiseForFoo);
const LazyBar = lazy(() => promiseForBar);

const root = ReactTestRenderer.create(
<Suspense fallback={<Text text="Loading..." />}>
<LazyFoo />
<LazyBar />
</Suspense>,
{
unstable_isConcurrent: true,
},
);

expect(root).toFlushAndYield(['Loading...']);
expect(root).toMatchRenderedOutput(null);

jest.advanceTimersByTime(1000);
await promiseForFoo;

expect(root).toFlushAndYield(['Foo', 'Loading...']);
expect(root).toMatchRenderedOutput(null);

jest.advanceTimersByTime(1000);
await promiseForBar;

expect(root).toFlushAndYield(['Foo', 'Bar']);
expect(root).toMatchRenderedOutput('FooBar');
});

it('does not support arbitrary promises, only module objects', async () => {
spyOnDev(console, 'error');

Expand All @@ -71,7 +116,7 @@ describe('ReactLazy', () => {
expect(root).toFlushAndYield(['Loading...']);
expect(root).toMatchRenderedOutput(null);

await LazyText;
await Promise.resolve();

if (__DEV__) {
expect(console.error).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -100,7 +145,7 @@ describe('ReactLazy', () => {
expect(root).toMatchRenderedOutput(null);

try {
await LazyText;
await Promise.resolve();
} catch (e) {}

expect(root).toFlushAndThrow('Bad network');
Expand Down Expand Up @@ -176,7 +221,7 @@ describe('ReactLazy', () => {
expect(root).toFlushAndYield(['Loading...']);
expect(root).toMatchRenderedOutput(null);

await LazyText;
await Promise.resolve();

expect(root).toFlushAndYield(['Hi']);
expect(root).toMatchRenderedOutput('Hi');
Expand Down Expand Up @@ -226,7 +271,7 @@ describe('ReactLazy', () => {
expect(root).toFlushAndYield(['Loading...']);
expect(root).toMatchRenderedOutput(null);

await Lazy;
await Promise.resolve();

expect(root).toFlushAndYield(['Lazy', 'Sibling', 'A']);
expect(root).toMatchRenderedOutput('SiblingA');
Expand Down Expand Up @@ -256,7 +301,7 @@ describe('ReactLazy', () => {
expect(root).toFlushAndYield(['Started loading', 'Loading...']);
expect(root).toMatchRenderedOutput(null);

await LazyFoo;
await Promise.resolve();

expect(() => {
expect(root).toFlushAndYield(['A', 'B']);
Expand Down Expand Up @@ -303,8 +348,7 @@ describe('ReactLazy', () => {
expect(root).toMatchRenderedOutput(null);
expect(ref.current).toBe(null);

await LazyClass;
await LazyForwardRef;
await Promise.resolve();

expect(root).toFlushAndYield(['Foo', 'forwardRef', 'Bar']);
expect(root).toMatchRenderedOutput('FooBar');
Expand Down

0 comments on commit 9fa5d93

Please sign in to comment.