Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Coalesce lifecycle deprecation warnings until the commit phase #12084

Merged
merged 1 commit into from
Jan 25, 2018
Merged

Coalesce lifecycle deprecation warnings until the commit phase #12084

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.internal.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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