From 48f6594474c449b3ed62fadedda0ffad5e3a807a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= <sema@fb.com>
Date: Wed, 10 Jul 2019 11:07:28 -0700
Subject: [PATCH] Add warning when single item or nested arrays are used with
 SuspenseList (#16094)

---
 .../src/ReactFiberBeginWork.js                | 69 +++++++++++++++-
 .../ReactSuspenseList-test.internal.js        | 82 ++++++++++++++++++-
 2 files changed, 149 insertions(+), 2 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js
index 1be03dfd27..0bf2484775 100644
--- a/packages/react-reconciler/src/ReactFiberBeginWork.js
+++ b/packages/react-reconciler/src/ReactFiberBeginWork.js
@@ -67,7 +67,7 @@ import shallowEqual from 'shared/shallowEqual';
 import getComponentName from 'shared/getComponentName';
 import ReactStrictModeWarnings from './ReactStrictModeWarnings';
 import {refineResolvedLazyComponent} from 'shared/ReactLazyComponent';
-import {REACT_LAZY_TYPE} from 'shared/ReactSymbols';
+import {REACT_LAZY_TYPE, getIteratorFn} from 'shared/ReactSymbols';
 import warning from 'shared/warning';
 import warningWithoutStack from 'shared/warningWithoutStack';
 import {
@@ -2094,6 +2094,72 @@ function validateTailOptions(
   }
 }
 
+function validateSuspenseListNestedChild(childSlot: mixed, index: number) {
+  if (__DEV__) {
+    let isArray = Array.isArray(childSlot);
+    let isIterable = !isArray && typeof getIteratorFn(childSlot) === 'function';
+    if (isArray || isIterable) {
+      let type = isArray ? 'array' : 'iterable';
+      warning(
+        false,
+        'A nested %s was passed to row #%s in <SuspenseList />. Wrap it in ' +
+          'an additional SuspenseList to configure its revealOrder: ' +
+          '<SuspenseList revealOrder=...> ... ' +
+          '<SuspenseList revealOrder=...>{%s}</SuspenseList> ... ' +
+          '</SuspenseList>',
+        type,
+        index,
+        type,
+      );
+      return false;
+    }
+  }
+  return true;
+}
+
+function validateSuspenseListChildren(
+  children: mixed,
+  revealOrder: SuspenseListRevealOrder,
+) {
+  if (__DEV__) {
+    if (
+      (revealOrder === 'forwards' || revealOrder === 'backwards') &&
+      (children !== undefined && children !== null && children !== false)
+    ) {
+      if (Array.isArray(children)) {
+        for (let i = 0; i < children.length; i++) {
+          if (!validateSuspenseListNestedChild(children[i], i)) {
+            return;
+          }
+        }
+      } else {
+        let iteratorFn = getIteratorFn(children);
+        if (typeof iteratorFn === 'function') {
+          const childrenIterator = iteratorFn.call(children);
+          if (childrenIterator) {
+            let step = childrenIterator.next();
+            let i = 0;
+            for (; !step.done; step = childrenIterator.next()) {
+              if (!validateSuspenseListNestedChild(step.value, i)) {
+                return;
+              }
+              i++;
+            }
+          }
+        } else {
+          warning(
+            false,
+            'A single row was passed to a <SuspenseList revealOrder="%s" />. ' +
+              'This is not useful since it needs multiple rows. ' +
+              'Did you mean to pass multiple children or an array?',
+            revealOrder,
+          );
+        }
+      }
+    }
+  }
+}
+
 function initSuspenseListRenderState(
   workInProgress: Fiber,
   isBackwards: boolean,
@@ -2142,6 +2208,7 @@ function updateSuspenseListComponent(
 
   validateRevealOrder(revealOrder);
   validateTailOptions(tailMode, revealOrder);
+  validateSuspenseListChildren(newChildren, revealOrder);
 
   reconcileChildren(current, workInProgress, newChildren, renderExpirationTime);
 
diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js
index 3f30ee0a1a..d8226c32e5 100644
--- a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js
+++ b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js
@@ -101,6 +101,85 @@ describe('ReactSuspenseList', () => {
     ]);
   });
 
+  it('warns if a single element is passed to a "forwards" list', () => {
+    function Foo({children}) {
+      return <SuspenseList revealOrder="forwards">{children}</SuspenseList>;
+    }
+
+    ReactNoop.render(<Foo />);
+    // No warning
+    Scheduler.unstable_flushAll();
+
+    ReactNoop.render(<Foo>{null}</Foo>);
+    // No warning
+    Scheduler.unstable_flushAll();
+
+    ReactNoop.render(<Foo>{false}</Foo>);
+    // No warning
+    Scheduler.unstable_flushAll();
+
+    ReactNoop.render(
+      <Foo>
+        <Suspense fallback="Loading">Child</Suspense>
+      </Foo>,
+    );
+
+    expect(() => Scheduler.unstable_flushAll()).toWarnDev([
+      'Warning: A single row was passed to a <SuspenseList revealOrder="forwards" />. ' +
+        'This is not useful since it needs multiple rows. ' +
+        'Did you mean to pass multiple children or an array?' +
+        '\n    in SuspenseList (at **)' +
+        '\n    in Foo (at **)',
+    ]);
+  });
+
+  it('warns if a single fragment is passed to a "backwards" list', () => {
+    function Foo() {
+      return (
+        <SuspenseList revealOrder="backwards">
+          <Fragment>{[]}</Fragment>
+        </SuspenseList>
+      );
+    }
+
+    ReactNoop.render(<Foo />);
+
+    expect(() => Scheduler.unstable_flushAll()).toWarnDev([
+      'Warning: A single row was passed to a <SuspenseList revealOrder="backwards" />. ' +
+        'This is not useful since it needs multiple rows. ' +
+        'Did you mean to pass multiple children or an array?' +
+        '\n    in SuspenseList (at **)' +
+        '\n    in Foo (at **)',
+    ]);
+  });
+
+  it('warns if a nested array is passed to a "forwards" list', () => {
+    function Foo({items}) {
+      return (
+        <SuspenseList revealOrder="forwards">
+          {items.map(name => (
+            <Suspense key={name} fallback="Loading">
+              {name}
+            </Suspense>
+          ))}
+          <div>Tail</div>
+        </SuspenseList>
+      );
+    }
+
+    ReactNoop.render(<Foo items={['A', 'B']} />);
+
+    expect(() => Scheduler.unstable_flushAll()).toWarnDev([
+      'Warning: A nested array was passed to row #0 in <SuspenseList />. ' +
+        'Wrap it in an additional SuspenseList to configure its revealOrder: ' +
+        '<SuspenseList revealOrder=...> ... ' +
+        '<SuspenseList revealOrder=...>{array}</SuspenseList> ... ' +
+        '</SuspenseList>' +
+        '\n    in SuspenseList (at **)' +
+        '\n    in Foo (at **)',
+    ]);
+  });
+
   it('shows content independently by default', async () => {
     let A = createAsyncText('A');
     let B = createAsyncText('B');
@@ -1162,7 +1241,8 @@ describe('ReactSuspenseList', () => {
     function Foo() {
       return (
         <SuspenseList revealOrder="forwards" tail="collapse">
-          <Suspense fallback="Loading">Content</Suspense>
+          <Suspense fallback="Loading">A</Suspense>
+          <Suspense fallback="Loading">B</Suspense>
         </SuspenseList>
       );
     }