Allow flushSync to noop in life cycles but with a warning (#18759)

facebook · Apr 28, 2020 · 53ce0c3 · 53ce0c3
1 parent f342a23
commit 53ce0c3
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 29 deletions.
diff --git a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js
@@ -127,7 +127,7 @@ describe('ReactDOMFiberAsync', () => {
     expect(ops).toEqual(['A', 'ABCD']);
   });
 
-  it('flushSync throws if already performing work', () => {
+  it('flushSync logs an error if already performing work', () => {
     class Component extends React.Component {
       componentDidUpdate() {
         ReactDOM.flushSync(() => {});
@@ -140,7 +140,7 @@ describe('ReactDOMFiberAsync', () => {
     // Initial mount
     ReactDOM.render(<Component />, container);
     // Update
-    expect(() => ReactDOM.render(<Component />, container)).toThrow(
+    expect(() => ReactDOM.render(<Component />, container)).toErrorDev(
       'flushSync was called from inside a lifecycle method',
     );
   });

diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -1228,14 +1228,17 @@ export function unbatchedUpdates<A, R>(fn: (a: A) => R, a: A): R {
 }
 
 export function flushSync<A, R>(fn: A => R, a: A): R {
-  if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
-    invariant(
-      false,
-      'flushSync was called from inside a lifecycle method. It cannot be ' +
-        'called when React is already rendering.',
-    );
-  }
   const prevExecutionContext = executionContext;
+  if ((prevExecutionContext & (RenderContext | CommitContext)) !== NoContext) {
+    if (__DEV__) {
+      console.error(
+        'flushSync was called from inside a lifecycle method. React cannot ' +
+          'flush when React is already rendering. Consider moving this call to ' +
+          'a scheduler task or micro task.',
+      );
+    }
+    return fn(a);
+  }
   executionContext |= BatchedContext;
   try {
     return runWithPriority(ImmediateSchedulerPriority, fn.bind(null, a));

diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
@@ -1150,14 +1150,17 @@ export function unbatchedUpdates<A, R>(fn: (a: A) => R, a: A): R {
 }
 
 export function flushSync<A, R>(fn: A => R, a: A): R {
-  if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
-    invariant(
-      false,
-      'flushSync was called from inside a lifecycle method. It cannot be ' +
-        'called when React is already rendering.',
-    );
-  }
   const prevExecutionContext = executionContext;
+  if ((prevExecutionContext & (RenderContext | CommitContext)) !== NoContext) {
+    if (__DEV__) {
+      console.error(
+        'flushSync was called from inside a lifecycle method. React cannot ' +
+          'flush when React is already rendering. Consider moving this call to ' +
+          'a scheduler task or micro task.',
+      );
+    }
+    return fn(a);
+  }
   executionContext |= BatchedContext;
   try {
     return runWithPriority(ImmediatePriority, fn.bind(null, a));

diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
@@ -1808,22 +1808,26 @@ describe('ReactHooksWithNoopRenderer', () => {
           ReactNoop.flushSync(() => {
             updateCount(props.count);
           });
+          // This shouldn't flush synchronously.
+          expect(ReactNoop.getChildren()).not.toEqual([
+            span('Count: ' + props.count),
+          ]);
         }, [props.count]);
         return <Text text={'Count: ' + count} />;
       }
-      act(() => {
-        ReactNoop.render(<Counter count={0} />, () =>
-          Scheduler.unstable_yieldValue('Sync effect'),
-        );
-        expect(Scheduler).toFlushAndYieldThrough([
-          'Count: (empty)',
-          'Sync effect',
-        ]);
-        expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]);
-        expect(() => {
-          ReactNoop.flushPassiveEffects();
-        }).toThrow('flushSync was called from inside a lifecycle method');
-      });
+      expect(() =>
+        act(() => {
+          ReactNoop.render(<Counter count={0} />, () =>
+            Scheduler.unstable_yieldValue('Sync effect'),
+          );
+          expect(Scheduler).toFlushAndYieldThrough([
+            'Count: (empty)',
+            'Sync effect',
+          ]);
+          expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]);
+        }),
+      ).toErrorDev('flushSync was called from inside a lifecycle method');
+      expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
     });
 
     it('unmounts previous effect', () => {