Remove useMutationEffect (facebook#14336)

useMutationEffect has problems (namely, refs aren't attached at the time that it runs) and we're not positive it's necessary. useLayoutEffect runs at the same time as componentDidMount/Update so it's sufficient for all existing use cases; it can be used in any case that useEffect happens too late. Until we figure out what we want to do, let's delete it.
n8schloss · Jan 31, 2019 · 1794f1a · 1794f1a
1 parent 88bdc8d
commit 1794f1a
Show file tree

Hide file tree

Showing 11 changed files with 22 additions and 275 deletions.
diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js
@@ -52,7 +52,6 @@ function getPrimitiveStackCache(): Map<string, Array<any>> {
       Dispatcher.useState(null);
       Dispatcher.useReducer((s, a) => s, null);
       Dispatcher.useRef(null);
-      Dispatcher.useMutationEffect(() => {});
       Dispatcher.useLayoutEffect(() => {});
       Dispatcher.useEffect(() => {});
       Dispatcher.useImperativeMethods(undefined, () => null);
@@ -140,18 +139,6 @@ function useRef<T>(initialValue: T): {current: T} {
   return ref;
 }
 
-function useMutationEffect(
-  create: () => mixed,
-  inputs: Array<mixed> | void | null,
-): void {
-  nextHook();
-  hookLog.push({
-    primitive: 'MutationEffect',
-    stackError: new Error(),
-    value: create,
-  });
-}
-
 function useLayoutEffect(
   create: () => mixed,
   inputs: Array<mixed> | void | null,
@@ -221,7 +208,6 @@ const Dispatcher = {
   useImperativeMethods,
   useLayoutEffect,
   useMemo,
-  useMutationEffect,
   useReducer,
   useRef,
   useState,

diff --git a/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.internal.js b/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.internal.js
@@ -129,7 +129,7 @@ describe('ReactHooksInspection', () => {
       return result;
     }
     function useBaz(value) {
-      React.useMutationEffect(effect);
+      React.useLayoutEffect(effect);
       let result = useCustom(value);
       return result;
     }
@@ -176,7 +176,7 @@ describe('ReactHooksInspection', () => {
         value: undefined,
         subHooks: [
           {
-            name: 'MutationEffect',
+            name: 'LayoutEffect',
             value: effect,
             subHooks: [],
           },

diff --git a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.internal.js b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.internal.js
@@ -79,7 +79,6 @@ describe('ReactHooksInspectionIntergration', () => {
       let [state2, dispatch] = React.useReducer((s, a) => a.value, 'b');
       let ref = React.useRef('c');
 
-      React.useMutationEffect(effect);
       React.useLayoutEffect(effect);
       React.useEffect(effect);
 
@@ -117,7 +116,6 @@ describe('ReactHooksInspectionIntergration', () => {
       {name: 'State', value: 'a', subHooks: []},
       {name: 'Reducer', value: 'b', subHooks: []},
       {name: 'Ref', value: 'c', subHooks: []},
-      {name: 'MutationEffect', value: effect, subHooks: []},
       {name: 'LayoutEffect', value: effect, subHooks: []},
       {name: 'Effect', value: effect, subHooks: []},
       {name: 'ImperativeMethods', value: outsideRef.current, subHooks: []},
@@ -134,7 +132,6 @@ describe('ReactHooksInspectionIntergration', () => {
       {name: 'State', value: 'A', subHooks: []},
       {name: 'Reducer', value: 'B', subHooks: []},
       {name: 'Ref', value: 'C', subHooks: []},
-      {name: 'MutationEffect', value: effect, subHooks: []},
       {name: 'LayoutEffect', value: effect, subHooks: []},
       {name: 'Effect', value: effect, subHooks: []},
       {name: 'ImperativeMethods', value: outsideRef.current, subHooks: []},

diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js
@@ -25,7 +25,6 @@ let useCallback;
 let useMemo;
 let useRef;
 let useImperativeMethods;
-let useMutationEffect;
 let useLayoutEffect;
 let forwardRef;
 let yieldedValues;
@@ -50,7 +49,6 @@ function initModules() {
   useMemo = React.useMemo;
   useRef = React.useRef;
   useImperativeMethods = React.useImperativeMethods;
-  useMutationEffect = React.useMutationEffect;
   useLayoutEffect = React.useLayoutEffect;
   forwardRef = React.forwardRef;
 
@@ -551,22 +549,6 @@ describe('ReactDOMServerHooks', () => {
     });
   });
 
-  describe('useMutationEffect', () => {
-    it('should warn when invoked during render', async () => {
-      function Counter() {
-        useMutationEffect(() => {
-          throw new Error('should not be invoked');
-        });
-
-        return <Text text="Count: 0" />;
-      }
-      const domNode = await serverRender(<Counter />, 1);
-      expect(clearYields()).toEqual(['Count: 0']);
-      expect(domNode.tagName).toEqual('SPAN');
-      expect(domNode.textContent).toEqual('Count: 0');
-    });
-  });
-
   describe('useLayoutEffect', () => {
     it('should warn when invoked during render', async () => {
       function Counter() {

diff --git a/packages/react-dom/src/server/ReactPartialRendererHooks.js b/packages/react-dom/src/server/ReactPartialRendererHooks.js
@@ -271,20 +271,6 @@ function useRef<T>(initialValue: T): {current: T} {
   }
 }
 
-function useMutationEffect(
-  create: () => mixed,
-  inputs: Array<mixed> | void | null,
-) {
-  warning(
-    false,
-    'useMutationEffect does nothing on the server, because its effect cannot ' +
-      "be encoded into the server renderer's output format. This will lead " +
-      'to a mismatch between the initial, non-hydrated UI and the intended ' +
-      'UI. To avoid this, useMutationEffect should only be used in ' +
-      'components that render exclusively on the client.',
-  );
-}
-
 export function useLayoutEffect(
   create: () => mixed,
   inputs: Array<mixed> | void | null,
@@ -358,7 +344,6 @@ export const Dispatcher = {
   useReducer,
   useRef,
   useState,
-  useMutationEffect,
   useLayoutEffect,
   // Callbacks are passed as they are in the server environment.
   useCallback: identity,

diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js
@@ -1087,6 +1087,8 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void {
       case ForwardRef:
       case MemoComponent:
       case SimpleMemoComponent: {
+        // Note: We currently never use MountMutation, but useLayout uses
+        // UnmountMutation.
         commitHookEffectList(UnmountMutation, MountMutation, finishedWork);
         return;
       }
@@ -1101,6 +1103,8 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void {
     case ForwardRef:
     case MemoComponent:
     case SimpleMemoComponent: {
+      // Note: We currently never use MountMutation, but useLayout uses
+      // UnmountMutation.
       commitHookEffectList(UnmountMutation, MountMutation, finishedWork);
       return;
     }

diff --git a/packages/react-reconciler/src/ReactFiberDispatcher.js b/packages/react-reconciler/src/ReactFiberDispatcher.js
@@ -15,7 +15,6 @@ import {
   useImperativeMethods,
   useLayoutEffect,
   useMemo,
-  useMutationEffect,
   useReducer,
   useRef,
   useState,
@@ -29,7 +28,6 @@ export const Dispatcher = {
   useImperativeMethods,
   useLayoutEffect,
   useMemo,
-  useMutationEffect,
   useReducer,
   useRef,
   useState,

diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js
@@ -16,15 +16,12 @@ import {NoWork} from './ReactFiberExpirationTime';
 import {enableHooks} from 'shared/ReactFeatureFlags';
 import {readContext} from './ReactFiberNewContext';
 import {
-  Snapshot as SnapshotEffect,
   Update as UpdateEffect,
   Passive as PassiveEffect,
 } from 'shared/ReactSideEffectTags';
 import {
   NoEffect as NoHookEffect,
-  UnmountSnapshot,
   UnmountMutation,
-  MountMutation,
   MountLayout,
   UnmountPassive,
   MountPassive,
@@ -517,18 +514,6 @@ export function useRef<T>(initialValue: T): {current: T} {
   return ref;
 }
 
-export function useMutationEffect(
-  create: () => mixed,
-  inputs: Array<mixed> | void | null,
-): void {
-  useEffectImpl(
-    SnapshotEffect | UpdateEffect,
-    UnmountSnapshot | MountMutation,
-    create,
-    inputs,
-  );
-}
-
 export function useLayoutEffect(
   create: () => mixed,
   inputs: Array<mixed> | void | null,
@@ -586,26 +571,21 @@ export function useImperativeMethods<T>(
   // TODO: I've implemented this on top of useEffect because it's almost the
   // same thing, and it would require an equal amount of code. It doesn't seem
   // like a common enough use case to justify the additional size.
-  useEffectImpl(
-    UpdateEffect,
-    UnmountMutation | MountLayout,
-    () => {
-      if (typeof ref === 'function') {
-        const refCallback = ref;
-        const inst = create();
-        refCallback(inst);
-        return () => refCallback(null);
-      } else if (ref !== null && ref !== undefined) {
-        const refObject = ref;
-        const inst = create();
-        refObject.current = inst;
-        return () => {
-          refObject.current = null;
-        };
-      }
-    },
-    nextInputs,
-  );
+  useLayoutEffect(() => {
+    if (typeof ref === 'function') {
+      const refCallback = ref;
+      const inst = create();
+      refCallback(inst);
+      return () => refCallback(null);
+    } else if (ref !== null && ref !== undefined) {
+      const refObject = ref;
+      const inst = create();
+      refObject.current = inst;
+      return () => {
+        refObject.current = null;
+      };
+    }
+  }, nextInputs);
 }
 
 export function useCallback<T>(