From 5aa0c5671fdddc46092d46420fff84a82df558ac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= <sema@fb.com>
Date: Mon, 7 Jun 2021 17:47:18 -0400
Subject: [PATCH] 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>
---
 .../src/__tests__/ReactLazy-test.internal.js  |  4 +-
 packages/react/src/ReactLazy.js               | 63 ++++++++++++-------
 2 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js
index b062ad8ab9..769679c9a7 100644
--- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js
+++ b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js
@@ -178,13 +178,13 @@ describe('ReactLazy', () => {
 
     await Promise.resolve();
 
+    expect(Scheduler).toFlushAndThrow('Element type is invalid');
     if (__DEV__) {
-      expect(console.error).toHaveBeenCalledTimes(1);
+      expect(console.error).toHaveBeenCalledTimes(3);
       expect(console.error.calls.argsFor(0)[0]).toContain(
         'Expected the result of a dynamic import() call',
       );
     }
-    expect(Scheduler).toFlushAndThrow('Element type is invalid');
   });
 
   it('throws if promise rejects', async () => {
diff --git a/packages/react/src/ReactLazy.js b/packages/react/src/ReactLazy.js
index 24156b69ac..0bc05df641 100644
--- a/packages/react/src/ReactLazy.js
+++ b/packages/react/src/ReactLazy.js
@@ -28,7 +28,7 @@ type PendingPayload = {
 
 type ResolvedPayload<T> = {
   _status: 1,
-  _result: T,
+  _result: {default: T},
 };
 
 type RejectedPayload = {
@@ -53,33 +53,21 @@ function lazyInitializer<T>(payload: Payload<T>): T {
     const ctor = payload._result;
     const thenable = ctor();
     // Transition to the next state.
-    const pending: PendingPayload = (payload: any);
-    pending._status = Pending;
-    pending._result = thenable;
+    // This might throw either because it's missing or throws. If so, we treat it
+    // as still uninitialized and try again next time. Which is the same as what
+    // happens if the ctor or any wrappers processing the ctor throws. This might
+    // end up fixing it if the resolution was a concurrency bug.
     thenable.then(
       moduleObject => {
-        if (payload._status === Pending) {
-          const defaultExport = moduleObject.default;
-          if (__DEV__) {
-            if (defaultExport === undefined) {
-              console.error(
-                'lazy: Expected the result of a dynamic import() call. ' +
-                  'Instead received: %s\n\nYour code should look like: \n  ' +
-                  // Break up imports to avoid accidentally parsing them as dependencies.
-                  'const MyComponent = lazy(() => imp' +
-                  "ort('./MyComponent'))",
-                moduleObject,
-              );
-            }
-          }
+        if (payload._status === Pending || payload._status === Uninitialized) {
           // Transition to the next state.
           const resolved: ResolvedPayload<T> = (payload: any);
           resolved._status = Resolved;
-          resolved._result = defaultExport;
+          resolved._result = moduleObject;
         }
       },
       error => {
-        if (payload._status === Pending) {
+        if (payload._status === Pending || payload._status === Uninitialized) {
           // Transition to the next state.
           const rejected: RejectedPayload = (payload: any);
           rejected._status = Rejected;
@@ -87,9 +75,42 @@ function lazyInitializer<T>(payload: Payload<T>): T {
         }
       },
     );
+    if (payload._status === Uninitialized) {
+      // In case, we're still uninitialized, then we're waiting for the thenable
+      // to resolve. Set it as pending in the meantime.
+      const pending: PendingPayload = (payload: any);
+      pending._status = Pending;
+      pending._result = thenable;
+    }
   }
   if (payload._status === Resolved) {
-    return payload._result;
+    const moduleObject = payload._result;
+    if (__DEV__) {
+      if (moduleObject === undefined) {
+        console.error(
+          'lazy: Expected the result of a dynamic import() call. ' +
+            'Instead received: %s\n\nYour code should look like: \n  ' +
+            // Break up imports to avoid accidentally parsing them as dependencies.
+            'const MyComponent = lazy(() => imp' +
+            "ort('./MyComponent'))\n\n" +
+            'Did you accidentally put curly braces around the import?',
+          moduleObject,
+        );
+      }
+    }
+    if (__DEV__) {
+      if (!('default' in moduleObject)) {
+        console.error(
+          'lazy: Expected the result of a dynamic import() call. ' +
+            'Instead received: %s\n\nYour code should look like: \n  ' +
+            // Break up imports to avoid accidentally parsing them as dependencies.
+            'const MyComponent = lazy(() => imp' +
+            "ort('./MyComponent'))",
+          moduleObject,
+        );
+      }
+    }
+    return moduleObject.default;
   } else {
     throw payload._result;
   }