Skip updating effect tag when skipping effect
For example, if you have `useEffect(..., [])`, there's no need to set .effectTag to `Update | Passive` on updates.
Sophie Alpert committed
Oct 16, 2018 at 13:02 UTC
5fc84efacce66272928815e8a6c83a341af9160e
7 files changed
+64
-39
packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js
+4
-4
@@ -24,7 +24,7 @@ let useContext;
24
let useCallback;
25
let useMemo;
26
let useRef;
27
-let useAPI;
27
+let useImperativeMethods;
28
let useMutationEffect;
29
let useLayoutEffect;
30
let forwardRef;
@@ -49,7 +49,7 @@ function initModules() {
49
useCallback = React.useCallback;
50
useMemo = React.useMemo;
51
useRef = React.useRef;
52
- useAPI = React.useAPI;
52
+ useImperativeMethods = React.useImperativeMethods;
53
useMutationEffect = React.useMutationEffect;
54
useLayoutEffect = React.useLayoutEffect;
55
forwardRef = React.forwardRef;
@@ -518,10 +518,10 @@ describe('ReactDOMServerHooks', () => {
518
});
519
});
520
521
- describe('useAPI', () => {
521
+ describe('useImperativeMethods', () => {
522
it('should not be invoked on the server', async () => {
523
function Counter(props, ref) {
524
- useAPI(ref, () => {
524
+ useImperativeMethods(ref, () => {
525
throw new Error('should not be invoked');
526
});
527
return <Text text={props.label + ': ' + ref.current} />;
packages/react-dom/src/server/ReactPartialRendererHooks.js
+2
-2
@@ -365,8 +365,8 @@ export const Dispatcher = {
365
useState,
366
useMutationEffect,
367
useLayoutEffect,
368
- // useAPI is not run in the server environment
369
- useAPI: noop,
368
+ // useImperativeMethods is not run in the server environment
369
+ useImperativeMethods: noop,
370
// Callbacks are not run in the server environment.
371
useCallback: noop,
372
// Effects are not run in the server environment.
packages/react-reconciler/src/ReactFiberDispatcher.js
+2
-2
@@ -9,10 +9,10 @@
9
10
import {readContext} from './ReactFiberNewContext';
11
import {
12
- useAPI,
12
useCallback,
13
useContext,
14
useEffect,
15
+ useImperativeMethods,
16
useLayoutEffect,
17
useMemo,
18
useMutationEffect,
@@ -23,10 +23,10 @@ import {
23
24
export const Dispatcher = {
25
readContext,
26
- useAPI,
26
useCallback,
27
useContext,
28
useEffect,
29
+ useImperativeMethods,
30
useLayoutEffect,
31
useMemo,
32
useMutationEffect,
packages/react-reconciler/src/ReactFiberHooks.js
+14
-15
@@ -179,9 +179,7 @@ export function finishHooks(
179
180
renderedWork.memoizedState = firstWorkInProgressHook;
181
renderedWork.expirationTime = remainingExpirationTime;
182
- if (componentUpdateQueue !== null) {
183
- renderedWork.updateQueue = (componentUpdateQueue: any);
184
- }
182
+ renderedWork.updateQueue = (componentUpdateQueue: any);
183
184
const didRenderTooFewHooks =
185
currentHook !== null && currentHook.next !== null;
@@ -576,26 +574,27 @@ function useEffectImpl(fiberEffectTag, hookEffectTag, create, inputs): void {
574
currentlyRenderingFiber = resolveCurrentlyRenderingFiber();
575
workInProgressHook = createWorkInProgressHook();
576
579
- let nextEffect;
577
let nextInputs = inputs !== undefined && inputs !== null ? inputs : [create];
578
+ let destroy = null;
579
if (currentHook !== null) {
580
const prevEffect = currentHook.memoizedState;
583
- const prevInputs = prevEffect.inputs;
584
- nextEffect = pushEffect(
585
- inputsAreEqual(nextInputs, prevInputs) ? NoHookEffect : hookEffectTag,
586
- create,
587
- prevEffect.destroy,
588
- nextInputs,
589
- );
590
- } else {
591
- nextEffect = pushEffect(hookEffectTag, create, null, nextInputs);
581
+ destroy = prevEffect.destroy;
582
+ if (inputsAreEqual(nextInputs, prevEffect.inputs)) {
583
+ pushEffect(NoHookEffect, create, destroy, nextInputs);
584
+ return;
585
+ }
586
}
587
594
- workInProgressHook.memoizedState = nextEffect;
588
currentlyRenderingFiber.effectTag |= fiberEffectTag;
589
+ workInProgressHook.memoizedState = pushEffect(
590
+ hookEffectTag,
591
+ create,
592
+ destroy,
593
+ nextInputs,
594
+ );
595
}
596
598
-export function useAPI<T>(
597
+export function useImperativeMethods<T>(
598
ref: {current: T | null} | ((inst: T | null) => mixed) | null | void,
599
create: () => T,
600
inputs: Array<mixed> | void | null,
packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
+38
-12
@@ -24,7 +24,7 @@ let useLayoutEffect;
24
let useCallback;
25
let useMemo;
26
let useRef;
27
-let useAPI;
27
+let useImperativeMethods;
28
let forwardRef;
29
let flushPassiveEffects;
30
let memo;
@@ -70,7 +70,7 @@ describe('ReactHooks', () => {
70
useCallback = React.useCallback;
71
useMemo = React.useMemo;
72
useRef = React.useRef;
73
- useAPI = React.useAPI;
73
+ useImperativeMethods = React.useImperativeMethods;
74
forwardRef = React.forwardRef;
75
memo = React.memo;
76
});
@@ -87,7 +87,7 @@ describe('ReactHooks', () => {
87
it('resumes after an interruption', () => {
88
function Counter(props, ref) {
89
const [count, updateCount] = useState(0);
90
- useAPI(ref, () => ({updateCount}));
90
+ useImperativeMethods(ref, () => ({updateCount}));
91
return <Text text={props.label + ': ' + count} />;
92
}
93
Counter = forwardRef(Counter);
@@ -171,7 +171,7 @@ describe('ReactHooks', () => {
171
it('simple mount and update', () => {
172
function Counter(props, ref) {
173
const [count, updateCount] = useState(0);
174
- useAPI(ref, () => ({updateCount}));
174
+ useImperativeMethods(ref, () => ({updateCount}));
175
return <Text text={'Count: ' + count} />;
176
}
177
Counter = forwardRef(Counter);
@@ -195,7 +195,7 @@ describe('ReactHooks', () => {
195
ReactNoop.yield('getInitialState');
196
return props.initialState;
197
});
198
- useAPI(ref, () => ({updateCount}));
198
+ useImperativeMethods(ref, () => ({updateCount}));
199
return <Text text={'Count: ' + count} />;
200
}
201
Counter = forwardRef(Counter);
@@ -213,7 +213,7 @@ describe('ReactHooks', () => {
213
function Counter(props, ref) {
214
const [count, updateCount] = useState(0);
215
const [label, updateLabel] = useState('Count');
216
- useAPI(ref, () => ({updateCount, updateLabel}));
216
+ useImperativeMethods(ref, () => ({updateCount, updateLabel}));
217
return <Text text={label + ': ' + count} />;
218
}
219
Counter = forwardRef(Counter);
@@ -232,7 +232,7 @@ describe('ReactHooks', () => {
232
it('callbacks', () => {
233
function Counter(props, ref) {
234
const [count, updateCount] = useState(0);
235
- useAPI(ref, () => ({updateCount}));
235
+ useImperativeMethods(ref, () => ({updateCount}));
236
return <Text text={'Count: ' + count} />;
237
}
238
Counter = forwardRef(Counter);
@@ -264,7 +264,7 @@ describe('ReactHooks', () => {
264
it('does not fire callbacks more than once when rebasing', () => {
265
function Counter(props, ref) {
266
const [count, updateCount] = useState(0);
267
- useAPI(ref, () => ({updateCount}));
267
+ useImperativeMethods(ref, () => ({updateCount}));
268
return <Text text={'Count: ' + count} />;
269
}
270
Counter = forwardRef(Counter);
@@ -507,7 +507,7 @@ describe('ReactHooks', () => {
507
function Counter({row: newRow}, ref) {
508
let [reducer, setReducer] = useState(() => reducerA);
509
let [count, dispatch] = useReducer(reducer, 0);
510
- useAPI(ref, () => ({dispatch}));
510
+ useImperativeMethods(ref, () => ({dispatch}));
511
if (count < 20) {
512
dispatch('increment');
513
// Swap reducers each time we increment
@@ -568,7 +568,7 @@ describe('ReactHooks', () => {
568
569
function Counter(props, ref) {
570
const [count, dispatch] = useReducer(reducer, 0);
571
- useAPI(ref, () => ({dispatch}));
571
+ useImperativeMethods(ref, () => ({dispatch}));
572
return <Text text={'Count: ' + count} />;
573
}
574
Counter = forwardRef(Counter);
@@ -609,7 +609,7 @@ describe('ReactHooks', () => {
609
610
function Counter(props, ref) {
611
const [count, dispatch] = useReducer(reducer, 0, initialAction);
612
- useAPI(ref, () => ({dispatch}));
612
+ useImperativeMethods(ref, () => ({dispatch}));
613
return <Text text={'Count: ' + count} />;
614
}
615
Counter = forwardRef(Counter);
@@ -910,7 +910,6 @@ describe('ReactHooks', () => {
910
expect(ReactNoop.flush()).toEqual(['Will set count to 1', 'Count: 2']);
911
expect(ReactNoop.getChildren()).toEqual([span('Count: 2')]);
912
913
- flushPassiveEffects();
913
expect(onInteractionScheduledWorkCompleted).toHaveBeenCalledTimes(1);
914
expect(onWorkCanceled).toHaveBeenCalledTimes(0);
915
});
@@ -1019,6 +1018,33 @@ describe('ReactHooks', () => {
1018
expect(ReactNoop.getChildren()).toEqual([]);
1019
});
1020
1021
+ it('unmounts on deletion after skipped effect', () => {
1022
+ function Counter(props) {
1023
+ useEffect(() => {
1024
+ ReactNoop.yield(`Did create [${props.count}]`);
1025
+ return () => {
1026
+ ReactNoop.yield(`Did destroy [${props.count}]`);
1027
+ };
1028
+ }, []);
1029
+ return <Text text={'Count: ' + props.count} />;
1030
+ }
1031
+ ReactNoop.render(<Counter count={0} />);
1032
+ expect(ReactNoop.flush()).toEqual(['Count: 0']);
1033
+ expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
1034
+ flushPassiveEffects();
1035
+ expect(ReactNoop.clearYields()).toEqual(['Did create [0]']);
1036
+
1037
+ ReactNoop.render(<Counter count={1} />);
1038
+ expect(ReactNoop.flush()).toEqual(['Count: 1']);
1039
+ expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
1040
+ flushPassiveEffects();
1041
+ expect(ReactNoop.clearYields()).toEqual(null);
1042
+
1043
+ ReactNoop.render(null);
1044
+ expect(ReactNoop.flush()).toEqual(['Did destroy [0]']);
1045
+ expect(ReactNoop.getChildren()).toEqual([]);
1046
+ });
1047
+
1048
it('skips effect if constructor has not changed', () => {
1049
function effect() {
1050
ReactNoop.yield(`Did mount`);
packages/react/src/React.js
+2
-2
@@ -29,10 +29,10 @@ import {lazy} from './ReactLazy';
29
import forwardRef from './forwardRef';
30
import memo from './memo';
31
import {
32
- useAPI,
32
useCallback,
33
useContext,
34
useEffect,
35
+ useImperativeMethods,
36
useLayoutEffect,
37
useMemo,
38
useMutationEffect,
@@ -89,10 +89,10 @@ if (enableStableConcurrentModeAPIs) {
89
}
90
91
if (enableHooks) {
92
- React.useAPI = useAPI;
92
React.useCallback = useCallback;
93
React.useContext = useContext;
94
React.useEffect = useEffect;
95
+ React.useImperativeMethods = useImperativeMethods;
96
React.useLayoutEffect = useLayoutEffect;
97
React.useMemo = useMemo;
98
React.useMutationEffect = useMutationEffect;
packages/react/src/ReactHooks.js
+2
-2
@@ -110,11 +110,11 @@ export function useMemo(
110
return dispatcher.useMemo(create, inputs);
111
}
112
113
-export function useAPI<T>(
113
+export function useImperativeMethods<T>(
114
ref: {current: T | null} | ((inst: T | null) => mixed) | null | void,
115
create: () => T,
116
inputs: Array<mixed> | void | null,
117
): void {
118
const dispatcher = resolveDispatcher();
119
- return dispatcher.useAPI(ref, create, inputs);
119
+ return dispatcher.useImperativeMethods(ref, create, inputs);
120
}