Skip to content

React.lazy constructor must return result of a dynamic import #13886

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 1 commit into from
Oct 19, 2018
Merged
Show file tree
Hide file tree
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
9 changes: 7 additions & 2 deletions packages/react-dom/src/__tests__/ReactServerRendering-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -571,14 +571,19 @@ describe('ReactDOMServer', () => {
ReactDOMServer.renderToString(<React.unstable_Suspense />);
}).toThrow('ReactDOMServer does not yet support Suspense.');

async function fakeImport(result) {
return {default: result};
}

expect(() => {
const LazyFoo = React.lazy(
() =>
const LazyFoo = React.lazy(() =>
fakeImport(
new Promise(resolve =>
resolve(function Foo() {
return <div />;
}),
),
),
);
ReactDOMServer.renderToString(<LazyFoo />);
}).toThrow('ReactDOMServer does not yet support lazy-loaded components.');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -444,14 +444,20 @@ describe('ReactDOMServerHydration', () => {
});

it('should be able to use lazy components after hydrating', async () => {
async function fakeImport(result) {
return {default: result};
}

const Lazy = React.lazy(
() =>
new Promise(resolve => {
setTimeout(
() =>
resolve(function World() {
return 'world';
}),
resolve(
fakeImport(function World() {
return 'world';
}),
),
1000,
);
}),
Expand Down
29 changes: 15 additions & 14 deletions packages/react-reconciler/src/ReactFiberLazyComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import type {LazyComponent} 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;
Expand All @@ -26,22 +27,22 @@ export function readLazyComponentType<T>(lazyComponent: LazyComponent<T>): T {
const ctor = lazyComponent._ctor;
const thenable = ctor();
thenable.then(
resolvedValue => {
moduleObject => {
if (lazyComponent._status === Pending) {
lazyComponent._status = Resolved;
if (typeof resolvedValue === 'object' && resolvedValue !== null) {
// If the `default` property is not empty, assume it's the result
// of an async import() and use that. Otherwise, use the
// resolved value itself.
const defaultExport = (resolvedValue: any).default;
resolvedValue =
defaultExport !== undefined && defaultExport !== null
? defaultExport
: resolvedValue;
} else {
resolvedValue = resolvedValue;
const defaultExport = moduleObject.default;
if (__DEV__) {
if (defaultExport === undefined) {
warning(
false,
'lazy: Expected the result of a dynamic import() call. ' +
'Instead received: %s\n\nYour code should look like: \n ' +
"const MyComponent = lazy(() => import('./MyComponent'))",
moduleObject,
);
}
}
lazyComponent._result = resolvedValue;
lazyComponent._status = Resolved;
lazyComponent._result = defaultExport;
}
},
error => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,10 @@ describe('ReactDebugFiberPerf', () => {
return <span />;
}

async function fakeImport(result) {
return {default: result};
}

let resolve;
const LazyFoo = React.lazy(
() =>
Expand All @@ -591,9 +595,11 @@ describe('ReactDebugFiberPerf', () => {
ReactNoop.flush();
expect(getFlameChart()).toMatchSnapshot();

resolve(function Foo() {
return <div />;
});
resolve(
fakeImport(function Foo() {
return <div />;
}),
);
await LazyFoo;

ReactNoop.render(
Expand Down
54 changes: 29 additions & 25 deletions packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,12 @@ describe('ReactLazy', () => {
return props.text;
}

async function fakeImport(result) {
return {default: result};
}

it('suspends until module has loaded', async () => {
const LazyText = lazy(async () => Text);
const LazyText = lazy(() => fakeImport(Text));

const root = ReactTestRenderer.create(
<Suspense fallback={<Text text="Loading..." />}>
Expand Down Expand Up @@ -51,8 +55,10 @@ describe('ReactLazy', () => {
expect(root).toMatchRenderedOutput('Hi again');
});

it('uses `default` property, if it exists', async () => {
const LazyText = lazy(async () => ({default: Text}));
it('does not support arbitrary promises, only module objects', async () => {
spyOnDev(console, 'error');

const LazyText = lazy(async () => Text);

const root = ReactTestRenderer.create(
<Suspense fallback={<Text text="Loading..." />}>
Expand All @@ -67,17 +73,13 @@ describe('ReactLazy', () => {

await LazyText;

expect(root).toFlushAndYield(['Hi']);
expect(root).toMatchRenderedOutput('Hi');

// Should not suspend on update
root.update(
<Suspense fallback={<Text text="Loading..." />}>
<LazyText text="Hi again" />
</Suspense>,
);
expect(root).toFlushAndYield(['Hi again']);
expect(root).toMatchRenderedOutput('Hi again');
if (__DEV__) {
expect(console.error).toHaveBeenCalledTimes(1);
expect(console.error.calls.argsFor(0)[0]).toContain(
'Expected the result of a dynamic import() call',
);
}
expect(root).toFlushAndThrow('Element type is invalid');
});

it('throws if promise rejects', async () => {
Expand Down Expand Up @@ -117,8 +119,8 @@ describe('ReactLazy', () => {
}
}

const LazyChildA = lazy(async () => Child);
const LazyChildB = lazy(async () => Child);
const LazyChildA = lazy(() => fakeImport(Child));
const LazyChildB = lazy(() => fakeImport(Child));

function Parent({swap}) {
return (
Expand Down Expand Up @@ -160,7 +162,7 @@ describe('ReactLazy', () => {
return <Text {...props} />;
}
T.defaultProps = {text: 'Hi'};
const LazyText = lazy(async () => T);
const LazyText = lazy(() => fakeImport(T));

const root = ReactTestRenderer.create(
<Suspense fallback={<Text text="Loading..." />}>
Expand Down Expand Up @@ -200,7 +202,7 @@ describe('ReactLazy', () => {
);
}
LazyImpl.defaultProps = {siblingText: 'Sibling'};
const Lazy = lazy(async () => LazyImpl);
const Lazy = lazy(() => fakeImport(LazyImpl));

class Stateful extends React.Component {
state = {text: 'A'};
Expand Down Expand Up @@ -239,7 +241,7 @@ describe('ReactLazy', () => {
const LazyFoo = lazy(() => {
ReactTestRenderer.unstable_yield('Started loading');
const Foo = props => <div>{[<Text text="A" />, <Text text="B" />]}</div>;
return Promise.resolve(Foo);
return fakeImport(Foo);
});

const root = ReactTestRenderer.create(
Expand All @@ -263,13 +265,13 @@ describe('ReactLazy', () => {
});

it('supports class and forwardRef components', async () => {
const LazyClass = lazy(async () => {
const LazyClass = lazy(() => {
class Foo extends React.Component {
render() {
return <Text text="Foo" />;
}
}
return Foo;
return fakeImport(Foo);
});

const LazyForwardRef = lazy(async () => {
Expand All @@ -278,10 +280,12 @@ describe('ReactLazy', () => {
return <Text text="Bar" />;
}
}
return React.forwardRef((props, ref) => {
ReactTestRenderer.unstable_yield('forwardRef');
return <Bar ref={ref} />;
});
return fakeImport(
React.forwardRef((props, ref) => {
ReactTestRenderer.unstable_yield('forwardRef');
return <Bar ref={ref} />;
}),
);
});

const ref = React.createRef();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ describe('pure', () => {
return <span prop={props.text} />;
}

async function fakeImport(result) {
return {default: result};
}

// Tests should run against both the lazy and non-lazy versions of `pure`.
// To make the tests work for both versions, we wrap the non-lazy version in
// a lazy function component.
Expand All @@ -42,11 +46,11 @@ describe('pure', () => {
function Indirection(props) {
return <Pure {...props} />;
}
return React.lazy(async () => Indirection);
return React.lazy(() => fakeImport(Indirection));
});
sharedTests('lazy', (...args) => {
const Pure = React.pure(...args);
return React.lazy(async () => Pure);
return React.lazy(() => fakeImport(Pure));
});

function sharedTests(label, pure) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,11 @@ describe('ReactSuspense', () => {
}
}

const LazyClass = React.lazy(() => Promise.resolve(Class));
async function fakeImport(result) {
return {default: result};
}

const LazyClass = React.lazy(() => fakeImport(Class));

const root = ReactTestRenderer.create(
<Suspense fallback={<Text text="Loading..." />}>
Expand Down
10 changes: 5 additions & 5 deletions packages/shared/ReactLazyComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ export type Thenable<T, R> = {

export type LazyComponent<T> = {
$$typeof: Symbol | number,
_ctor: () => Thenable<T, mixed>,
_ctor: () => Thenable<{default: T}, mixed>,
_status: 0 | 1 | 2,
_result: any,
};

type ResolvedLazyComponentThenable<T> = {
type ResolvedLazyComponent<T> = {
$$typeof: Symbol | number,
_ctor: () => Thenable<T, mixed>,
_ctor: () => Thenable<{default: T}, mixed>,
_status: 1,
_result: any,
};
Expand All @@ -30,13 +30,13 @@ export const Resolved = 1;
export const Rejected = 2;

export function getResultFromResolvedLazyComponent<T>(
lazyComponent: ResolvedLazyComponentThenable<T>,
lazyComponent: ResolvedLazyComponent<T>,
): T {
return lazyComponent._result;
}

export function refineResolvedLazyComponent<T>(
lazyComponent: LazyComponent<T>,
): ResolvedLazyComponentThenable<T> | null {
): ResolvedLazyComponent<T> | null {
return lazyComponent._status === Resolved ? lazyComponent._result : null;
}