@samitouri / QOS-React / commits / c2a2d8a539

Remove useMutationEffect (#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.

Sophie Alpert committed Nov 27, 2018 at 13:05 UTC c2a2d8a539bf02e40c43d36adc2826e228f30955

11 files changed +22 -275

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

-14

@@ -52,7 +52,6 @@ function getPrimitiveStackCache(): Map<string, Array<any>> {

52 Dispatcher.useState(null);

53 Dispatcher.useReducer((s, a) => s, null);

54 Dispatcher.useRef(null);

55 - Dispatcher.useMutationEffect(() => {});

55 Dispatcher.useLayoutEffect(() => {});

56 Dispatcher.useEffect(() => {});

57 Dispatcher.useImperativeMethods(undefined, () => null);

@@ -140,18 +139,6 @@ function useRef<T>(initialValue: T): {current: T} {

139 return ref;

140 }

141

143 -function useMutationEffect(

144 - create: () => mixed,

145 - inputs: Array<mixed> | void | null,

146 -): void {

147 - nextHook();

148 - hookLog.push({

149 - primitive: 'MutationEffect',

150 - stackError: new Error(),

151 - value: create,

152 - });

153 -}

154 -

142 function useLayoutEffect(

143 create: () => mixed,

144 inputs: Array<mixed> | void | null,

@@ -221,7 +208,6 @@ const Dispatcher = {

208 useImperativeMethods,

209 useLayoutEffect,

210 useMemo,

224 - useMutationEffect,

211 useReducer,

212 useRef,

213 useState,

packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.internal.js

+2 -2

@@ -129,7 +129,7 @@ describe('ReactHooksInspection', () => {

129 return result;

130 }

131 function useBaz(value) {

132 - React.useMutationEffect(effect);

132 + React.useLayoutEffect(effect);

133 let result = useCustom(value);

134 return result;

135 }

@@ -176,7 +176,7 @@ describe('ReactHooksInspection', () => {

176 value: undefined,

177 subHooks: [

178 {

179 - name: 'MutationEffect',

179 + name: 'LayoutEffect',

180 value: effect,

181 subHooks: [],

182 },

packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.internal.js

-3

@@ -79,7 +79,6 @@ describe('ReactHooksInspectionIntergration', () => {

79 let [state2, dispatch] = React.useReducer((s, a) => a.value, 'b');

80 let ref = React.useRef('c');

82 - React.useMutationEffect(effect);

82 React.useLayoutEffect(effect);

83 React.useEffect(effect);

@@ -117,7 +116,6 @@ describe('ReactHooksInspectionIntergration', () => {

116 {name: 'State', value: 'a', subHooks: []},

117 {name: 'Reducer', value: 'b', subHooks: []},

118 {name: 'Ref', value: 'c', subHooks: []},

120 - {name: 'MutationEffect', value: effect, subHooks: []},

119 {name: 'LayoutEffect', value: effect, subHooks: []},

120 {name: 'Effect', value: effect, subHooks: []},

121 {name: 'ImperativeMethods', value: outsideRef.current, subHooks: []},

@@ -134,7 +132,6 @@ describe('ReactHooksInspectionIntergration', () => {

132 {name: 'State', value: 'A', subHooks: []},

133 {name: 'Reducer', value: 'B', subHooks: []},

134 {name: 'Ref', value: 'C', subHooks: []},

137 - {name: 'MutationEffect', value: effect, subHooks: []},

135 {name: 'LayoutEffect', value: effect, subHooks: []},

136 {name: 'Effect', value: effect, subHooks: []},

137 {name: 'ImperativeMethods', value: outsideRef.current, subHooks: []},

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

-18

@@ -25,7 +25,6 @@ let useCallback;

25 let useMemo;

26 let useRef;

27 let useImperativeMethods;

28 -let useMutationEffect;

28 let useLayoutEffect;

29 let forwardRef;

30 let yieldedValues;

@@ -50,7 +49,6 @@ function initModules() {

49 useMemo = React.useMemo;

50 useRef = React.useRef;

51 useImperativeMethods = React.useImperativeMethods;

53 - useMutationEffect = React.useMutationEffect;

52 useLayoutEffect = React.useLayoutEffect;

53 forwardRef = React.forwardRef;

@@ -551,22 +549,6 @@ describe('ReactDOMServerHooks', () => {

549 });

550 });

551

554 - describe('useMutationEffect', () => {

555 - it('should warn when invoked during render', async () => {

556 - function Counter() {

557 - useMutationEffect(() => {

558 - throw new Error('should not be invoked');

559 - });

560 -

561 - return <Text text="Count: 0" />;

562 - }

563 - const domNode = await serverRender(<Counter />, 1);

564 - expect(clearYields()).toEqual(['Count: 0']);

565 - expect(domNode.tagName).toEqual('SPAN');

566 - expect(domNode.textContent).toEqual('Count: 0');

567 - });

568 - });

569 -

552 describe('useLayoutEffect', () => {

553 it('should warn when invoked during render', async () => {

554 function Counter() {

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

-15

@@ -271,20 +271,6 @@ function useRef<T>(initialValue: T): {current: T} {

271 }

272 }

273

274 -function useMutationEffect(

275 - create: () => mixed,

276 - inputs: Array<mixed> | void | null,

277 -) {

278 - warning(

279 - false,

280 - 'useMutationEffect does nothing on the server, because its effect cannot ' +

281 - "be encoded into the server renderer's output format. This will lead " +

282 - 'to a mismatch between the initial, non-hydrated UI and the intended ' +

283 - 'UI. To avoid this, useMutationEffect should only be used in ' +

284 - 'components that render exclusively on the client.',

285 - );

286 -}

287 -

274 export function useLayoutEffect(

275 create: () => mixed,

276 inputs: Array<mixed> | void | null,

@@ -358,7 +344,6 @@ export const Dispatcher = {

344 useReducer,

345 useRef,

346 useState,

361 - useMutationEffect,

347 useLayoutEffect,

348 // Callbacks are passed as they are in the server environment.

349 useCallback: identity,

packages/react-reconciler/src/ReactFiberCommitWork.js

@@ -1087,6 +1087,8 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void {

1087 case ForwardRef:

1088 case MemoComponent:

1089 case SimpleMemoComponent: {

1090 + // Note: We currently never use MountMutation, but useLayout uses

1091 + // UnmountMutation.

1092 commitHookEffectList(UnmountMutation, MountMutation, finishedWork);

1093 return;

1094 }

@@ -1101,6 +1103,8 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void {

1103 case ForwardRef:

1104 case MemoComponent:

1105 case SimpleMemoComponent: {

1106 + // Note: We currently never use MountMutation, but useLayout uses

1107 + // UnmountMutation.

1108 commitHookEffectList(UnmountMutation, MountMutation, finishedWork);

1109 return;

1110 }

packages/react-reconciler/src/ReactFiberDispatcher.js

-2

@@ -15,7 +15,6 @@ import {

15 useImperativeMethods,

16 useLayoutEffect,

17 useMemo,

18 - useMutationEffect,

18 useReducer,

19 useRef,

20 useState,

@@ -29,7 +28,6 @@ export const Dispatcher = {

28 useImperativeMethods,

29 useLayoutEffect,

30 useMemo,

32 - useMutationEffect,

31 useReducer,

32 useRef,

33 useState,

packages/react-reconciler/src/ReactFiberHooks.js

+15 -35

@@ -16,15 +16,12 @@ import {NoWork} from './ReactFiberExpirationTime';

16 import {enableHooks} from 'shared/ReactFeatureFlags';

17 import {readContext} from './ReactFiberNewContext';

18 import {

19 - Snapshot as SnapshotEffect,

19 Update as UpdateEffect,

20 Passive as PassiveEffect,

21 } from 'shared/ReactSideEffectTags';

22 import {

23 NoEffect as NoHookEffect,

25 - UnmountSnapshot,

24 UnmountMutation,

27 - MountMutation,

25 MountLayout,

26 UnmountPassive,

27 MountPassive,

@@ -517,18 +514,6 @@ export function useRef<T>(initialValue: T): {current: T} {

514 return ref;

515 }

516

520 -export function useMutationEffect(

521 - create: () => mixed,

522 - inputs: Array<mixed> | void | null,

523 -): void {

524 - useEffectImpl(

525 - SnapshotEffect | UpdateEffect,

526 - UnmountSnapshot | MountMutation,

527 - create,

528 - inputs,

529 - );

530 -}

531 -

517 export function useLayoutEffect(

518 create: () => mixed,

519 inputs: Array<mixed> | void | null,

@@ -586,26 +571,21 @@ export function useImperativeMethods<T>(

571 // TODO: I've implemented this on top of useEffect because it's almost the

572 // same thing, and it would require an equal amount of code. It doesn't seem

573 // like a common enough use case to justify the additional size.

589 - useEffectImpl(

590 - UpdateEffect,

591 - UnmountMutation | MountLayout,

592 - () => {

593 - if (typeof ref === 'function') {

594 - const refCallback = ref;

595 - const inst = create();

596 - refCallback(inst);

597 - return () => refCallback(null);

598 - } else if (ref !== null && ref !== undefined) {

599 - const refObject = ref;

600 - const inst = create();

601 - refObject.current = inst;

602 - return () => {

603 - refObject.current = null;

604 - };

605 - }

606 - },

607 - nextInputs,

608 - );

574 + useLayoutEffect(() => {

575 + if (typeof ref === 'function') {

576 + const refCallback = ref;

577 + const inst = create();

578 + refCallback(inst);

579 + return () => refCallback(null);

580 + } else if (ref !== null && ref !== undefined) {

581 + const refObject = ref;

582 + const inst = create();

583 + refObject.current = inst;

584 + return () => {

585 + refObject.current = null;

586 + };

587 + }

588 + }, nextInputs);

589 }

590

591 export function useCallback<T>(

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

+1 -176

@@ -19,7 +19,6 @@ let SchedulerTracing;

19 let useState;

20 let useReducer;

21 let useEffect;

22 -let useMutationEffect;

22 let useLayoutEffect;

23 let useCallback;

24 let useMemo;

@@ -68,7 +67,6 @@ describe('ReactHooksWithNoopRenderer', () => {

67 useState = React.useState;

68 useReducer = React.useReducer;

69 useEffect = React.useEffect;

71 - useMutationEffect = React.useMutationEffect;

70 useLayoutEffect = React.useLayoutEffect;

71 useCallback = React.useCallback;

72 useMemo = React.useMemo;

@@ -1260,7 +1258,7 @@ describe('ReactHooksWithNoopRenderer', () => {

1258 });

1259 });

1260

1263 - describe('useMutationEffect and useLayoutEffect', () => {

1261 + describe('useLayoutEffect', () => {

1262 it('fires layout effects after the host has been mutated', () => {

1263 function getCommittedText() {

1264 const children = ReactNoop.getChildren();

@@ -1286,89 +1284,6 @@ describe('ReactHooksWithNoopRenderer', () => {

1284 expect(ReactNoop.getChildren()).toEqual([span(1)]);

1285 });

1286

1289 - it('fires mutation effects before layout effects', () => {

1290 - let committedText = '(empty)';

1291 -

1292 - function Counter(props) {

1293 - useMutationEffect(() => {

1294 - ReactNoop.yield(`Mount mutation [current: ${committedText}]`);

1295 - committedText = props.count + '';

1296 - return () => {

1297 - ReactNoop.yield(`Unmount mutation [current: ${committedText}]`);

1298 - };

1299 - });

1300 - useLayoutEffect(() => {

1301 - ReactNoop.yield(`Mount layout [current: ${committedText}]`);

1302 - return () => {

1303 - ReactNoop.yield(`Unmount layout [current: ${committedText}]`);

1304 - };

1305 - });

1306 - useEffect(() => {

1307 - ReactNoop.yield(`Mount normal [current: ${committedText}]`);

1308 - return () => {

1309 - ReactNoop.yield(`Unmount normal [current: ${committedText}]`);

1310 - };

1311 - });

1312 - return null;

1313 - }

1314 -

1315 - ReactNoop.render(<Counter count={0} />);

1316 - expect(ReactNoop.flush()).toEqual([

1317 - 'Mount mutation [current: (empty)]',

1318 - 'Mount layout [current: 0]',

1319 - ]);

1320 - expect(committedText).toEqual('0');

1321 - flushPassiveEffects();

1322 - expect(ReactNoop.clearYields()).toEqual(['Mount normal [current: 0]']);

1323 -

1324 - // Unmount everything

1325 - ReactNoop.render(null);

1326 - expect(ReactNoop.flush()).toEqual([

1327 - 'Unmount mutation [current: 0]',

1328 - 'Unmount layout [current: 0]',

1329 - 'Unmount normal [current: 0]',

1330 - ]);

1331 - });

1332 -

1333 - it('force flushes passive effects before firing new mutation effects', () => {

1334 - let committedText = '(empty)';

1335 -

1336 - function Counter(props) {

1337 - useMutationEffect(() => {

1338 - ReactNoop.yield(`Mount mutation [current: ${committedText}]`);

1339 - committedText = props.count + '';

1340 - return () => {

1341 - ReactNoop.yield(`Unmount mutation [current: ${committedText}]`);

1342 - };

1343 - });

1344 - useEffect(() => {

1345 - ReactNoop.yield(`Mount normal [current: ${committedText}]`);

1346 - return () => {

1347 - ReactNoop.yield(`Unmount normal [current: ${committedText}]`);

1348 - };

1349 - });

1350 - return null;

1351 - }

1352 -

1353 - ReactNoop.render(<Counter count={0} />);

1354 - expect(ReactNoop.flush()).toEqual(['Mount mutation [current: (empty)]']);

1355 - expect(committedText).toEqual('0');

1356 -

1357 - ReactNoop.render(<Counter count={1} />);

1358 - expect(ReactNoop.flush()).toEqual([

1359 - 'Mount normal [current: 0]',

1360 - 'Unmount mutation [current: 0]',

1361 - 'Mount mutation [current: 0]',

1362 - ]);

1363 - expect(committedText).toEqual('1');

1364 -

1365 - flushPassiveEffects();

1366 - expect(ReactNoop.clearYields()).toEqual([

1367 - 'Unmount normal [current: 1]',

1368 - 'Mount normal [current: 1]',

1369 - ]);

1370 - });

1371 -

1287 it('force flushes passive effects before firing new layout effects', () => {

1288 let committedText = '(empty)';

1289

@@ -1410,96 +1325,6 @@ describe('ReactHooksWithNoopRenderer', () => {

1325 'Mount normal [current: 1]',

1326 ]);

1327 });

1413 -

1414 - it('fires all mutation effects before firing any layout effects', () => {

1415 - let committedA = '(empty)';

1416 - let committedB = '(empty)';

1417 -

1418 - function CounterA(props) {

1419 - useMutationEffect(() => {

1420 - ReactNoop.yield(

1421 - `Mount A mutation [A: ${committedA}, B: ${committedB}]`,

1422 - );

1423 - committedA = props.count + '';

1424 - return () => {

1425 - ReactNoop.yield(

1426 - `Unmount A mutation [A: ${committedA}, B: ${committedB}]`,

1427 - );

1428 - };

1429 - });

1430 - useLayoutEffect(() => {

1431 - ReactNoop.yield(

1432 - `Mount layout A [A: ${committedA}, B: ${committedB}]`,

1433 - );

1434 - return () => {

1435 - ReactNoop.yield(

1436 - `Unmount layout A [A: ${committedA}, B: ${committedB}]`,

1437 - );

1438 - };

1439 - });

1440 - return null;

1441 - }

1442 -

1443 - function CounterB(props) {

1444 - useMutationEffect(() => {

1445 - ReactNoop.yield(

1446 - `Mount B mutation [A: ${committedA}, B: ${committedB}]`,

1447 - );

1448 - committedB = props.count + '';

1449 - return () => {

1450 - ReactNoop.yield(

1451 - `Unmount B mutation [A: ${committedA}, B: ${committedB}]`,

1452 - );

1453 - };

1454 - });

1455 - useLayoutEffect(() => {

1456 - ReactNoop.yield(

1457 - `Mount layout B [A: ${committedA}, B: ${committedB}]`,

1458 - );

1459 - return () => {

1460 - ReactNoop.yield(

1461 - `Unmount layout B [A: ${committedA}, B: ${committedB}]`,

1462 - );

1463 - };

1464 - });

1465 - return null;

1466 - }

1467 -

1468 - ReactNoop.render(

1469 - <React.Fragment>

1470 - <CounterA count={0} />

1471 - <CounterB count={0} />

1472 - </React.Fragment>,

1473 - );

1474 - expect(ReactNoop.flush()).toEqual([

1475 - // All mutation effects fire before all layout effects

1476 - 'Mount A mutation [A: (empty), B: (empty)]',

1477 - 'Mount B mutation [A: 0, B: (empty)]',

1478 - 'Mount layout A [A: 0, B: 0]',

1479 - 'Mount layout B [A: 0, B: 0]',

1480 - ]);

1481 - expect([committedA, committedB]).toEqual(['0', '0']);

1482 -

1483 - ReactNoop.render(

1484 - <React.Fragment>

1485 - <CounterA count={1} />

1486 - <CounterB count={1} />

1487 - </React.Fragment>,

1488 - );

1489 - expect(ReactNoop.flush()).toEqual([

1490 - // Note: This shows that the clean-up function of a layout effect is

1491 - // fired in the same phase as the set-up function of a mutation.

1492 - 'Unmount A mutation [A: 0, B: 0]',

1493 - 'Unmount B mutation [A: 0, B: 0]',

1494 - 'Mount A mutation [A: 0, B: 0]',

1495 - 'Unmount layout A [A: 1, B: 0]',

1496 - 'Mount B mutation [A: 1, B: 0]',

1497 - 'Unmount layout B [A: 1, B: 1]',

1498 - 'Mount layout A [A: 1, B: 1]',

1499 - 'Mount layout B [A: 1, B: 1]',

1500 - ]);

1501 - expect([committedA, committedB]).toEqual(['1', '1']);

1502 - });

1328 });

1329

1330 describe('useCallback', () => {

packages/react/src/React.js

-2

@@ -35,7 +35,6 @@ import {

35 useImperativeMethods,

36 useLayoutEffect,

37 useMemo,

38 - useMutationEffect,

38 useReducer,

39 useRef,

40 useState,

@@ -102,7 +101,6 @@ if (enableHooks) {

101 React.useImperativeMethods = useImperativeMethods;

102 React.useLayoutEffect = useLayoutEffect;

103 React.useMemo = useMemo;

105 - React.useMutationEffect = useMutationEffect;

104 React.useReducer = useReducer;

105 React.useRef = useRef;

106 React.useState = useState;

packages/react/src/ReactHooks.js

-8

@@ -78,14 +78,6 @@ export function useEffect(

78 return dispatcher.useEffect(create, inputs);

79 }

81 -export function useMutationEffect(

82 - create: () => mixed,

83 - inputs: Array<mixed> | void | null,

84 -) {

85 - const dispatcher = resolveDispatcher();

86 - return dispatcher.useMutationEffect(create, inputs);

87 -}

88 -

81 export function useLayoutEffect(

82 create: () => mixed,

83 inputs: Array<mixed> | void | null,