Fix Issue with Undefined Lazy Imports By Refactoring Lazy Initialization Order (#21642)
* Add a DEV warning for common case * Don't set Pending flag before we know it's a promise * Move default exports extraction to render phase This is really where most unwrapping happen. The resolved promise is the module object and then we read things from it. This way it lines up a bit closer with the Promise model too since the promise resolving to React gets passed this same value. If this throws, then it throws during render so it's caught properly and you can break on it and even see it on the right stack. * Check if the default is in the module object instead of if it's undefined Normally we'd just check if something is undefined but in this case it's valid to have an undefined value in the export but if you don't have a property then you're probably importing the wrong kind of object. * We need to check if it's uninitialized for sync resolution Co-authored-by: Dan Abramov <dan.abramov@me.com>
Sebastian Markbåge committed
Jun 7, 2021 at 17:47 UTC
5aa0c5671fdddc46092d46420fff84a82df558ac
2 files changed
+44
-23
packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js
+2
-2
@@ -178,13 +178,13 @@ describe('ReactLazy', () => {
178
179
await Promise.resolve();
180
181
+ expect(Scheduler).toFlushAndThrow('Element type is invalid');
182
if (__DEV__) {
182
- expect(console.error).toHaveBeenCalledTimes(1);
183
+ expect(console.error).toHaveBeenCalledTimes(3);
184
expect(console.error.calls.argsFor(0)[0]).toContain(
185
'Expected the result of a dynamic import() call',
186
);
187
}
187
- expect(Scheduler).toFlushAndThrow('Element type is invalid');
188
});
189
190
it('throws if promise rejects', async () => {
packages/react/src/ReactLazy.js
+42
-21
@@ -28,7 +28,7 @@ type PendingPayload = {
28
29
type ResolvedPayload<T> = {
30
_status: 1,
31
- _result: T,
31
+ _result: {default: T},
32
};
33
34
type RejectedPayload = {
@@ -53,33 +53,21 @@ function lazyInitializer<T>(payload: Payload<T>): T {
53
const ctor = payload._result;
54
const thenable = ctor();
55
// Transition to the next state.
56
- const pending: PendingPayload = (payload: any);
57
- pending._status = Pending;
58
- pending._result = thenable;
56
+ // This might throw either because it's missing or throws. If so, we treat it
57
+ // as still uninitialized and try again next time. Which is the same as what
58
+ // happens if the ctor or any wrappers processing the ctor throws. This might
59
+ // end up fixing it if the resolution was a concurrency bug.
60
thenable.then(
61
moduleObject => {
61
- if (payload._status === Pending) {
62
- const defaultExport = moduleObject.default;
63
- if (__DEV__) {
64
- if (defaultExport === undefined) {
65
- console.error(
66
- 'lazy: Expected the result of a dynamic import() call. ' +
67
- 'Instead received: %s\n\nYour code should look like: \n ' +
68
- // Break up imports to avoid accidentally parsing them as dependencies.
69
- 'const MyComponent = lazy(() => imp' +
70
- "ort('./MyComponent'))",
71
- moduleObject,
72
- );
73
- }
74
- }
62
+ if (payload._status === Pending || payload._status === Uninitialized) {
63
// Transition to the next state.
64
const resolved: ResolvedPayload<T> = (payload: any);
65
resolved._status = Resolved;
78
- resolved._result = defaultExport;
66
+ resolved._result = moduleObject;
67
}
68
},
69
error => {
82
- if (payload._status === Pending) {
70
+ if (payload._status === Pending || payload._status === Uninitialized) {
71
// Transition to the next state.
72
const rejected: RejectedPayload = (payload: any);
73
rejected._status = Rejected;
@@ -87,9 +75,42 @@ function lazyInitializer<T>(payload: Payload<T>): T {
75
}
76
},
77
);
78
+ if (payload._status === Uninitialized) {
79
+ // In case, we're still uninitialized, then we're waiting for the thenable
80
+ // to resolve. Set it as pending in the meantime.
81
+ const pending: PendingPayload = (payload: any);
82
+ pending._status = Pending;
83
+ pending._result = thenable;
84
+ }
85
}
86
if (payload._status === Resolved) {
92
- return payload._result;
87
+ const moduleObject = payload._result;
88
+ if (__DEV__) {
89
+ if (moduleObject === undefined) {
90
+ console.error(
91
+ 'lazy: Expected the result of a dynamic import() call. ' +
92
+ 'Instead received: %s\n\nYour code should look like: \n ' +
93
+ // Break up imports to avoid accidentally parsing them as dependencies.
94
+ 'const MyComponent = lazy(() => imp' +
95
+ "ort('./MyComponent'))\n\n" +
96
+ 'Did you accidentally put curly braces around the import?',
97
+ moduleObject,
98
+ );
99
+ }
100
+ }
101
+ if (__DEV__) {
102
+ if (!('default' in moduleObject)) {
103
+ console.error(
104
+ 'lazy: Expected the result of a dynamic import() call. ' +
105
+ 'Instead received: %s\n\nYour code should look like: \n ' +
106
+ // Break up imports to avoid accidentally parsing them as dependencies.
107
+ 'const MyComponent = lazy(() => imp' +
108
+ "ort('./MyComponent'))",
109
+ moduleObject,
110
+ );
111
+ }
112
+ }
113
+ return moduleObject.default;
114
} else {
115
throw payload._result;
116
}