Skip to content

Commit

Permalink
Coalesce lifecycle deprecation warnings until the commit phase
Browse files Browse the repository at this point in the history 
Builds on top of PR facebook#12083 and resolves issue facebook#12044.

Coalesces deprecation warnings until the commit phase. This proposal extends the  utility introduced in facebook#12060 to also coalesce deprecation warnings.

New warning format will look like this:
> componentWillMount is deprecated and will be removed in the next major version. Use componentDidMount instead. As a temporary workaround, you can rename to UNSAFE_componentWillMount.
>
> Please update the following components: Foo, Bar
>
> Learn more about this warning here:
> https://fb.me/react-async-component-lifecycle-hooks
  • Loading branch information
@bvaughn
bvaughn committed Jan 25, 2018
1 parent be51e6a commit b6d9aa7
Show file tree
Hide file tree
Showing 5 changed files with 159 additions and 103 deletions.
24 changes: 13 additions & 11 deletions packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,21 @@ describe('ReactComponentLifeCycle', () => {

const container = document.createElement('div');
expect(() => ReactDOM.render(<MyComponent x={1} />, container)).toWarnDev([
'Warning: MyComponent: componentWillMount() is deprecated and will be ' +
'removed in the next major version.',
'componentWillMount is deprecated and will be removed in the next major version. ' +
'Use componentDidMount instead. As a temporary workaround, ' +
'you can rename to UNSAFE_componentWillMount.' +
'\n\nPlease update the following components: MyComponent',
'componentWillReceiveProps is deprecated and will be removed in the next major version. ' +
'Use static getDerivedStateFromProps instead.' +
'\n\nPlease update the following components: MyComponent',
'componentWillUpdate is deprecated and will be removed in the next major version. ' +
'Use componentDidUpdate instead. As a temporary workaround, ' +
'you can rename to UNSAFE_componentWillUpdate.' +
'\n\nPlease update the following components: MyComponent',
]);

expect(() => ReactDOM.render(<MyComponent x={2} />, container)).toWarnDev([
'Warning: MyComponent: componentWillReceiveProps() is deprecated and ' +
'will be removed in the next major version.',
'Warning: MyComponent: componentWillUpdate() is deprecated and will be ' +
'removed in the next major version.',
]);

// Dedupe check (instantiate and update)
// Dedupe check (update and instantiate new
ReactDOM.render(<MyComponent x={2} />, container);
ReactDOM.render(<MyComponent key="new" x={1} />, container);
ReactDOM.render(<MyComponent key="new" x={2} />, container);
});
});
78 changes: 8 additions & 70 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,21 +42,13 @@ import {hasContextChanged} from './ReactFiberContext';
const fakeInternalInstance = {};
const isArray = Array.isArray;

let didWarnAboutLegacyWillMount;
let didWarnAboutLegacyWillReceiveProps;
let didWarnAboutLegacyWillUpdate;
let didWarnAboutStateAssignmentForComponent;
let didWarnAboutUndefinedDerivedState;
let didWarnAboutUninitializedState;
let didWarnAboutWillReceivePropsAndDerivedState;
let warnOnInvalidCallback;

