Skip to content

Commit 3f1f3dc

Browse files
anushreesubramanigaearon
authored andcommitted
Deduplicated many warnings (#11140) (#11216)
* Deduplicated many warnings (#11140) * Deduplicated the following warnings: 1. Can only update a mounted or mounting component. This usually means you called setState, replaceState, or forceUpdate on an unmounted component. This is a no-op 2. %s.componentWillReceiveProps(): Assigning directly to this.state is deprecated (except inside a component's constructor). Use setState instead.' 3. An update (setState, replaceState, or forceUpdate) was scheduled from inside an update function. Update functions should be pure, with zero side-effects. Consider using componentDidUpdate or a callback. 4. setState(...): Cannot call setState() inside getChildContext() * Code review changes made for #11140 * Minor style fix * Test deduplication for noop updates in server renderer * Test deduplication for cWRP warning * Test deduplication for cWM setState warning * Test deduplication for unnmounted setState warning * Fix existing Flow typing * Test deduplication for invalid updates * Test deduplication of update-in-updater warning
1 parent 7f10fae commit 3f1f3dc

10 files changed

+96
-24
lines changed

packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,10 @@ describe('ReactComponentLifeCycle', () => {
221221
'unmounted component. This is a no-op.\n\nPlease check the code for the ' +
222222
'StatefulComponent component.',
223223
);
224+
225+
// Check deduplication
226+
ReactTestUtils.renderIntoDocument(<StatefulComponent />);
227+
expectDev(console.error.calls.count()).toBe(1);
224228
});
225229

226230
it('should correctly determine if a component is mounted', () => {

packages/react-dom/src/__tests__/ReactCompositeComponent-test.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,9 @@ describe('ReactCompositeComponent', () => {
252252
'component. This is a no-op.\n\nPlease check the code for the ' +
253253
'Component component.',
254254
);
255+
256+
instance.forceUpdate();
257+
expectDev(console.error.calls.count()).toBe(1);
255258
});
256259

257260
it('should warn about `setState` on unmounted components', () => {
@@ -391,6 +394,11 @@ describe('ReactCompositeComponent', () => {
391394
expect(instance).toBe(instance2);
392395
expect(renderedState).toBe(1);
393396
expect(instance2.state.value).toBe(1);
397+
398+
// Test deduplication
399+
ReactDOM.unmountComponentAtNode(container);
400+
ReactDOM.render(<Component prop={123} />, container);
401+
expectDev(console.error.calls.count()).toBe(1);
394402
});
395403

396404
it('should warn about `setState` in getChildContext', () => {
@@ -424,6 +432,11 @@ describe('ReactCompositeComponent', () => {
424432
expectDev(console.error.calls.argsFor(0)[0]).toBe(
425433
'Warning: setState(...): Cannot call setState() inside getChildContext()',
426434
);
435+
436+
// Test deduplication
437+
ReactDOM.unmountComponentAtNode(container);
438+
ReactDOM.render(<Component />, container);
439+
expectDev(console.error.calls.count()).toBe(1);
427440
});
428441

429442
it('should cleanup even if render() fatals', () => {

packages/react-dom/src/__tests__/ReactCompositeComponentState-test.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,10 @@ describe('ReactCompositeComponent-state', () => {
421421
"this.state is deprecated (except inside a component's constructor). " +
422422
'Use setState instead.',
423423
);
424+
425+
// Check deduplication
426+
ReactDOM.render(<Test />, container);
427+
expect(console.error.calls.count()).toEqual(1);
424428
});
425429

426430
it('should treat assigning to this.state inside cWM as a replaceState, with a warning', () => {

packages/react-dom/src/__tests__/ReactServerRendering-test.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -656,8 +656,11 @@ describe('ReactDOMServer', () => {
656656
' This usually means you called setState() outside componentWillMount() on the server.' +
657657
' This is a no-op.\n\nPlease check the code for the Foo component.',
658658
);
659+
659660
var markup = ReactDOMServer.renderToStaticMarkup(<Foo />);
660661
expect(markup).toBe('<div>hello</div>');
662+
jest.runOnlyPendingTimers();
663+
expectDev(console.error.calls.count()).toBe(1);
661664
});
662665

663666
it('warns with a no-op when an async forceUpdate is triggered', () => {

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ var didWarnDefaultChecked = false;
118118
var didWarnDefaultSelectValue = false;
119119
var didWarnDefaultTextareaValue = false;
120120
var didWarnInvalidOptionChildren = false;
121+
var didWarnAboutNoopUpdateForComponent = {};
121122
var valuePropNames = ['value', 'defaultValue'];
122123
var newlineEatingTags = {
123124
listing: true,
@@ -181,15 +182,23 @@ function warnNoop(
181182
) {
182183
if (__DEV__) {
183184
var constructor = publicInstance.constructor;
185+
const componentName =
186+
(constructor && getComponentName(constructor)) || 'ReactClass';
187+
const warningKey = `${componentName}.${callerName}`;
188+
if (didWarnAboutNoopUpdateForComponent[warningKey]) {
189+
return;
190+
}
191+
184192
warning(
185193
false,
186194
'%s(...): Can only update a mounting component. ' +
187195
'This usually means you called %s() outside componentWillMount() on the server. ' +
188196
'This is a no-op.\n\nPlease check the code for the %s component.',
189197
callerName,
190198
callerName,
191-
(constructor && getComponentName(constructor)) || 'ReactClass',
199+
componentName,
192200
);
201+
didWarnAboutNoopUpdateForComponent[warningKey] = true;
193202
}
194203
}
195204

packages/react-reconciler/src/ReactFiberClassComponent.js

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ if (__DEV__) {
4141
var warning = require('fbjs/lib/warning');
4242

4343
var {startPhaseTimer, stopPhaseTimer} = require('./ReactDebugFiberPerf');
44+
var didWarnAboutStateAssignmentForComponent = {};
4445

4546
var warnOnInvalidCallback = function(callback: mixed, callerName: string) {
4647
warning(
@@ -393,13 +394,17 @@ module.exports = function(
393394

394395
if (instance.state !== oldState) {
395396
if (__DEV__) {
396-
warning(
397-
false,
398-
'%s.componentWillReceiveProps(): Assigning directly to ' +
399-
"this.state is deprecated (except inside a component's " +
400-
'constructor). Use setState instead.',
401-
getComponentName(workInProgress),
402-
);
397+
const componentName = getComponentName(workInProgress) || 'Component';
398+
if (!didWarnAboutStateAssignmentForComponent[componentName]) {
399+
warning(
400+
false,
401+
'%s.componentWillReceiveProps(): Assigning directly to ' +
402+
"this.state is deprecated (except inside a component's " +
403+
'constructor). Use setState instead.',
404+
componentName,
405+
);
406+
didWarnAboutStateAssignmentForComponent[componentName] = true;
407+
}
403408
}
404409
updater.enqueueReplaceState(instance, instance.state, null);
405410
}

packages/react-reconciler/src/ReactFiberScheduler.js

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -101,41 +101,49 @@ if (__DEV__) {
101101
} = require('./ReactDebugFiberPerf');
102102

103103
var didWarnAboutStateTransition = false;
104+
var didWarnSetStateChildContext = false;
105+
var didWarnStateUpdateForUnmountedComponent = {};
104106

105-
var warnAboutUpdateOnUnmounted = function(
106-
instance: React$ComponentType<any>,
107-
) {
108-
const ctor = instance.constructor;
107+
var warnAboutUpdateOnUnmounted = function(fiber: Fiber) {
108+
const componentName = getComponentName(fiber) || 'ReactClass';
109+
if (didWarnStateUpdateForUnmountedComponent[componentName]) {
110+
return;
111+
}
109112
warning(
110113
false,
111-
'Can only update a mounted or mounting component. This usually means ' +
112-
'you called setState, replaceState, or forceUpdate on an unmounted ' +
113-
'component. This is a no-op.\n\nPlease check the code for the ' +
114-
'%s component.',
115-
(ctor && (ctor.displayName || ctor.name)) || 'ReactClass',
114+
'Can only update a mounted or mounting ' +
115+
'component. This usually means you called setState, replaceState, ' +
116+
'or forceUpdate on an unmounted component. This is a no-op.\n\nPlease ' +
117+
'check the code for the %s component.',
118+
componentName,
116119
);
120+
didWarnStateUpdateForUnmountedComponent[componentName] = true;
117121
};
118122

119-
var warnAboutInvalidUpdates = function(instance: React$ComponentType<any>) {
123+
var warnAboutInvalidUpdates = function(instance: React$Component<any>) {
120124
switch (ReactDebugCurrentFiber.phase) {
121125
case 'getChildContext':
126+
if (didWarnSetStateChildContext) {
127+
return;
128+
}
122129
warning(
123130
false,
124131
'setState(...): Cannot call setState() inside getChildContext()',
125132
);
133+
didWarnSetStateChildContext = true;
126134
break;
127135
case 'render':
128136
if (didWarnAboutStateTransition) {
129137
return;
130138
}
131-
didWarnAboutStateTransition = true;
132139
warning(
133140
false,
134141
'Cannot update during an existing state transition (such as within ' +
135142
"`render` or another component's constructor). Render methods should " +
136143
'be a pure function of props and state; constructor side-effects are ' +
137144
'an anti-pattern, but can be moved to `componentWillMount`.',
138145
);
146+
didWarnAboutStateTransition = true;
139147
break;
140148
}
141149
};
@@ -1229,7 +1237,7 @@ module.exports = function<T, P, I, TI, PI, C, CC, CX, PL>(
12291237
} else {
12301238
if (__DEV__) {
12311239
if (!isErrorRecovery && fiber.tag === ClassComponent) {
1232-
warnAboutUpdateOnUnmounted(fiber.stateNode);
1240+
warnAboutUpdateOnUnmounted(fiber);
12331241
}
12341242
}
12351243
return;

packages/react-reconciler/src/ReactFiberUpdateQueue.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ const {NoWork} = require('./ReactFiberExpirationTime');
2020

2121
if (__DEV__) {
2222
var warning = require('fbjs/lib/warning');
23+
var didWarnUpdateInsideUpdate = false;
2324
}
2425

2526
type PartialState<State, Props> =
@@ -132,14 +133,18 @@ function insertUpdateIntoFiber<State>(
132133

133134
// Warn if an update is scheduled from inside an updater function.
134135
if (__DEV__) {
135-
if (queue1.isProcessing || (queue2 !== null && queue2.isProcessing)) {
136+
if (
137+
(queue1.isProcessing || (queue2 !== null && queue2.isProcessing)) &&
138+
!didWarnUpdateInsideUpdate
139+
) {
136140
warning(
137141
false,
138142
'An update (setState, replaceState, or forceUpdate) was scheduled ' +
139143
'from inside an update function. Update functions should be pure, ' +
140144
'with zero side-effects. Consider using componentDidUpdate or a ' +
141145
'callback.',
142146
);
147+
didWarnUpdateInsideUpdate = true;
143148
}
144149
}
145150

packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,19 @@ describe('ReactIncrementalUpdates', () => {
341341
expect(instance.state).toEqual({a: 'a', b: 'b'});
342342

343343
expectDev(console.error.calls.count()).toBe(1);
344-
console.error.calls.reset();
344+
expectDev(console.error.calls.argsFor(0)[0]).toContain(
345+
'An update (setState, replaceState, or forceUpdate) was scheduled ' +
346+
'from inside an update function. Update functions should be pure, ' +
347+
'with zero side-effects. Consider using componentDidUpdate or a ' +
348+
'callback.',
349+
);
350+
351+
// Test deduplication
352+
instance.setState(function a() {
353+
this.setState({a: 'a'});
354+
return {b: 'b'};
355+
});
356+
ReactNoop.flush();
357+
expectDev(console.error.calls.count()).toBe(1);
345358
});
346359
});

packages/react/src/ReactNoopUpdateQueue.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,29 @@
99

1010
if (__DEV__) {
1111
var warning = require('fbjs/lib/warning');
12+
var didWarnStateUpdateForUnmountedComponent = {};
1213
}
1314

1415
function warnNoop(publicInstance, callerName) {
1516
if (__DEV__) {
1617
var constructor = publicInstance.constructor;
18+
const componentName =
19+
(constructor && (constructor.displayName || constructor.name)) ||
20+
'ReactClass';
21+
const warningKey = `${componentName}.${callerName}`;
22+
if (didWarnStateUpdateForUnmountedComponent[warningKey]) {
23+
return;
24+
}
1725
warning(
1826
false,
1927
'%s(...): Can only update a mounted or mounting component. ' +
2028
'This usually means you called %s() on an unmounted component. ' +
2129
'This is a no-op.\n\nPlease check the code for the %s component.',
2230
callerName,
2331
callerName,
24-
(constructor && (constructor.displayName || constructor.name)) ||
25-
'ReactClass',
32+
componentName,
2633
);
34+
didWarnStateUpdateForUnmountedComponent[warningKey] = true;
2735
}
2836
}
2937

0 commit comments

Comments
 (0)