Skip to content

Commit

Permalink
React.lazy constructor must return result of a dynamic import
Browse files Browse the repository at this point in the history
We may want to change the protocol later, so until then we'll be
restrictive. Heuristic is to check for existence of `default`.
  • Loading branch information
acdlite committed Oct 19, 2018
1 parent 738f23f commit 20897b0
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 50 deletions.
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
30 changes: 16 additions & 14 deletions packages/react-reconciler/src/ReactFiberLazyComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import type {LazyComponentThenable} from 'shared/ReactLazyComponent';

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

import warning from 'shared/warning';

export function readLazyComponentType<T>(
thenable: LazyComponentThenable<T>,
): T {
Expand All @@ -26,22 +28,22 @@ export function readLazyComponentType<T>(
default: {
thenable._status = Pending;
thenable.then(
resolvedValue => {
moduleObject => {
if (thenable._status === Pending) {
thenable._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,
);
}
}
thenable._result = resolvedValue;
thenable._status = Resolved;
thenable._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, ref) {
return <Pure {...props} ref={ref} />;
}
return React.lazy(async () => React.forwardRef(Indirection));
return React.lazy(() => fakeImport(React.forwardRef(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

0 comments on commit 20897b0

Please sign in to comment.