Skip to content

Commit 71b64d5

Browse files
authored
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.
1 parent 790c8ef commit 71b64d5

2 files changed

Lines changed: 28 additions & 16 deletions

File tree

‎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
importinvariantfrom'shared/invariant';
37+
importwarningfrom'shared/warning';
38+
importgetComponentNamefrom'shared/getComponentName';
3739
importareHookInputsEqualfrom'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
lethook;
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
letqueue: 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
constdispatch: 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(<ApploadC={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(<AppshowMore={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)