Warn if number of hooks increases (#14591)
Eventually, we'll likely support adding hooks to the end (to enable progressive enhancement), but let's warn until we figure out how it should work.
Andrew Clark committed
Jan 16, 2019 at 17:25 UTC
71b64d521180f443afa7f98283b1807d69b3acbc
2 files changed
+28
-16
packages/react-reconciler/src/ReactFiberHooks.js
+18
-14
@@ -34,6 +34,8 @@ import {
34
} from './ReactFiberScheduler';
35
36
import invariant from 'shared/invariant';
37
+import warning from 'shared/warning';
38
+import getComponentName from 'shared/getComponentName';
39
import areHookInputsEqual from 'shared/areHookInputsEqual';
40
import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork';
41
@@ -104,8 +106,6 @@ let componentUpdateQueue: FunctionComponentUpdateQueue | null = null;
106
// map of queue -> render-phase updates, which are discarded once the component
107
// completes without re-rendering.
108
107
-// Whether the work-in-progress hook is a re-rendered hook
108
-let isReRender: boolean = false;
109
// Whether an update was scheduled during the currently executing render pass.
110
let didScheduleRenderPhaseUpdate: boolean = false;
111
// Lazily created map of render-phase updates
@@ -147,7 +147,6 @@ export function renderWithHooks(
147
// remainingExpirationTime = NoWork;
148
// componentUpdateQueue = null;
149
150
- // isReRender = false;
150
// didScheduleRenderPhaseUpdate = false;
151
// renderPhaseUpdates = null;
152
// numberOfReRenders = -1;
@@ -163,6 +162,21 @@ export function renderWithHooks(
162
componentUpdateQueue = null;
163
164
children = Component(props, refOrContext);
165
+
166
+ if (__DEV__) {
167
+ if (
168
+ current !== null &&
169
+ workInProgressHook !== null &&
170
+ currentHook === null
171
+ ) {
172
+ warning(
173
+ false,
174
+ '%s: Rendered more hooks than during the previous render. This is ' +
175
+ 'not currently supported and may lead to unexpected behavior.',
176
+ getComponentName(Component),
177
+ );
178
+ }
179
+ }
180
} while (didScheduleRenderPhaseUpdate);
181
182
renderPhaseUpdates = null;
@@ -188,9 +202,6 @@ export function renderWithHooks(
202
remainingExpirationTime = NoWork;
203
componentUpdateQueue = null;
204
191
- // Always set during createWorkInProgress
192
- // isReRender = false;
193
-
205
// These were reset above
206
// didScheduleRenderPhaseUpdate = false;
207
// renderPhaseUpdates = null;
@@ -236,9 +247,6 @@ export function resetHooks(): void {
247
remainingExpirationTime = NoWork;
248
componentUpdateQueue = null;
249
239
- // Always set during createWorkInProgress
240
- // isReRender = false;
241
-
250
didScheduleRenderPhaseUpdate = false;
251
renderPhaseUpdates = null;
252
numberOfReRenders = -1;
@@ -272,7 +280,6 @@ function createWorkInProgressHook(): Hook {
280
if (workInProgressHook === null) {
281
// This is the first hook in the list
282
if (firstWorkInProgressHook === null) {
275
- isReRender = false;
283
currentHook = firstCurrentHook;
284
if (currentHook === null) {
285
// This is a newly mounted hook
@@ -284,13 +291,11 @@ function createWorkInProgressHook(): Hook {
291
firstWorkInProgressHook = workInProgressHook;
292
} else {
293
// There's already a work-in-progress. Reuse it.
287
- isReRender = true;
294
currentHook = firstCurrentHook;
295
workInProgressHook = firstWorkInProgressHook;
296
}
297
} else {
298
if (workInProgressHook.next === null) {
293
- isReRender = false;
299
let hook;
300
if (currentHook === null) {
301
// This is a newly mounted hook
@@ -309,7 +314,6 @@ function createWorkInProgressHook(): Hook {
314
workInProgressHook = workInProgressHook.next = hook;
315
} else {
316
// There's already a work-in-progress. Reuse it.
312
- isReRender = true;
317
workInProgressHook = workInProgressHook.next;
318
currentHook = currentHook !== null ? currentHook.next : null;
319
}
@@ -357,7 +361,7 @@ export function useReducer<S, A>(
361
let queue: UpdateQueue<S, A> | null = (workInProgressHook.queue: any);
362
if (queue !== null) {
363
// Already have a queue, so this is an update.
360
- if (isReRender) {
364
+ if (numberOfReRenders > 0) {
365
// This is a re-render. Apply the new render phase updates to the previous
366
// work-in-progress hook.
367
const dispatch: Dispatch<A> = (queue.dispatch: any);
packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js
+10
-2
@@ -1634,7 +1634,11 @@ describe('ReactHooksWithNoopRenderer', () => {
1634
]);
1635
1636
ReactNoop.render(<App loadC={true} />);
1637
- expect(ReactNoop.flush()).toEqual(['A: 2, B: 3, C: 0']);
1637
+ expect(() => {
1638
+ expect(ReactNoop.flush()).toEqual(['A: 2, B: 3, C: 0']);
1639
+ }).toWarnDev([
1640
+ 'App: Rendered more hooks than during the previous render',
1641
+ ]);
1642
expect(ReactNoop.getChildren()).toEqual([span('A: 2, B: 3, C: 0')]);
1643
1644
updateC(4);
@@ -1708,7 +1712,11 @@ describe('ReactHooksWithNoopRenderer', () => {
1712
expect(ReactNoop.clearYields()).toEqual(['Mount A']);
1713
1714
ReactNoop.render(<App showMore={true} />);
1711
- expect(ReactNoop.flush()).toEqual([]);
1715
+ expect(() => {
1716
+ expect(ReactNoop.flush()).toEqual([]);
1717
+ }).toWarnDev([
1718
+ 'App: Rendered more hooks than during the previous render',
1719
+ ]);
1720
flushPassiveEffects();
1721
expect(ReactNoop.clearYields()).toEqual(['Mount B']);
1722