Bugfix for facebook#13886

Fixes a bug where a lazy component does not cache the result of its constructor.
acdlite · Oct 19, 2018 · d5fffee · d5fffee
1 parent 6938dca
commit d5fffee
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 22 deletions.
diff --git a/packages/jest-react/src/JestReact.js b/packages/jest-react/src/JestReact.js
@@ -35,19 +35,17 @@ function assertYieldsWereCleared(root) {
 }
 
 export function toFlushAndYield(root, expectedYields) {
+  assertYieldsWereCleared(root);
+  const actualYields = root.unstable_flushAll();
   return captureAssertion(() => {
-    assertYieldsWereCleared(root);
-    const actualYields = root.unstable_flushAll();
     expect(actualYields).toEqual(expectedYields);
   });
 }
 
 export function toFlushAndYieldThrough(root, expectedYields) {
+  assertYieldsWereCleared(root);
+  const actualYields = root.unstable_flushNumberOfYields(expectedYields.length);
   return captureAssertion(() => {
-    assertYieldsWereCleared(root);
-    const actualYields = root.unstable_flushNumberOfYields(
-      expectedYields.length,
-    );
     expect(actualYields).toEqual(expectedYields);
   });
 }
@@ -76,8 +74,8 @@ export function toHaveYielded(ReactTestRenderer, expectedYields) {
 }
 
 export function toFlushAndThrow(root, ...rest) {
+  assertYieldsWereCleared(root);
   return captureAssertion(() => {
-    assertYieldsWereCleared(root);
     expect(() => {
       root.unstable_flushAll();
     }).toThrow(...rest);

diff --git a/packages/react-reconciler/src/ReactFiberLazyComponent.js b/packages/react-reconciler/src/ReactFiberLazyComponent.js
@@ -7,21 +7,27 @@
  * @flow
  */
 
-import type {LazyComponent} from 'shared/ReactLazyComponent';
+import type {LazyComponent, Thenable} 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;
+  const result = lazyComponent._result;
   switch (status) {
-    case Resolved:
-      const Component: T = lazyComponent._result;
+    case Resolved: {
+      const Component: T = result;
       return Component;
-    case Rejected:
-      throw lazyComponent._result;
-    case Pending:
-      throw lazyComponent;
+    }
+    case Rejected: {
+      const error: mixed = result;
+      throw error;
+    }
+    case Pending: {
+      const thenable: Thenable<T, mixed> = result;
+      throw thenable;
+    }
     default: {
       lazyComponent._status = Pending;
       const ctor = lazyComponent._ctor;
@@ -52,6 +58,7 @@ export function readLazyComponentType<T>(lazyComponent: LazyComponent<T>): T {
           }
         },
       );
+      lazyComponent._result = thenable;
       throw thenable;
     }
   }

diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js
@@ -21,6 +21,10 @@ describe('ReactLazy', () => {
     return props.text;
   }
 
+  function delay(ms) {
+    return new Promise(resolve => setTimeout(() => resolve(), ms));
+  }
+
   async function fakeImport(result) {
     return {default: result};
   }
@@ -40,7 +44,7 @@ describe('ReactLazy', () => {
     expect(root).toFlushAndYield(['Loading...']);
     expect(root).toMatchRenderedOutput(null);
 
-    await LazyText;
+    await {};
 
     expect(root).toFlushAndYield(['Hi']);
     expect(root).toMatchRenderedOutput('Hi');
@@ -55,6 +59,47 @@ describe('ReactLazy', () => {
     expect(root).toMatchRenderedOutput('Hi again');
   });
 
+  it('multiple lazy components', async () => {
+    function Foo() {
+      return <Text text="Foo" />;
+    }
+
+    function Bar() {
+      return <Text text="Bar" />;
+    }
+
+    const promiseForFoo = delay(1000).then(() => fakeImport(Foo));
+    const promiseForBar = delay(2000).then(() => fakeImport(Bar));
+
+    const LazyFoo = lazy(() => promiseForFoo);
+    const LazyBar = lazy(() => promiseForBar);
+
+    const root = ReactTestRenderer.create(
+      <Suspense fallback={<Text text="Loading..." />}>
+        <LazyFoo />
+        <LazyBar />
+      </Suspense>,
+      {
+        unstable_isConcurrent: true,
+      },
+    );
+
+    expect(root).toFlushAndYield(['Loading...']);
+    expect(root).toMatchRenderedOutput(null);
+
+    jest.advanceTimersByTime(1000);
+    await promiseForFoo;
+
+    expect(root).toFlushAndYield(['Foo', 'Loading...']);
+    expect(root).toMatchRenderedOutput(null);
+
+    jest.advanceTimersByTime(1000);
+    await promiseForBar;
+
+    expect(root).toFlushAndYield(['Foo', 'Bar']);
+    expect(root).toMatchRenderedOutput('FooBar');
+  });
+
   it('does not support arbitrary promises, only module objects', async () => {
     spyOnDev(console, 'error');
 
@@ -71,7 +116,7 @@ describe('ReactLazy', () => {
     expect(root).toFlushAndYield(['Loading...']);
     expect(root).toMatchRenderedOutput(null);
 
-    await LazyText;
+    await {};
 
     if (__DEV__) {
       expect(console.error).toHaveBeenCalledTimes(1);
@@ -100,7 +145,7 @@ describe('ReactLazy', () => {
     expect(root).toMatchRenderedOutput(null);
 
     try {
-      await LazyText;
+      await {};
     } catch (e) {}
 
     expect(root).toFlushAndThrow('Bad network');
@@ -176,7 +221,7 @@ describe('ReactLazy', () => {
     expect(root).toFlushAndYield(['Loading...']);
     expect(root).toMatchRenderedOutput(null);
 
-    await LazyText;
+    await {};
 
     expect(root).toFlushAndYield(['Hi']);
     expect(root).toMatchRenderedOutput('Hi');
@@ -226,7 +271,7 @@ describe('ReactLazy', () => {
     expect(root).toFlushAndYield(['Loading...']);
     expect(root).toMatchRenderedOutput(null);
 
-    await Lazy;
+    await {};
 
     expect(root).toFlushAndYield(['Lazy', 'Sibling', 'A']);
     expect(root).toMatchRenderedOutput('SiblingA');
@@ -256,7 +301,7 @@ describe('ReactLazy', () => {
     expect(root).toFlushAndYield(['Started loading', 'Loading...']);
     expect(root).toMatchRenderedOutput(null);
 
-    await LazyFoo;
+    await {};
 
     expect(() => {
       expect(root).toFlushAndYield(['A', 'B']);
@@ -303,8 +348,7 @@ describe('ReactLazy', () => {
     expect(root).toMatchRenderedOutput(null);
     expect(ref.current).toBe(null);
 
-    await LazyClass;
-    await LazyForwardRef;
+    await {};
 
     expect(root).toFlushAndYield(['Foo', 'forwardRef', 'Bar']);
     expect(root).toMatchRenderedOutput('FooBar');