Skip to content

Commit

Permalink
[Fizz] Minor Fixes for Warning Parity (#21618)
Browse files Browse the repository at this point in the history
  • Loading branch information
sebmarkbage authored Jun 3, 2021
1 parent 1b7b359 commit e0d9b28
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,9 @@ describe('ReactDOMServerLifecycles', () => {
}
}

ReactDOMServer.renderToString(<Component />);
expect(() => ReactDOMServer.renderToString(<Component />)).toErrorDev(
'Unsafe legacy lifecycles will not be called for components using new component APIs.',
);
});

it('should update instance.state with value returned from getDerivedStateFromProps', () => {
Expand Down Expand Up @@ -279,8 +281,8 @@ describe('ReactDOMServerLifecycles', () => {
}
}

expect(() => ReactDOMServer.renderToString(<Component />)).toWarnDev(
'componentWillMount has been renamed',
expect(() => ReactDOMServer.renderToString(<Component />)).toErrorDev(
'Unsafe legacy lifecycles will not be called for components using new component APIs.',
);
});

Expand Down
120 changes: 97 additions & 23 deletions packages/react-dom/src/server/ReactPartialRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ const didWarnAboutModulePatternComponent = {};
const didWarnAboutDeprecatedWillMount = {};
const didWarnAboutUndefinedDerivedState = {};
const didWarnAboutUninitializedState = {};
const didWarnAboutLegacyLifecyclesAndDerivedState = {};
const valuePropNames = ['value', 'defaultValue'];
const newlineEatingTags = {
listing: true,
Expand Down Expand Up @@ -475,6 +476,79 @@ function resolve(
didWarnAboutUninitializedState[componentName] = true;
}
}

// If new component APIs are defined, "unsafe" lifecycles won't be called.
// Warn about these lifecycles if they are present.
// Don't warn about react-lifecycles-compat polyfilled methods though.
if (
typeof Component.getDerivedStateFromProps === 'function' ||
typeof inst.getSnapshotBeforeUpdate === 'function'
) {
let foundWillMountName = null;
let foundWillReceivePropsName = null;
let foundWillUpdateName = null;
if (
typeof inst.componentWillMount === 'function' &&
inst.componentWillMount.__suppressDeprecationWarning !== true
) {
foundWillMountName = 'componentWillMount';
} else if (typeof inst.UNSAFE_componentWillMount === 'function') {
foundWillMountName = 'UNSAFE_componentWillMount';
}
if (
typeof inst.componentWillReceiveProps === 'function' &&
inst.componentWillReceiveProps.__suppressDeprecationWarning !==
true
) {
foundWillReceivePropsName = 'componentWillReceiveProps';
} else if (
typeof inst.UNSAFE_componentWillReceiveProps === 'function'
) {
foundWillReceivePropsName = 'UNSAFE_componentWillReceiveProps';
}
if (
typeof inst.componentWillUpdate === 'function' &&
inst.componentWillUpdate.__suppressDeprecationWarning !== true
) {
foundWillUpdateName = 'componentWillUpdate';
} else if (typeof inst.UNSAFE_componentWillUpdate === 'function') {
foundWillUpdateName = 'UNSAFE_componentWillUpdate';
}
if (
foundWillMountName !== null ||
foundWillReceivePropsName !== null ||
foundWillUpdateName !== null
) {
const componentName =
getComponentNameFromType(Component) || 'Component';
const newApiName =
typeof Component.getDerivedStateFromProps === 'function'
? 'getDerivedStateFromProps()'
: 'getSnapshotBeforeUpdate()';
if (!didWarnAboutLegacyLifecyclesAndDerivedState[componentName]) {
didWarnAboutLegacyLifecyclesAndDerivedState[
componentName
] = true;
console.error(
'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' +
'%s uses %s but also contains the following legacy lifecycles:%s%s%s\n\n' +
'The above lifecycles should be removed. Learn more about this warning here:\n' +
'https://reactjs.org/link/unsafe-component-lifecycles',
componentName,
newApiName,
foundWillMountName !== null
? `\n ${foundWillMountName}`
: '',
foundWillReceivePropsName !== null
? `\n ${foundWillReceivePropsName}`
: '',
foundWillUpdateName !== null
? `\n ${foundWillUpdateName}`
: '',
);
}
}
}
}

