Skip to content

Commit d076142

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 695f36f commit d076142

File tree

2 files changed

+30
-18
lines changed

2 files changed

+30
-18
lines changed

packages/react-reconciler/src/ReactFiberHooks.js

+20-16
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ import {
3636
} from './ReactFiberScheduler';
3737

3838
import invariant from 'shared/invariant';
39+
import warning from 'shared/warning';
40+
import getComponentName from 'shared/getComponentName';
3941
import areHookInputsEqual from 'shared/areHookInputsEqual';
4042

4143
type Update<S, A> = {
@@ -105,8 +107,6 @@ let componentUpdateQueue: FunctionComponentUpdateQueue | null = null;
105107
// map of queue -> render-phase updates, which are discarded once the component
106108
// completes without re-rendering.
107109

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

151-
// isReRender = false;
152151
// didScheduleRenderPhaseUpdate = false;
153152
// renderPhaseUpdates = null;
154153
// numberOfReRenders = -1;
155154

155+
const renderedWork: Fiber = (currentlyRenderingFiber: any);
156+
156157
let children;
157158
do {
158159
didScheduleRenderPhaseUpdate = false;
@@ -164,13 +165,26 @@ export function renderWithHooks(
164165
componentUpdateQueue = null;
165166

166167
children = Component(props, refOrContext);
168+
169+
if (__DEV__) {
170+
if (
171+
current !== null &&
172+
workInProgressHook !== null &&
173+
currentHook === null
174+
) {
175+
warning(
176+
false,
177+
'%s: Rendered more hooks than during the previous render. This is ' +
178+
'not currently supported and may lead to unexpected behavior.',
179+
getComponentName(Component),
180+
);
181+
}
182+
}
167183
} while (didScheduleRenderPhaseUpdate);
168184

169185
renderPhaseUpdates = null;
170186
numberOfReRenders = -1;
171187

172-
const renderedWork: Fiber = (currentlyRenderingFiber: any);
173-
174188
if (
175189
current !== null &&
176190
(renderedWork.effectTag & PerformedWork) === NoEffect
@@ -201,9 +215,6 @@ export function renderWithHooks(
201215
remainingExpirationTime = NoWork;
202216
componentUpdateQueue = null;
203217

204-
// Always set during createWorkInProgress
205-
// isReRender = false;
206-
207218
// These were reset above
208219
// didScheduleRenderPhaseUpdate = false;
209220
// renderPhaseUpdates = null;
@@ -237,9 +248,6 @@ export function resetHooks(): void {
237248
remainingExpirationTime = NoWork;
238249
componentUpdateQueue = null;
239250

240-
// Always set during createWorkInProgress
241-
// isReRender = false;
242-
243251
didScheduleRenderPhaseUpdate = false;
244252
renderPhaseUpdates = null;
245253
numberOfReRenders = -1;
@@ -273,7 +281,6 @@ function createWorkInProgressHook(): Hook {
273281
if (workInProgressHook === null) {
274282
// This is the first hook in the list
275283
if (firstWorkInProgressHook === null) {
276-
isReRender = false;
277284
currentHook = firstCurrentHook;
278285
if (currentHook === null) {
279286
// This is a newly mounted hook
@@ -285,13 +292,11 @@ function createWorkInProgressHook(): Hook {
285292
firstWorkInProgressHook = workInProgressHook;
286293
} else {
287294
// There's already a work-in-progress. Reuse it.
288-
isReRender = true;
289295
currentHook = firstCurrentHook;
290296
workInProgressHook = firstWorkInProgressHook;
291297
}
292298
} else {
293299
if (workInProgressHook.next === null) {
294-
isReRender = false;
295300
let hook;
296301
if (currentHook === null) {
297302
// This is a newly mounted hook
@@ -310,7 +315,6 @@ function createWorkInProgressHook(): Hook {
310315
workInProgressHook = workInProgressHook.next = hook;
311316
} else {
312317
// There's already a work-in-progress. Reuse it.
313-
isReRender = true;
314318
workInProgressHook = workInProgressHook.next;
315319
currentHook = currentHook !== null ? currentHook.next : null;
316320
}
@@ -358,7 +362,7 @@ export function useReducer<S, A>(
358362
let queue: UpdateQueue<S, A> | null = (workInProgressHook.queue: any);
359363
if (queue !== null) {
360364
// Already have a queue, so this is an update.
361-
if (isReRender) {
365+
if (numberOfReRenders > 0) {
362366
// This is a re-render. Apply the new render phase updates to the previous
363367
// work-in-progress hook.
364368
const dispatch: Dispatch<A> = (queue.dispatch: any);

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

+10-2
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)