Phased dispatcher (#14701)

* Move DEV-only function right above where it's used

I don't like looking at this top-level function #petty

* Use different dispatchers for functions & classes

Classes support readContext, but not any of the other dispatcher
methods. Function support all methods.

This is a more robust version of our previous strategy of checking
whether `currentlyRenderingFiber` is null.

As a next step, we can use a separate dispatcher for each phase of the
render cycle (mount versus update).

* Use separate dispatchers for mount and update

* Remove mount code from update path

Deletes mount-specific code from the update path, since it should be
unreachable. To continue supporting progressive enhancement (mounting
new hooks at the end of the list), we detect when there are no more
current hooks and switch back to the mount dispatcher. Progressive
enhancement isn't officially supported yet, so it will continue to warn.

* Factoring nits

* Fix Flow

Had to cheat more than I would like

* More Flow nits

* Switch back to using a special dispatcher for nested hooks in DEV

In order for this strategy to work, I had to revert progressive
enhancement support (appending hooks to the end). It was previously a
warning but now it results in an error. We'll reconsider later.

* Always pass args to updateState and updateReducer

Even though the extra args are only used on mount, to ensure
type consistency.

packages/react-debug-tools/src/ReactDebugHooks.js

@@ -10,7 +10,7 @@
 import type {ReactContext, ReactProviderType} from 'shared/ReactTypes';
 import type {Fiber} from 'react-reconciler/src/ReactFiber';
 import type {Hook} from 'react-reconciler/src/ReactFiberHooks';
-import typeof {Dispatcher as DispatcherType} from 'react-reconciler/src/ReactFiberDispatcher';
+import type {Dispatcher as DispatcherType} from 'react-reconciler/src/ReactFiberHooks';
 
 import ErrorStackParser from 'error-stack-parser';
 import ReactSharedInternals from 'shared/ReactSharedInternals';

packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js

@@ -432,21 +432,25 @@ describe('ReactDOMServerHooks', () => {
       expect(domNode.textContent).toEqual('hi');
     });
 
-    itRenders('with a warning for useRef inside useReducer', async render => {
-      function App() {
-        const [value, dispatch] = useReducer((state, action) => {
-          useRef(0);
-          return state + 1;
-        }, 0);
-        if (value === 0) {
-          dispatch();
+    itThrowsWhenRendering(
+      'with a warning for useRef inside useReducer',
+      async render => {
+        function App() {
+          const [value, dispatch] = useReducer((state, action) => {
+            useRef(0);
+            return state + 1;
+          }, 0);
+          if (value === 0) {
+            dispatch();
+          }
+          return value;
         }
-        return value;
-      }
 
-      const domNode = await render(<App />, 1);
-      expect(domNode.textContent).toEqual('1');
-    });
+        const domNode = await render(<App />, 1);
+        expect(domNode.textContent).toEqual('1');
+      },
+      'Rendered more hooks than during the previous render',
+    );
 
     itRenders('with a warning for useRef inside useState', async render => {
       function App() {

packages/react-dom/src/server/ReactPartialRendererHooks.js

@@ -7,7 +7,7 @@
  * @flow
  */
 
-import typeof {Dispatcher as DispatcherType} from 'react-reconciler/src/ReactFiberDispatcher';
+import type {Dispatcher as DispatcherType} from 'react-reconciler/src/ReactFiberHooks';
 import type {ThreadID} from './ReactThreadIDAllocator';
 import type {ReactContext} from 'shared/ReactTypes';
 
@@ -113,6 +113,9 @@ function areHookInputsEqual(
 }
 
 function createHook(): Hook {
+  if (numberOfReRenders > 0) {
+    invariant(false, 'Rendered more hooks than during the previous render');
+  }
   return {
     memoizedState: null,
     queue: null,

packages/react-reconciler/src/ReactFiberDispatcher.js

@@ -1,36 +0,0 @@
-/**
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * This source code is licensed under the MIT license found in the
- * LICENSE file in the root directory of this source tree.
- *
- * @flow
- */
-
-import {readContext} from './ReactFiberNewContext';
-import {
-  useCallback,
-  useContext,
-  useEffect,
-  useImperativeHandle,
-  useDebugValue,
-  useLayoutEffect,
-  useMemo,
-  useReducer,
-  useRef,
-  useState,
-} from './ReactFiberHooks';
-
-export const Dispatcher = {
-  readContext,
-  useCallback,
-  useContext,
-  useEffect,
-  useImperativeHandle,
-  useDebugValue,
-  useLayoutEffect,
-  useMemo,
-  useReducer,
-  useRef,
-  useState,
-};

packages/react-reconciler/src/ReactFiberScheduler.js

@@ -164,7 +164,7 @@ import {
   commitDetachRef,
   commitPassiveHookEffects,
 } from './ReactFiberCommitWork';
-import {Dispatcher} from './ReactFiberDispatcher';
+import {ContextOnlyDispatcher} from './ReactFiberHooks';
 
 export type Thenable = {
   then(resolve: () => mixed, reject?: () => mixed): mixed,
@@ -1216,7 +1216,8 @@ function renderRoot(root: FiberRoot, isYieldy: boolean): void {
   flushPassiveEffects();
 
   isWorking = true;
-  ReactCurrentDispatcher.current = Dispatcher;
+  const previousDispatcher = ReactCurrentDispatcher.current;
+  ReactCurrentDispatcher.current = ContextOnlyDispatcher;
 
   const expirationTime = root.nextExpirationTimeToWorkOn;
 
@@ -1377,7 +1378,7 @@ function renderRoot(root: FiberRoot, isYieldy: boolean): void {
 
   // We're done performing work. Time to clean up.
   isWorking = false;
-  ReactCurrentDispatcher.current = null;
+  ReactCurrentDispatcher.current = previousDispatcher;
   resetContextDependences();
   resetHooks();
 

packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js

@@ -719,7 +719,7 @@ describe('ReactHooks', () => {
     const root = ReactTestRenderer.create(<App />);
     expect(() => root.update(<App />)).toThrow(
       // The exact message doesn't matter, just make sure we don't allow this
-      "Cannot read property 'readContext' of null",
+      'Context can only be read while React is rendering',
     );
   });
 
@@ -740,7 +740,7 @@ describe('ReactHooks', () => {
 
     expect(() => ReactTestRenderer.create(<App />)).toThrow(
       // The exact message doesn't matter, just make sure we don't allow this
-      "Cannot read property 'readContext' of null",
+      'Context can only be read while React is rendering',
     );
   });
 
@@ -803,7 +803,10 @@ describe('ReactHooks', () => {
   });
 
   it('warns when calling hooks inside useReducer', () => {
-    const {useReducer, useRef} = React;
+    const {useReducer, useState, useRef} = React;
+
+    spyOnDev(console, 'error');
+
     function App() {
       const [value, dispatch] = useReducer((state, action) => {
         useRef(0);
@@ -812,11 +815,19 @@ describe('ReactHooks', () => {
       if (value === 0) {
         dispatch('foo');
       }
+      useState();
       return value;
     }
-    expect(() => ReactTestRenderer.create(<App />)).toWarnDev(
-      'Hooks can only be called inside the body of a function component',
-    );
+    expect(() => {
+      ReactTestRenderer.create(<App />);
+    }).toThrow('Rendered more hooks than during the previous render.');
+
+    if (__DEV__) {
+      expect(console.error).toHaveBeenCalledTimes(3);
+      expect(console.error.calls.argsFor(0)[0]).toContain(
+        'Hooks can only be called inside the body of a function component',
+      );
+    }
   });
 
   it("throws when calling hooks inside useState's initialize function", () => {

packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js

@@ -1596,11 +1596,11 @@ describe('ReactHooksWithNoopRenderer', () => {
     });
   });
 
-  describe('progressive enhancement', () => {
+  describe('progressive enhancement (not supported)', () => {
     it('mount additional state', () => {
       let updateA;
       let updateB;
-      let updateC;
+      // let updateC;
 
       function App(props) {
         const [A, _updateA] = useState(0);
@@ -1610,9 +1610,7 @@ describe('ReactHooksWithNoopRenderer', () => {
 
         let C;
         if (props.loadC) {
-          const [_C, _updateC] = useState(0);
-          C = _C;
-          updateC = _updateC;
+          useState(0);
         } else {
           C = '[not loaded]';
         }
@@ -1636,14 +1634,14 @@ describe('ReactHooksWithNoopRenderer', () => {
       ReactNoop.render(<App loadC={true} />);
       expect(() => {
         expect(ReactNoop.flush()).toEqual(['A: 2, B: 3, C: 0']);
-      }).toWarnDev([
-        'App: Rendered more hooks than during the previous render',
-      ]);
-      expect(ReactNoop.getChildren()).toEqual([span('A: 2, B: 3, C: 0')]);
+      }).toThrow('Rendered more hooks than during the previous render');
 
-      updateC(4);
-      expect(ReactNoop.flush()).toEqual(['A: 2, B: 3, C: 4']);
-      expect(ReactNoop.getChildren()).toEqual([span('A: 2, B: 3, C: 4')]);
+      // Uncomment if/when we support this again
+      // expect(ReactNoop.getChildren()).toEqual([span('A: 2, B: 3, C: 0')]);
+
+      // updateC(4);
+      // expect(ReactNoop.flush()).toEqual(['A: 2, B: 3, C: 4']);
+      // expect(ReactNoop.getChildren()).toEqual([span('A: 2, B: 3, C: 4')]);
     });
 
     it('unmount state', () => {
@@ -1714,17 +1712,17 @@ describe('ReactHooksWithNoopRenderer', () => {
       ReactNoop.render(<App showMore={true} />);
       expect(() => {
         expect(ReactNoop.flush()).toEqual([]);
-      }).toWarnDev([
-        'App: Rendered more hooks than during the previous render',
-      ]);
-      flushPassiveEffects();
-      expect(ReactNoop.clearYields()).toEqual(['Mount B']);
+      }).toThrow('Rendered more hooks than during the previous render');
 
-      ReactNoop.render(<App showMore={false} />);
-      expect(() => ReactNoop.flush()).toThrow(
-        'Rendered fewer hooks than expected. This may be caused by an ' +
-          'accidental early return statement.',
-      );
+      // Uncomment if/when we support this again
+      // flushPassiveEffects();
+      // expect(ReactNoop.clearYields()).toEqual(['Mount B']);
+
+      // ReactNoop.render(<App showMore={false} />);
+      // expect(() => ReactNoop.flush()).toThrow(
+      //   'Rendered fewer hooks than expected. This may be caused by an ' +
+      //     'accidental early return statement.',
+      // );
     });
   });
 });

packages/react-test-renderer/src/ReactShallowRenderer.js

@@ -18,7 +18,7 @@ import ReactSharedInternals from 'shared/ReactSharedInternals';
 import warning from 'shared/warning';
 import is from 'shared/objectIs';
 
-import typeof {Dispatcher as DispatcherType} from 'react-reconciler/src/ReactFiberDispatcher';
+import type {Dispatcher as DispatcherType} from 'react-reconciler/src/ReactFiberHooks';
 import type {ReactContext} from 'shared/ReactTypes';
 import type {ReactElement} from 'shared/ReactElementType';
 

packages/react/src/ReactCurrentDispatcher.js

@@ -7,7 +7,7 @@
  * @flow
  */
 
-import typeof {Dispatcher} from 'react-reconciler/src/ReactFiberDispatcher';
+import type {Dispatcher} from 'react-reconciler/src/ReactFiberHooks';
 
 /**
  * Keeps track of the current dispatcher.