const partialState = Component.getDerivedStateFromProps.call(
Expand Down Expand Up @@ -575,32 +649,32 @@ function resolve(
typeof inst.componentWillMount === 'function'
) {
if (typeof inst.componentWillMount === 'function') {
if (__DEV__) {
if (
warnAboutDeprecatedLifecycles &&
inst.componentWillMount.__suppressDeprecationWarning !== true
) {
const componentName =
getComponentNameFromType(Component) || 'Unknown';

if (!didWarnAboutDeprecatedWillMount[componentName]) {
console.warn(
// keep this warning in sync with ReactStrictModeWarning.js
'componentWillMount has been renamed, and is not recommended for use. ' +
'See https://reactjs.org/link/unsafe-component-lifecycles for details.\n\n' +
'* Move code from componentWillMount to componentDidMount (preferred in most cases) ' +
'or the constructor.\n' +
'\nPlease update the following components: %s',
componentName,
);
didWarnAboutDeprecatedWillMount[componentName] = true;
}
}
}

// In order to support react-lifecycles-compat polyfilled components,
// Unsafe lifecycles should not be invoked for any component with the new gDSFP.
if (typeof Component.getDerivedStateFromProps !== 'function') {
if (__DEV__) {
if (
warnAboutDeprecatedLifecycles &&
inst.componentWillMount.__suppressDeprecationWarning !== true
) {
const componentName =
getComponentNameFromType(Component) || 'Unknown';

if (!didWarnAboutDeprecatedWillMount[componentName]) {
console.warn(
// keep this warning in sync with ReactStrictModeWarning.js
'componentWillMount has been renamed, and is not recommended for use. ' +
'See https://reactjs.org/link/unsafe-component-lifecycles for details.\n\n' +
'* Move code from componentWillMount to componentDidMount (preferred in most cases) ' +
'or the constructor.\n' +
'\nPlease update the following components: %s',
componentName,
);
didWarnAboutDeprecatedWillMount[componentName] = true;
}
}
}

inst.componentWillMount();
}
}
Expand Down
28 changes: 27 additions & 1 deletion packages/react-server/src/ReactFizzClassComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,17 @@
import {emptyContextObject} from './ReactFizzContext';
import {readContext} from './ReactFizzNewContext';

import {disableLegacyContext} from 'shared/ReactFeatureFlags';
import {
disableLegacyContext,
warnAboutDeprecatedLifecycles,
} from 'shared/ReactFeatureFlags';
import {get as getInstance, set as setInstance} from 'shared/ReactInstanceMap';
import getComponentNameFromType from 'shared/getComponentNameFromType';
import {REACT_CONTEXT_TYPE, REACT_PROVIDER_TYPE} from 'shared/ReactSymbols';
import isArray from 'shared/isArray';

const didWarnAboutNoopUpdateForComponent = {};
const didWarnAboutDeprecatedWillMount = {};

let didWarnAboutUninitializedState;
let didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate;
Expand Down Expand Up @@ -531,6 +535,28 @@ function callComponentWillMount(type, instance) {
const oldState = instance.state;

if (typeof instance.componentWillMount === 'function') {
if (__DEV__) {
if (
warnAboutDeprecatedLifecycles &&
instance.componentWillMount.__suppressDeprecationWarning !== true
) {
const componentName = getComponentNameFromType(type) || 'Unknown';

if (!didWarnAboutDeprecatedWillMount[componentName]) {
console.warn(
// keep this warning in sync with ReactStrictModeWarning.js
'componentWillMount has been renamed, and is not recommended for use. ' +
'See https://reactjs.org/link/unsafe-component-lifecycles for details.\n\n' +
'* Move code from componentWillMount to componentDidMount (preferred in most cases) ' +
'or the constructor.\n' +
'\nPlease update the following components: %s',
componentName,
);
didWarnAboutDeprecatedWillMount[componentName] = true;
}
}
}

instance.componentWillMount();
}
if (typeof instance.UNSAFE_componentWillMount === 'function') {
Expand Down
16 changes: 15 additions & 1 deletion packages/react-server/src/ReactFizzServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1032,13 +1032,27 @@ function renderElement(
}
}
let info = '';
if (__DEV__) {
if (
type === undefined ||
(typeof type === 'object' &&
type !== null &&
Object.keys(type).length === 0)
) {
info +=
' You likely forgot to export your component from the file ' +
"it's defined in, or you might have mixed up default and " +
'named imports.';
}
}
invariant(
false,
'Element type is invalid: expected a string (for built-in ' +
'components) or a class/function (for composite components) ' +
'but got: %s.%s',
type == null ? type : typeof type,
'',
info,
);
}

Expand Down

0 comments on commit e0d9b28

Please sign in to comment.