Skip to content

Commit 1454a8b

Browse files
authored
Don't bother comparing constructor when deps are not provided (#14594)
* Don't bother comparing constructor when deps are not provided When no dependencies are passed to an effect hook, what we used to do is compare the effect constructor. If there was no change, then we would skip firing the effect. In practice, this is a useless optimization because the constructor will always be different when you pass an inline closure. And if you don't pass an inline closure, then you can't access any props or state. There are some edge cases where an effect that doesn't close over props or state could be useful, like reference counting the number of mounted components. But those are rare and can be addressed by passing an empty array of dependencies. By removing this "optimization," we can avoid retaining the constructor in the majority of cases where it's a closure that changes on every render. I made corresponding changes to the other hooks that accept dependencies, too (useMemo, useCallback, and useImperativeHandle). * Improve hook dependencies warning It now includes the name of the hook in the message. * Nits
1 parent 71b64d5 commit 1454a8b

File tree

5 files changed

+221
-84
lines changed

5 files changed

+221
-84
lines changed

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

+60-1
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,12 @@
1010
import typeof {Dispatcher as DispatcherType} from 'react-reconciler/src/ReactFiberDispatcher';
1111
import type {ThreadID} from './ReactThreadIDAllocator';
1212
import type {ReactContext} from 'shared/ReactTypes';
13-
import areHookInputsEqual from 'shared/areHookInputsEqual';
1413

1514
import {validateContextBounds} from './ReactPartialRendererContext';
1615

1716
import invariant from 'shared/invariant';
1817
import warning from 'shared/warning';
18+
import is from 'shared/objectIs';
1919

2020
type BasicStateAction<S> = (S => S) | S;
2121
type Dispatch<A> = A => void;
@@ -49,6 +49,9 @@ let renderPhaseUpdates: Map<UpdateQueue<any>, Update<any>> | null = null;
4949
let numberOfReRenders: number = 0;
5050
const RE_RENDER_LIMIT = 25;
5151

52+
// In DEV, this is the name of the currently executing primitive hook
53+
let currentHookNameInDev: ?string;
54+
5255
function resolveCurrentlyRenderingComponent(): Object {
5356
invariant(
5457
currentlyRenderingComponent !== null,
@@ -57,6 +60,48 @@ function resolveCurrentlyRenderingComponent(): Object {
5760
return currentlyRenderingComponent;
5861
}
5962

63+
function areHookInputsEqual(
64+
nextDeps: Array<mixed>,
65+
prevDeps: Array<mixed> | null,
66+
) {
67+
if (prevDeps === null) {
68+
if (__DEV__) {
69+
warning(
70+
false,
71+
'%s received a final argument during this render, but not during ' +
72+
'the previous render. Even though the final argument is optional, ' +
73+
'its type cannot change between renders.',
74+
currentHookNameInDev,
75+
);
76+
}
77+
return false;
78+
}
79+
80+
if (__DEV__) {
81+
// Don't bother comparing lengths in prod because these arrays should be
82+
// passed inline.
83+
if (nextDeps.length !== prevDeps.length) {
84+
warning(
85+
false,
86+
'The final argument passed to %s changed size between renders. The ' +
87+
'order and size of this array must remain constant.\n\n' +
88+
'Previous: %s\n' +
89+
'Incoming: %s',
90+
currentHookNameInDev,
91+
`[${nextDeps.join(', ')}]`,
92+
`[${prevDeps.join(', ')}]`,
93+
);
94+
}
95+
}
96+
for (let i = 0; i < prevDeps.length && i < nextDeps.length; i++) {
97+
if (is(nextDeps[i], prevDeps[i])) {
98+
continue;
99+
}
100+
return false;
101+
}
102+
return true;
103+
}
104+
60105
function createHook(): Hook {
61106
return {
62107
memoizedState: null,
@@ -153,6 +198,9 @@ function useContext<T>(
153198
context: ReactContext<T>,
154199
observedBits: void | number | boolean,
155200
): T {
201+
if (__DEV__) {
202+
currentHookNameInDev = 'useContext';
203+
}
156204
resolveCurrentlyRenderingComponent();
157205
let threadID = currentThreadID;
158206
validateContextBounds(context, threadID);
@@ -166,6 +214,9 @@ function basicStateReducer<S>(state: S, action: BasicStateAction<S>): S {
166214
export function useState<S>(
167215
initialState: (() => S) | S,
168216
): [S, Dispatch<BasicStateAction<S>>] {
217+
if (__DEV__) {
218+
currentHookNameInDev = 'useState';
219+
}
169220
return useReducer(
170221
basicStateReducer,
171222
// useReducer has a special case to support lazy useState initializers
@@ -178,6 +229,11 @@ export function useReducer<S, A>(
178229
initialState: S,
179230
initialAction: A | void | null,
180231
): [S, Dispatch<A>] {
232+
if (__DEV__) {
233+
if (reducer !== basicStateReducer) {
234+
currentHookNameInDev = 'useReducer';
235+
}
236+
}
181237
currentlyRenderingComponent = resolveCurrentlyRenderingComponent();
182238
workInProgressHook = createWorkInProgressHook();
183239
if (isReRender) {
@@ -276,6 +332,9 @@ export function useLayoutEffect(
276332
create: () => mixed,
277333
inputs: Array<mixed> | void | null,
278334
) {
335+
if (__DEV__) {
336+
currentHookNameInDev = 'useLayoutEffect';
337+
}
279338
warning(
280339
false,
281340
'useLayoutEffect does nothing on the server, because its effect cannot ' +

0 commit comments

Comments
 (0)