Skip to content

Commit e3e4f78

Browse files
committed
Warn if number of hooks increases
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.
1 parent 6c9ae30 commit e3e4f78

File tree

2 files changed

+28
-16
lines changed

2 files changed

+28
-16
lines changed

packages/react-reconciler/src/ReactFiberHooks.js

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ import {
3434
} from './ReactFiberScheduler';
3535

3636
import invariant from 'shared/invariant';
37+
import warning from 'shared/warning';
38+
import getComponentName from 'shared/getComponentName';
3739
import areHookInputsEqual from 'shared/areHookInputsEqual';
3840
import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork';
3941

@@ -104,8 +106,6 @@ let componentUpdateQueue: FunctionComponentUpdateQueue | null = null;
104106
// map of queue -> render-phase updates, which are discarded once the component
105107
// completes without re-rendering.
106108

107-
// Whether the work-in-progress hook is a re-rendered hook
108-
let isReRender: boolean = false;
109109
// Whether an update was scheduled during the currently executing render pass.
110110
let didScheduleRenderPhaseUpdate: boolean = false;
111111
// Lazily created map of render-phase updates
@@ -147,7 +147,6 @@ export function renderWithHooks(
147147
// remainingExpirationTime = NoWork;
148148
// componentUpdateQueue = null;
149149

150-
// isReRender = false;
151150
// didScheduleRenderPhaseUpdate = false;
152151
// renderPhaseUpdates = null;
153152
// numberOfReRenders = -1;
@@ -163,6 +162,21 @@ export function renderWithHooks(
163162
componentUpdateQueue = null;
164163

165164
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+
}
166180
} while (didScheduleRenderPhaseUpdate);
167181

168182
renderPhaseUpdates = null;
@@ -188,9 +202,6 @@ export function renderWithHooks(
188202
remainingExpirationTime = NoWork;
189203
componentUpdateQueue = null;
190204

191-
// Always set during createWorkInProgress
192-
// isReRender = false;
193-
194205
// These were reset above
195206
// didScheduleRenderPhaseUpdate = false;
196207
// renderPhaseUpdates = null;
@@ -236,9 +247,6 @@ export function resetHooks(): void {
236247
remainingExpirationTime = NoWork;
237248
componentUpdateQueue = null;
238249

239-
// Always set during createWorkInProgress
240-
// isReRender = false;
241-
242250
didScheduleRenderPhaseUpdate = false;
243251
renderPhaseUpdates = null;
244252
numberOfReRenders = -1;
@@ -272,7 +280,6 @@ function createWorkInProgressHook(): Hook {
272280
if (workInProgressHook === null) {
273281
// This is the first hook in the list
274282
if (firstWorkInProgressHook === null) {
275-
isReRender = false;
276283
currentHook = firstCurrentHook;
277284
if (currentHook === null) {
278285
// This is a newly mounted hook
@@ -284,13 +291,11 @@ function createWorkInProgressHook(): Hook {
284291
firstWorkInProgressHook = workInProgressHook;
285292
} else {
286293
// There's already a work-in-progress. Reuse it.
287-
isReRender = true;
288294
currentHook = firstCurrentHook;
289295
workInProgressHook = firstWorkInProgressHook;
290296
}
291297
} else {
292298
if (workInProgressHook.next === null) {
293-
isReRender = false;
294299
let hook;
295300
if (currentHook === null) {
296301
// This is a newly mounted hook
@@ -309,7 +314,6 @@ function createWorkInProgressHook(): Hook {
309314
workInProgressHook = workInProgressHook.next = hook;
310315
} else {
311316
// There's already a work-in-progress. Reuse it.
312-
isReRender = true;
313317
workInProgressHook = workInProgressHook.next;
314318
currentHook = currentHook !== null ? currentHook.next : null;
315319
}
@@ -357,7 +361,7 @@ export function useReducer<S, A>(
357361
let queue: UpdateQueue<S, A> | null = (workInProgressHook.queue: any);
358362
if (queue !== null) {
359363
// Already have a queue, so this is an update.
360-
if (isReRender) {
364+
if (numberOfReRenders > 0) {
361365
// This is a re-render. Apply the new render phase updates to the previous
362366
// work-in-progress hook.
363367
const dispatch: Dispatch<A> = (queue.dispatch: any);

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1634,7 +1634,11 @@ describe('ReactHooksWithNoopRenderer', () => {
16341634
]);
16351635

16361636
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+
]);
16381642
expect(ReactNoop.getChildren()).toEqual([span('A: 2, B: 3, C: 0')]);
16391643

16401644
updateC(4);
@@ -1708,7 +1712,11 @@ describe('ReactHooksWithNoopRenderer', () => {
17081712
expect(ReactNoop.clearYields()).toEqual(['Mount A']);
17091713

17101714
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+
]);
17121720
flushPassiveEffects();
17131721
expect(ReactNoop.clearYields()).toEqual(['Mount B']);
17141722

0 commit comments

Comments
 (0)