if (__DEV__) {
if (warnAboutDeprecatedLifecycles) {
didWarnAboutLegacyWillMount = {};
didWarnAboutLegacyWillReceiveProps = {};
didWarnAboutLegacyWillUpdate = {};
}
didWarnAboutStateAssignmentForComponent = {};
didWarnAboutUndefinedDerivedState = {};
didWarnAboutUninitializedState = {};
Expand Down Expand Up @@ -462,25 +454,6 @@ export default function(
const oldState = instance.state;

if (typeof instance.componentWillMount === 'function') {
if (__DEV__) {
if (warnAboutDeprecatedLifecycles) {
const componentName = getComponentName(workInProgress) || 'Component';
if (!didWarnAboutLegacyWillMount[componentName]) {
warning(
false,
'%s: componentWillMount() is deprecated and will be ' +
'removed in the next major version. Read about the motivations ' +
'behind this change: ' +
'https://fb.me/react-async-component-lifecycle-hooks' +
'\n\n' +
'As a temporary workaround, you can rename to ' +
'UNSAFE_componentWillMount instead.',
componentName,
);
didWarnAboutLegacyWillMount[componentName] = true;
}
}
}
instance.componentWillMount();
} else {
instance.UNSAFE_componentWillMount();
Expand Down Expand Up @@ -510,27 +483,6 @@ export default function(
) {
const oldState = instance.state;
if (typeof instance.componentWillReceiveProps === 'function') {
if (__DEV__) {
if (warnAboutDeprecatedLifecycles) {
const componentName = getComponentName(workInProgress) || 'Component';
if (!didWarnAboutLegacyWillReceiveProps[componentName]) {
warning(
false,
'%s: componentWillReceiveProps() is deprecated and ' +
'will be removed in the next major version. Use ' +
'static getDerivedStateFromProps() instead. Read about the ' +
'motivations behind this change: ' +
'https://fb.me/react-async-component-lifecycle-hooks' +
'\n\n' +
'As a temporary workaround, you can rename to ' +
'UNSAFE_componentWillReceiveProps instead.',
componentName,
);
didWarnAboutLegacyWillReceiveProps[componentName] = true;
}
}
}

startPhaseTimer(workInProgress, 'componentWillReceiveProps');
instance.componentWillReceiveProps(newProps, newContext);
stopPhaseTimer();
Expand Down Expand Up @@ -652,7 +604,14 @@ export default function(

if (__DEV__) {
if (workInProgress.internalContextTag & StrictMode) {
ReactStrictModeWarnings.recordLifecycleWarnings(
ReactStrictModeWarnings.recordUnsafeLifecycleWarnings(
workInProgress,
instance,
);
}

if (warnAboutDeprecatedLifecycles) {
ReactStrictModeWarnings.recordDeprecationWarnings(
workInProgress,
instance,
);
Expand Down Expand Up @@ -893,27 +852,6 @@ export default function(
typeof instance.componentWillUpdate === 'function'
) {
if (typeof instance.componentWillUpdate === 'function') {
if (__DEV__) {
if (warnAboutDeprecatedLifecycles) {
const componentName =
getComponentName(workInProgress) || 'Component';
if (!didWarnAboutLegacyWillUpdate[componentName]) {
warning(
false,
'%s: componentWillUpdate() is deprecated and will be ' +
'removed in the next major version. Read about the motivations ' +
'behind this change: ' +
'https://fb.me/react-async-component-lifecycle-hooks' +
'\n\n' +
'As a temporary workaround, you can rename to ' +
'UNSAFE_componentWillUpdate instead.',
componentName,
);
didWarnAboutLegacyWillUpdate[componentName] = true;
}
}
}

startPhaseTimer(workInProgress, 'componentWillUpdate');
instance.componentWillUpdate(newProps, newState, newContext);
stopPhaseTimer();
Expand Down
11 changes: 9 additions & 2 deletions packages/react-reconciler/src/ReactFiberScheduler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ import {
HostPortal,
ClassComponent,
} from 'shared/ReactTypeOfWork';
import {enableUserTimingAPI} from 'shared/ReactFeatureFlags';
import {
enableUserTimingAPI,
warnAboutDeprecatedLifecycles,
} from 'shared/ReactFeatureFlags';
import getComponentName from 'shared/getComponentName';
import invariant from 'fbjs/lib/invariant';
import warning from 'fbjs/lib/warning';
Expand Down Expand Up @@ -312,7 +315,11 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(

function commitAllLifeCycles() {
if (__DEV__) {
ReactStrictModeWarnings.flushPendingAsyncWarnings();
ReactStrictModeWarnings.flushPendingUnsafeLifecycleWarnings();

if (warnAboutDeprecatedLifecycles) {
ReactStrictModeWarnings.flushPendingDeprecationWarnings();
}
}

while (nextEffect !== null) {
Expand Down
133 changes: 119 additions & 14 deletions packages/react-reconciler/src/ReactStrictModeWarnings.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ type FiberToLifecycleMap = Map<Fiber, LifecycleToComponentsMap>;

const ReactStrictModeWarnings = {
discardPendingWarnings(): void {},
flushPendingAsyncWarnings(): void {},
recordLifecycleWarnings(fiber: Fiber, instance: any): void {},
flushPendingDeprecationWarnings(): void {},
flushPendingUnsafeLifecycleWarnings(): void {},
recordDeprecationWarnings(fiber: Fiber, instance: any): void {},
recordUnsafeLifecycleWarnings(fiber: Fiber, instance: any): void {},
};

if (__DEV__) {
Expand All @@ -34,17 +36,24 @@ if (__DEV__) {
UNSAFE_componentWillUpdate: 'componentDidUpdate',
};

let pendingWarningsMap: FiberToLifecycleMap = new Map();
let pendingComponentWillMountWarnings: Array<Fiber> = [];
let pendingComponentWillReceivePropsWarnings: Array<Fiber> = [];
let pendingComponentWillUpdateWarnings: Array<Fiber> = [];
let pendingUnsafeLifecycleWarnings: FiberToLifecycleMap = new Map();

// Tracks components we have already warned about.
const didWarnSet = new Set();
const didWarnAboutDeprecatedLifecycles = new Set();
const didWarnAboutUnsafeLifecycles = new Set();

ReactStrictModeWarnings.discardPendingWarnings = () => {
pendingWarningsMap = new Map();
pendingComponentWillMountWarnings = [];
pendingComponentWillReceivePropsWarnings = [];
pendingComponentWillUpdateWarnings = [];
pendingUnsafeLifecycleWarnings = new Map();
};

ReactStrictModeWarnings.flushPendingAsyncWarnings = () => {
((pendingWarningsMap: any): FiberToLifecycleMap).forEach(
ReactStrictModeWarnings.flushPendingUnsafeLifecycleWarnings = () => {
((pendingUnsafeLifecycleWarnings: any): FiberToLifecycleMap).forEach(
(lifecycleWarningsMap, strictRoot) => {
const lifecyclesWarningMesages = [];

Expand All @@ -54,7 +63,7 @@ if (__DEV__) {
const componentNames = new Set();
lifecycleWarnings.forEach(fiber => {
componentNames.add(getComponentName(fiber) || 'Component');
didWarnSet.add(fiber.type);
didWarnAboutUnsafeLifecycles.add(fiber.type);
});

const formatted = lifecycle.replace('UNSAFE_', '');
Expand Down Expand Up @@ -88,7 +97,7 @@ if (__DEV__) {
},
);

pendingWarningsMap = new Map();
pendingUnsafeLifecycleWarnings = new Map();
};

const getStrictRoot = (fiber: Fiber): Fiber => {
Expand All @@ -105,7 +114,103 @@ if (__DEV__) {
return maybeStrictRoot;
};

ReactStrictModeWarnings.recordLifecycleWarnings = (
ReactStrictModeWarnings.flushPendingDeprecationWarnings = () => {
if (pendingComponentWillMountWarnings.length > 0) {
const uniqueNames = new Set();
pendingComponentWillMountWarnings.forEach(fiber => {
uniqueNames.add(getComponentName(fiber) || 'Component');
didWarnAboutDeprecatedLifecycles.add(fiber.type);
});

const sortedNames = Array.from(uniqueNames)
.sort()
.join(', ');

warning(
false,
'componentWillMount is deprecated and will be removed in the next major version. ' +
'Use componentDidMount instead. As a temporary workaround, ' +
'you can rename to UNSAFE_componentWillMount.' +
'\n\nPlease update the following components: %s' +
'\n\nLearn more about this warning here:' +
'\nhttps://fb.me/react-async-component-lifecycle-hooks',
sortedNames,
);

pendingComponentWillMountWarnings = [];
}

if (pendingComponentWillReceivePropsWarnings.length > 0) {
const uniqueNames = new Set();
pendingComponentWillReceivePropsWarnings.forEach(fiber => {
uniqueNames.add(getComponentName(fiber) || 'Component');
didWarnAboutDeprecatedLifecycles.add(fiber.type);
});

const sortedNames = Array.from(uniqueNames)
.sort()
.join(', ');

warning(
false,
'componentWillReceiveProps is deprecated and will be removed in the next major version. ' +
'Use static getDerivedStateFromProps instead.' +
'\n\nPlease update the following components: %s' +
'\n\nLearn more about this warning here:' +
'\nhttps://fb.me/react-async-component-lifecycle-hooks',
sortedNames,
);

pendingComponentWillReceivePropsWarnings = [];
}

if (pendingComponentWillUpdateWarnings.length > 0) {
const uniqueNames = new Set();
pendingComponentWillUpdateWarnings.forEach(fiber => {
uniqueNames.add(getComponentName(fiber) || 'Component');
didWarnAboutDeprecatedLifecycles.add(fiber.type);
});

const sortedNames = Array.from(uniqueNames)
.sort()
.join(', ');

warning(
false,
'componentWillUpdate is deprecated and will be removed in the next major version. ' +
'Use componentDidUpdate instead. As a temporary workaround, ' +
'you can rename to UNSAFE_componentWillUpdate.' +
'\n\nPlease update the following components: %s' +
'\n\nLearn more about this warning here:' +
'\nhttps://fb.me/react-async-component-lifecycle-hooks',
sortedNames,
);

pendingComponentWillUpdateWarnings = [];
}
};

ReactStrictModeWarnings.recordDeprecationWarnings = (
fiber: Fiber,
instance: any,
) => {
// Dedup strategy: Warn once per component.
if (didWarnAboutDeprecatedLifecycles.has(fiber.type)) {
return;
}

if (typeof instance.componentWillMount === 'function') {
pendingComponentWillMountWarnings.push(fiber);
}
if (typeof instance.componentWillReceiveProps === 'function') {
pendingComponentWillReceivePropsWarnings.push(fiber);
}
if (typeof instance.componentWillUpdate === 'function') {
pendingComponentWillUpdateWarnings.push(fiber);
}
};

ReactStrictModeWarnings.recordUnsafeLifecycleWarnings = (
fiber: Fiber,
instance: any,
) => {
Expand All @@ -116,21 +221,21 @@ if (__DEV__) {
// are often vague and are likely to collide between 3rd party libraries.
// An expand property is probably okay to use here since it's DEV-only,
// and will only be set in the event of serious warnings.
if (didWarnSet.has(fiber.type)) {
if (didWarnAboutUnsafeLifecycles.has(fiber.type)) {
return;
}

let warningsForRoot;
if (!pendingWarningsMap.has(strictRoot)) {
if (!pendingUnsafeLifecycleWarnings.has(strictRoot)) {
warningsForRoot = {
UNSAFE_componentWillMount: [],
UNSAFE_componentWillReceiveProps: [],
UNSAFE_componentWillUpdate: [],
};

pendingWarningsMap.set(strictRoot, warningsForRoot);
pendingUnsafeLifecycleWarnings.set(strictRoot, warningsForRoot);
} else {
warningsForRoot = pendingWarningsMap.get(strictRoot);
warningsForRoot = pendingUnsafeLifecycleWarnings.get(strictRoot);
}

const unsafeLifecycles = [];
Expand Down
16 changes: 10 additions & 6 deletions packages/react/src/__tests__/createReactClassIntegration-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,18 @@ describe('create-react-class-integration', () => {
'Warning: MyComponent: isMounted is deprecated. Instead, make sure to ' +
'clean up subscriptions and pending requests in componentWillUnmount ' +
'to prevent memory leaks.',
'Warning: MyComponent: componentWillMount() is deprecated and will be ' +
'removed in the next major version.',
'componentWillMount is deprecated and will be removed in the next major version. ' +
'Use componentDidMount instead. As a temporary workaround, ' +
'you can rename to UNSAFE_componentWillMount.' +
'\n\nPlease update the following components: MyComponent',
'componentWillUpdate is deprecated and will be removed in the next major version. ' +
'Use componentDidUpdate instead. As a temporary workaround, ' +
'you can rename to UNSAFE_componentWillUpdate.' +
'\n\nPlease update the following components: MyComponent',
]);

expect(() => ReactDOM.render(<Component />, container)).toWarnDev(
'Warning: MyComponent: componentWillUpdate() is deprecated and will be ' +
'removed in the next major version.',
);
// Dedupe
ReactDOM.render(<Component />, container);

ReactDOM.unmountComponentAtNode(container);
instance.log('after unmount');
Expand Down

0 comments on commit b6d9aa7

Please sign in to comment.