React.lazy constructor must return result of a dynamic import (#13886)
We may want to change the protocol later, so until then we'll be restrictive. Heuristic is to check for existence of `default`.
Andrew Clark committed
Oct 18, 2018 at 19:58 UTC
d9a3cc070c3b554ce6c4b43b917c1ba8951f9b5f
8 files changed
+85
-55
packages/react-dom/src/__tests__/ReactServerRendering-test.js
+7
-2
@@ -571,14 +571,19 @@ describe('ReactDOMServer', () => {
571
ReactDOMServer.renderToString(<React.unstable_Suspense />);
572
}).toThrow('ReactDOMServer does not yet support Suspense.');
573
574
+ async function fakeImport(result) {
575
+ return {default: result};
576
+ }
577
+
578
expect(() => {
575
- const LazyFoo = React.lazy(
576
- () =>
579
+ const LazyFoo = React.lazy(() =>
580
+ fakeImport(
581
new Promise(resolve =>
582
resolve(function Foo() {
583
return <div />;
584
}),
585
),
586
+ ),
587
);
588
ReactDOMServer.renderToString(<LazyFoo />);
589
}).toThrow('ReactDOMServer does not yet support lazy-loaded components.');
packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js
+9
-3
@@ -444,14 +444,20 @@ describe('ReactDOMServerHydration', () => {
444
});
445
446
it('should be able to use lazy components after hydrating', async () => {
447
+ async function fakeImport(result) {
448
+ return {default: result};
449
+ }
450
+
451
const Lazy = React.lazy(
452
() =>
453
new Promise(resolve => {
454
setTimeout(
455
() =>
452
- resolve(function World() {
453
- return 'world';
454
- }),
456
+ resolve(
457
+ fakeImport(function World() {
458
+ return 'world';
459
+ }),
460
+ ),
461
1000,
462
);
463
}),
packages/react-reconciler/src/ReactFiberLazyComponent.js
+15
-14
@@ -10,6 +10,7 @@
10
import type {LazyComponent} from 'shared/ReactLazyComponent';
11
12
import {Resolved, Rejected, Pending} from 'shared/ReactLazyComponent';
13
+import warning from 'shared/warning';
14
15
export function readLazyComponentType<T>(lazyComponent: LazyComponent<T>): T {
16
const status = lazyComponent._status;
@@ -26,22 +27,22 @@ export function readLazyComponentType<T>(lazyComponent: LazyComponent<T>): T {
27
const ctor = lazyComponent._ctor;
28
const thenable = ctor();
29
thenable.then(
29
- resolvedValue => {
30
+ moduleObject => {
31
if (lazyComponent._status === Pending) {
31
- lazyComponent._status = Resolved;
32
- if (typeof resolvedValue === 'object' && resolvedValue !== null) {
33
- // If the `default` property is not empty, assume it's the result
34
- // of an async import() and use that. Otherwise, use the
35
- // resolved value itself.
36
- const defaultExport = (resolvedValue: any).default;
37
- resolvedValue =
38
- defaultExport !== undefined && defaultExport !== null
39
- ? defaultExport
40
- : resolvedValue;
41
- } else {
42
- resolvedValue = resolvedValue;
32
+ const defaultExport = moduleObject.default;
33
+ if (__DEV__) {
34
+ if (defaultExport === undefined) {
35
+ warning(
36
+ false,
37
+ 'lazy: Expected the result of a dynamic import() call. ' +
38
+ 'Instead received: %s\n\nYour code should look like: \n ' +
39
+ "const MyComponent = lazy(() => import('./MyComponent'))",
40
+ moduleObject,
41
+ );
42
+ }
43
}
44
- lazyComponent._result = resolvedValue;
44
+ lazyComponent._status = Resolved;
45
+ lazyComponent._result = defaultExport;
46
}
47
},
48
error => {
packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js
+9
-3
@@ -573,6 +573,10 @@ describe('ReactDebugFiberPerf', () => {
573
return <span />;
574
}
575
576
+ async function fakeImport(result) {
577
+ return {default: result};
578
+ }
579
+
580
let resolve;
581
const LazyFoo = React.lazy(
582
() =>
@@ -591,9 +595,11 @@ describe('ReactDebugFiberPerf', () => {
595
ReactNoop.flush();
596
expect(getFlameChart()).toMatchSnapshot();
597
594
- resolve(function Foo() {
595
- return <div />;
596
- });
598
+ resolve(
599
+ fakeImport(function Foo() {
600
+ return <div />;
601
+ }),
602
+ );
603
await LazyFoo;
604
605
ReactNoop.render(
packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js
+29
-25
@@ -21,8 +21,12 @@ describe('ReactLazy', () => {
21
return props.text;
22
}
23
24
+ async function fakeImport(result) {
25
+ return {default: result};
26
+ }
27
+
28
it('suspends until module has loaded', async () => {
25
- const LazyText = lazy(async () => Text);
29
+ const LazyText = lazy(() => fakeImport(Text));
30
31
const root = ReactTestRenderer.create(
32
<Suspense fallback={<Text text="Loading..." />}>
@@ -51,8 +55,10 @@ describe('ReactLazy', () => {
55
expect(root).toMatchRenderedOutput('Hi again');
56
});
57
54
- it('uses `default` property, if it exists', async () => {
55
- const LazyText = lazy(async () => ({default: Text}));
58
+ it('does not support arbitrary promises, only module objects', async () => {
59
+ spyOnDev(console, 'error');
60
+
61
+ const LazyText = lazy(async () => Text);
62
63
const root = ReactTestRenderer.create(
64
<Suspense fallback={<Text text="Loading..." />}>
@@ -67,17 +73,13 @@ describe('ReactLazy', () => {
73
74
await LazyText;
75
70
- expect(root).toFlushAndYield(['Hi']);
71
- expect(root).toMatchRenderedOutput('Hi');
72
-
73
- // Should not suspend on update
74
- root.update(
75
- <Suspense fallback={<Text text="Loading..." />}>
76
- <LazyText text="Hi again" />
77
- </Suspense>,
78
- );
79
- expect(root).toFlushAndYield(['Hi again']);
80
- expect(root).toMatchRenderedOutput('Hi again');
76
+ if (__DEV__) {
77
+ expect(console.error).toHaveBeenCalledTimes(1);
78
+ expect(console.error.calls.argsFor(0)[0]).toContain(
79
+ 'Expected the result of a dynamic import() call',
80
+ );
81
+ }
82
+ expect(root).toFlushAndThrow('Element type is invalid');
83
});
84
85
it('throws if promise rejects', async () => {
@@ -117,8 +119,8 @@ describe('ReactLazy', () => {
119
}
120
}
121
120
- const LazyChildA = lazy(async () => Child);
121
- const LazyChildB = lazy(async () => Child);
122
+ const LazyChildA = lazy(() => fakeImport(Child));
123
+ const LazyChildB = lazy(() => fakeImport(Child));
124
125
function Parent({swap}) {
126
return (
@@ -160,7 +162,7 @@ describe('ReactLazy', () => {
162
return <Text {...props} />;
163
}
164
T.defaultProps = {text: 'Hi'};
163
- const LazyText = lazy(async () => T);
165
+ const LazyText = lazy(() => fakeImport(T));
166
167
const root = ReactTestRenderer.create(
168
<Suspense fallback={<Text text="Loading..." />}>
@@ -200,7 +202,7 @@ describe('ReactLazy', () => {
202
);
203
}
204
LazyImpl.defaultProps = {siblingText: 'Sibling'};
203
- const Lazy = lazy(async () => LazyImpl);
205
+ const Lazy = lazy(() => fakeImport(LazyImpl));
206
207
class Stateful extends React.Component {
208
state = {text: 'A'};
@@ -239,7 +241,7 @@ describe('ReactLazy', () => {
241
const LazyFoo = lazy(() => {
242
ReactTestRenderer.unstable_yield('Started loading');
243
const Foo = props => <div>{[<Text text="A" />, <Text text="B" />]}</div>;
242
- return Promise.resolve(Foo);
244
+ return fakeImport(Foo);
245
});
246
247
const root = ReactTestRenderer.create(
@@ -263,13 +265,13 @@ describe('ReactLazy', () => {
265
});
266
267
it('supports class and forwardRef components', async () => {
266
- const LazyClass = lazy(async () => {
268
+ const LazyClass = lazy(() => {
269
class Foo extends React.Component {
270
render() {
271
return <Text text="Foo" />;
272
}
273
}
272
- return Foo;
274
+ return fakeImport(Foo);
275
});
276
277
const LazyForwardRef = lazy(async () => {
@@ -278,10 +280,12 @@ describe('ReactLazy', () => {
280
return <Text text="Bar" />;
281
}
282
}
281
- return React.forwardRef((props, ref) => {
282
- ReactTestRenderer.unstable_yield('forwardRef');
283
- return <Bar ref={ref} />;
284
- });
283
+ return fakeImport(
284
+ React.forwardRef((props, ref) => {
285
+ ReactTestRenderer.unstable_yield('forwardRef');
286
+ return <Bar ref={ref} />;
287
+ }),
288
+ );
289
});
290
291
const ref = React.createRef();
packages/react-reconciler/src/__tests__/ReactPure-test.internal.js
+6
-2
@@ -34,6 +34,10 @@ describe('pure', () => {
34
return <span prop={props.text} />;
35
}
36
37
+ async function fakeImport(result) {
38
+ return {default: result};
39
+ }
40
+
41
// Tests should run against both the lazy and non-lazy versions of `pure`.
42
// To make the tests work for both versions, we wrap the non-lazy version in
43
// a lazy function component.
@@ -42,11 +46,11 @@ describe('pure', () => {
46
function Indirection(props) {
47
return <Pure {...props} />;
48
}
45
- return React.lazy(async () => Indirection);
49
+ return React.lazy(() => fakeImport(Indirection));
50
});
51
sharedTests('lazy', (...args) => {
52
const Pure = React.pure(...args);
49
- return React.lazy(async () => Pure);
53
+ return React.lazy(() => fakeImport(Pure));
54
});
55
56
function sharedTests(label, pure) {
packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js
+5
-1
@@ -256,7 +256,11 @@ describe('ReactSuspense', () => {
256
}
257
}
258
259
- const LazyClass = React.lazy(() => Promise.resolve(Class));
259
+ async function fakeImport(result) {
260
+ return {default: result};
261
+ }
262
+
263
+ const LazyClass = React.lazy(() => fakeImport(Class));
264
265
const root = ReactTestRenderer.create(
266
<Suspense fallback={<Text text="Loading..." />}>
packages/shared/ReactLazyComponent.js
+5
-5
@@ -13,14 +13,14 @@ export type Thenable<T, R> = {
13
14
export type LazyComponent<T> = {
15
$$typeof: Symbol | number,
16
- _ctor: () => Thenable<T, mixed>,
16
+ _ctor: () => Thenable<{default: T}, mixed>,
17
_status: 0 | 1 | 2,
18
_result: any,
19
};
20
21
-type ResolvedLazyComponentThenable<T> = {
21
+type ResolvedLazyComponent<T> = {
22
$$typeof: Symbol | number,
23
- _ctor: () => Thenable<T, mixed>,
23
+ _ctor: () => Thenable<{default: T}, mixed>,
24
_status: 1,
25
_result: any,
26
};
@@ -30,13 +30,13 @@ export const Resolved = 1;
30
export const Rejected = 2;
31
32
export function getResultFromResolvedLazyComponent<T>(
33
- lazyComponent: ResolvedLazyComponentThenable<T>,
33
+ lazyComponent: ResolvedLazyComponent<T>,
34
): T {
35
return lazyComponent._result;
36
}
37
38
export function refineResolvedLazyComponent<T>(
39
lazyComponent: LazyComponent<T>,
40
-): ResolvedLazyComponentThenable<T> | null {
40
+): ResolvedLazyComponent<T> | null {
41
return lazyComponent._status === Resolved ? lazyComponent._result : null;
42
}