@samitouri / QOS-React / commits / 9e9e3970e4

Warn for Hook set-state on unmounted component

Sophie Alpert committed Oct 5, 2018 at 15:32 UTC 9e9e3970e424e3a6d36b29d22cb7326d6c5705f8
3 files changed +45 -10
packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
+2 -2
@@ -332,7 +332,7 @@ describe('ReactCompositeComponent', () => {
332 ReactDOM.unmountComponentAtNode(container);
333
334 expect(() => instance.forceUpdate()).toWarnDev(
335 - "Warning: Can't call setState (or forceUpdate) on an unmounted " +
335 + "Warning: Can't perform a React state update on an unmounted " +
336 'component. This is a no-op, but it indicates a memory leak in your ' +
337 'application. To fix, cancel all subscriptions and asynchronous ' +
338 'tasks in the componentWillUnmount method.\n' +
@@ -379,7 +379,7 @@ describe('ReactCompositeComponent', () => {
379 expect(() => {
380 instance.setState({value: 2});
381 }).toWarnDev(
382 - "Warning: Can't call setState (or forceUpdate) on an unmounted " +
382 + "Warning: Can't perform a React state update on an unmounted " +
383 'component. This is a no-op, but it indicates a memory leak in your ' +
384 'application. To fix, cancel all subscriptions and asynchronous ' +
385 'tasks in the componentWillUnmount method.\n' +
packages/react-reconciler/src/ReactFiberScheduler.js
+22 -8
@@ -45,11 +45,14 @@ import {
45 Passive,
46 } from 'shared/ReactSideEffectTags';
47 import {
48 - HostRoot,
48 ClassComponent,
49 HostComponent,
50 ContextProvider,
51 + ForwardRef,
52 + FunctionComponent,
53 HostPortal,
54 + HostRoot,
55 + PureComponent,
56 } from 'shared/ReactWorkTags';
57 import {
58 enableSchedulerTracing,
@@ -197,19 +200,21 @@ if (__DEV__) {
200 didWarnSetStateChildContext = false;
201 const didWarnStateUpdateForUnmountedComponent = {};
202
200 - warnAboutUpdateOnUnmounted = function(fiber: Fiber) {
203 + warnAboutUpdateOnUnmounted = function(fiber: Fiber, isClass: boolean) {
204 // We show the whole stack but dedupe on the top component's name because
205 // the problematic code almost always lies inside that component.
203 - const componentName = getComponentName(fiber.type) || 'ReactClass';
206 + const componentName = getComponentName(fiber.type) || 'ReactComponent';
207 if (didWarnStateUpdateForUnmountedComponent[componentName]) {
208 return;
209 }
210 warningWithoutStack(
211 false,
209 - "Can't call setState (or forceUpdate) on an unmounted component. This " +
212 + "Can't perform a React state update on an unmounted component. This " +
213 'is a no-op, but it indicates a memory leak in your application. To ' +
211 - 'fix, cancel all subscriptions and asynchronous tasks in the ' +
212 - 'componentWillUnmount method.%s',
214 + 'fix, cancel all subscriptions and asynchronous tasks in %s.%s',
215 + isClass
216 + ? 'the componentWillUnmount method'
217 + : 'a useEffect cleanup function',
218 ReactCurrentFiber.getStackByFiberInDevAndProd(fiber),
219 );
220 didWarnStateUpdateForUnmountedComponent[componentName] = true;
@@ -1785,8 +1790,17 @@ function scheduleWorkToRoot(fiber: Fiber, expirationTime): FiberRoot | null {
1790 }
1791
1792 if (root === null) {
1788 - if (__DEV__ && fiber.tag === ClassComponent) {
1789 - warnAboutUpdateOnUnmounted(fiber);
1793 + if (__DEV__) {
1794 + switch (fiber.tag) {
1795 + case ClassComponent:
1796 + warnAboutUpdateOnUnmounted(fiber, true);
1797 + break;
1798 + case FunctionComponent:
1799 + case ForwardRef:
1800 + case PureComponent:
1801 + warnAboutUpdateOnUnmounted(fiber, false);
1802 + break;
1803 + }
1804 }
1805 return null;
1806 }
packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
+21
@@ -309,6 +309,27 @@ describe('ReactHooks', () => {
309
310 expect(updaters).toEqual([updaters[0], updaters[0], updaters[0]]);
311 });
312 +
313 + it('warns on set after unmount', () => {
314 + let _updateCount;
315 + function Counter(props, ref) {
316 + const [, updateCount] = useState(0);
317 + _updateCount = updateCount;
318 + return null;
319 + }
320 +
321 + ReactNoop.render(<Counter />);
322 + ReactNoop.flush();
323 + ReactNoop.render(null);
324 + ReactNoop.flush();
325 + expect(() => _updateCount(1)).toWarnDev(
326 + "Warning: Can't perform a React state update on an unmounted " +
327 + 'component. This is a no-op, but it indicates a memory leak in your ' +
328 + 'application. To fix, cancel all subscriptions and asynchronous ' +
329 + 'tasks in a useEffect cleanup function.\n' +
330 + ' in Counter (at **)',
331 + );
332 + });
333 });
334
335 describe('updates during the render phase', () => {