Skip to content

Commit

Permalink
Always warn about legacy context within StrictMode tree (facebook#13760)
Browse files Browse the repository at this point in the history
  • Loading branch information
bvaughn authored and jetoneza committed Jan 23, 2019
1 parent fc61849 commit 312d3ba
Show file tree
Hide file tree
Showing 16 changed files with 101 additions and 45 deletions.
6 changes: 1 addition & 5 deletions packages/react-reconciler/src/ReactFiberScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ import {
enableUserTimingAPI,
replayFailedUnitOfWorkWithInvokeGuardedCallback,
warnAboutDeprecatedLifecycles,
warnAboutLegacyContextAPI,
enableSuspense,
} from 'shared/ReactFeatureFlags';
import getComponentName from 'shared/getComponentName';
Expand Down Expand Up @@ -479,14 +478,11 @@ function commitAllLifeCycles(
) {
if (__DEV__) {
ReactStrictModeWarnings.flushPendingUnsafeLifecycleWarnings();
ReactStrictModeWarnings.flushLegacyContextWarning();

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

if (warnAboutLegacyContextAPI) {
ReactStrictModeWarnings.flushLegacyContextWarning();
}
}
while (nextEffect !== null) {
const effectTag = nextEffect.effectTag;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1936,7 +1936,11 @@ describe('ReactIncremental', () => {
</div>
</Intl>,
);
ReactNoop.flush();
expect(ReactNoop.flush).toWarnDev(
'Legacy context API has been detected within a strict-mode tree: \n\n' +
'Please update the following components: Intl, ShowBoth, ShowLocale',
{withoutStack: true},
);
expect(ops).toEqual([
'Intl {}',
'ShowLocale {"locale":"fr"}',
Expand Down Expand Up @@ -1981,7 +1985,11 @@ describe('ReactIncremental', () => {
<ShowBoth />
</Intl>,
);
ReactNoop.flush();
expect(ReactNoop.flush).toWarnDev(
'Legacy context API has been detected within a strict-mode tree: \n\n' +
'Please update the following components: Router, ShowRoute',
{withoutStack: true},
);
expect(ops).toEqual([
'ShowLocale {"locale":"sv"}',
'ShowBoth {"locale":"sv"}',
Expand Down Expand Up @@ -2021,7 +2029,11 @@ describe('ReactIncremental', () => {
}

ReactNoop.render(<Recurse />);
ReactNoop.flush();
expect(ReactNoop.flush).toWarnDev(
'Legacy context API has been detected within a strict-mode tree: \n\n' +
'Please update the following components: Recurse',
{withoutStack: true},
);
expect(ops).toEqual([
'Recurse {}',
'Recurse {"n":2}',
Expand Down Expand Up @@ -2054,7 +2066,11 @@ describe('ReactIncremental', () => {
};

ReactNoop.render(<Recurse />);
ReactNoop.flush();
expect(ReactNoop.flush).toWarnDev(
'Legacy context API has been detected within a strict-mode tree: \n\n' +
'Please update the following components: Recurse',
{withoutStack: true},
);
expect(ops).toEqual([
'Recurse {}',
'Recurse {"n":2}',
Expand Down Expand Up @@ -2112,7 +2128,11 @@ describe('ReactIncremental', () => {
]);

ops.length = 0;
ReactNoop.flush();
expect(ReactNoop.flush).toWarnDev(
'Legacy context API has been detected within a strict-mode tree: \n\n' +
'Please update the following components: Intl, ShowLocale',
{withoutStack: true},
);
expect(ops).toEqual([
'ShowLocale {"locale":"fr"}',
'Intl {}',
Expand Down Expand Up @@ -2192,7 +2212,11 @@ describe('ReactIncremental', () => {
</IndirectionFn>
</Intl>,
);
ReactNoop.flush();
expect(ReactNoop.flush).toWarnDev(
'Legacy context API has been detected within a strict-mode tree: \n\n' +
'Please update the following components: Intl, ShowLocaleClass, ShowLocaleFn',
{withoutStack: true},
);
expect(ops).toEqual([
'Intl:read {}',
'Intl:provide {"locale":"fr"}',
Expand Down Expand Up @@ -2280,7 +2304,11 @@ describe('ReactIncremental', () => {
</IndirectionFn>
</Stateful>,
);
ReactNoop.flush();
expect(ReactNoop.flush).toWarnDev(
'Legacy context API has been detected within a strict-mode tree: \n\n' +
'Please update the following components: Intl, ShowLocaleClass, ShowLocaleFn',
{withoutStack: true},
);
expect(ops).toEqual([
'Intl:read {}',
'Intl:provide {"locale":"fr"}',
Expand Down Expand Up @@ -2345,7 +2373,11 @@ describe('ReactIncremental', () => {

// Init
ReactNoop.render(<Root />);
ReactNoop.flush();
expect(ReactNoop.flush).toWarnDev(
'Legacy context API has been detected within a strict-mode tree: \n\n' +
'Please update the following components: Child',
{withoutStack: true},
);

// Trigger an update in the middle of the tree
instance.setState({});
Expand Down Expand Up @@ -2391,7 +2423,11 @@ describe('ReactIncremental', () => {

// Init
ReactNoop.render(<Root />);
ReactNoop.flush();
expect(ReactNoop.flush).toWarnDev(
'Legacy context API has been detected within a strict-mode tree: \n\n' +
'Please update the following components: ContextProvider',
{withoutStack: true},
);

// Trigger an update in the middle of the tree
// This is necessary to reproduce the error as it curently exists.
Expand Down Expand Up @@ -2437,8 +2473,12 @@ describe('ReactIncremental', () => {

ReactNoop.render(<MyComponent />);
expect(ReactNoop.flush).toWarnDev(
'componentWillReceiveProps: Please update the following components ' +
'to use static getDerivedStateFromProps instead: MyComponent',
[
'componentWillReceiveProps: Please update the following components ' +
'to use static getDerivedStateFromProps instead: MyComponent',
'Legacy context API has been detected within a strict-mode tree: \n\n' +
'Please update the following components: MyComponent',
],
{withoutStack: true},
);

Expand Down Expand Up @@ -2582,7 +2622,11 @@ describe('ReactIncremental', () => {
</TopContextProvider>,
);

ReactNoop.flush();
expect(ReactNoop.flush).toWarnDev(
'Legacy context API has been detected within a strict-mode tree: \n\n' +
'Please update the following components: Child, TopContextProvider',
{withoutStack: true},
);
expect(rendered).toEqual(['count:0']);
instance.updateCount();
ReactNoop.flush();
Expand Down Expand Up @@ -2640,7 +2684,11 @@ describe('ReactIncremental', () => {
</TopContextProvider>,
);

ReactNoop.flush();
expect(ReactNoop.flush).toWarnDev(
'Legacy context API has been detected within a strict-mode tree: \n\n' +
'Please update the following components: Child, MiddleContextProvider, TopContextProvider',
{withoutStack: true},
);
expect(rendered).toEqual(['count:0']);
instance.updateCount();
ReactNoop.flush();
Expand Down Expand Up @@ -2707,7 +2755,11 @@ describe('ReactIncremental', () => {
</TopContextProvider>,
);

ReactNoop.flush();
expect(ReactNoop.flush).toWarnDev(
'Legacy context API has been detected within a strict-mode tree: \n\n' +
'Please update the following components: Child, MiddleContextProvider, TopContextProvider',
{withoutStack: true},
);
expect(rendered).toEqual(['count:0']);
instance.updateCount();
ReactNoop.flush();
Expand Down Expand Up @@ -2784,7 +2836,11 @@ describe('ReactIncremental', () => {
</TopContextProvider>,
);

ReactNoop.flush();
expect(ReactNoop.flush).toWarnDev(
'Legacy context API has been detected within a strict-mode tree: \n\n' +
'Please update the following components: Child, MiddleContextProvider, TopContextProvider',
{withoutStack: true},
);
expect(rendered).toEqual(['count:0, name:brian']);
topInstance.updateCount();
ReactNoop.flush();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1092,7 +1092,11 @@ describe('ReactIncrementalErrorHandling', () => {
<Connector />
</Provider>,
);
ReactNoop.flush();
expect(ReactNoop.flush).toWarnDev(
'Legacy context API has been detected within a strict-mode tree: \n\n' +
'Please update the following components: Connector, Provider',
{withoutStack: true},
);

// If the context stack does not unwind, span will get 'abcde'
expect(ReactNoop.getChildren()).toEqual([span('a')]);
Expand Down Expand Up @@ -1581,6 +1585,12 @@ describe('ReactIncrementalErrorHandling', () => {
};

ReactNoop.render(<Provider />);
expect(() => ReactNoop.flush()).toThrow('Oops!');
expect(() => {
expect(() => ReactNoop.flush()).toThrow('Oops!');
}).toWarnDev(
'Legacy context API has been detected within a strict-mode tree: \n\n' +
'Please update the following components: Provider',
{withoutStack: true},
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -357,12 +357,16 @@ describe('ReactDebugFiberPerf', () => {
ReactNoop.render(<AllLifecycles />);
addComment('Mount');
expect(ReactNoop.flush).toWarnDev(
'componentWillMount: Please update the following components ' +
'to use componentDidMount instead: AllLifecycles' +
'\n\ncomponentWillReceiveProps: Please update the following components ' +
'to use static getDerivedStateFromProps instead: AllLifecycles' +
'\n\ncomponentWillUpdate: Please update the following components ' +
'to use componentDidUpdate instead: AllLifecycles',
[
'componentWillMount: Please update the following components ' +
'to use componentDidMount instead: AllLifecycles' +
'\n\ncomponentWillReceiveProps: Please update the following components ' +
'to use static getDerivedStateFromProps instead: AllLifecycles' +
'\n\ncomponentWillUpdate: Please update the following components ' +
'to use componentDidUpdate instead: AllLifecycles',
'Legacy context API has been detected within a strict-mode tree: \n\n' +
'Please update the following components: AllLifecycles',
],
{withoutStack: true},
);
ReactNoop.render(<AllLifecycles />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1144,7 +1144,13 @@ describe('ReactNewContext', () => {
</App>
</LegacyProvider>,
);
expect(ReactNoop.flush()).toEqual(['LegacyProvider', 'App', 'Child']);
expect(() => {
expect(ReactNoop.flush()).toEqual(['LegacyProvider', 'App', 'Child']);
}).toWarnDev(
'Legacy context API has been detected within a strict-mode tree: \n\n' +
'Please update the following components: LegacyProvider',
{withoutStack: true},
);
expect(ReactNoop.getChildren()).toEqual([span('Child')]);

// Update App with same value (should bail out)
Expand Down
2 changes: 0 additions & 2 deletions packages/react/src/__tests__/ReactStrictMode-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -822,8 +822,6 @@ describe('ReactStrictMode', () => {
React = require('react');
ReactTestRenderer = require('react-test-renderer');
PropTypes = require('prop-types');
ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.warnAboutLegacyContextAPI = true;
});

it('should warn if the legacy context API have been used in strict mode', () => {
Expand Down
3 changes: 0 additions & 3 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
// Warn about deprecated, async-unsafe lifecycles; relates to RFC #6:
export const warnAboutDeprecatedLifecycles = false;

// Warn about legacy context API
export const warnAboutLegacyContextAPI = false;

// Gather advanced timing metrics for Profiler subtrees.
export const enableProfilerTimer = __PROFILE__;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ export const debugRenderPhaseSideEffectsForStrictMode = false;
export const enableUserTimingAPI = __DEV__;
export const enableSuspense = false;
export const warnAboutDeprecatedLifecycles = false;
export const warnAboutLegacyContextAPI = __DEV__;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
export const enableProfilerTimer = __PROFILE__;
export const enableSchedulerTracing = __PROFILE__;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ export const debugRenderPhaseSideEffectsForStrictMode = false;
export const enableUserTimingAPI = __DEV__;
export const enableSuspense = false;
export const warnAboutDeprecatedLifecycles = false;
export const warnAboutLegacyContextAPI = false;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
export const enableProfilerTimer = __PROFILE__;
export const enableSchedulerTracing = __PROFILE__;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ export const {

// The rest of the flags are static for better dead code elimination.
export const enableUserTimingAPI = __DEV__;
export const warnAboutLegacyContextAPI = __DEV__;
export const enableProfilerTimer = __PROFILE__;
export const enableSchedulerTracing = __PROFILE__;
export const enableSuspenseServerRenderer = false;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ export const enableSuspense = false;
export const enableUserTimingAPI = __DEV__;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
export const warnAboutDeprecatedLifecycles = false;
export const warnAboutLegacyContextAPI = false;
export const enableProfilerTimer = __PROFILE__;
export const enableSchedulerTracing = __PROFILE__;
export const enableSuspenseServerRenderer = false;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.persistent.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ export const debugRenderPhaseSideEffectsForStrictMode = false;
export const enableUserTimingAPI = __DEV__;
export const enableSuspense = false;
export const warnAboutDeprecatedLifecycles = false;
export const warnAboutLegacyContextAPI = false;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
export const enableProfilerTimer = __PROFILE__;
export const enableSchedulerTracing = __PROFILE__;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ export const debugRenderPhaseSideEffectsForStrictMode = false;
export const enableUserTimingAPI = __DEV__;
export const enableSuspense = false;
export const warnAboutDeprecatedLifecycles = false;
export const warnAboutLegacyContextAPI = false;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
export const enableProfilerTimer = false;
export const enableSchedulerTracing = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ export const debugRenderPhaseSideEffectsForStrictMode = false;
export const enableUserTimingAPI = __DEV__;
export const enableSuspense = true;
export const warnAboutDeprecatedLifecycles = false;
export const warnAboutLegacyContextAPI = false;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
export const enableProfilerTimer = false;
export const enableSchedulerTracing = false;
Expand Down
3 changes: 0 additions & 3 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@ export const {
disableInputAttributeSyncing,
} = require('ReactFeatureFlags');

// The rest of the flags are static for better dead code elimination.
export const warnAboutLegacyContextAPI = __DEV__;

// In www, we have experimental support for gathering data
// from User Timing API calls in production. By default, we
// only emit performance.mark/measure calls in __DEV__. But if
Expand Down
1 change: 0 additions & 1 deletion scripts/rollup/shims/react-native-fb/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ const ReactFeatureFlags = {
debugRenderPhaseSideEffects: false,
debugRenderPhaseSideEffectsForStrictMode: false,
warnAboutDeprecatedLifecycles: true,
warnAboutLegacyContextAPI: true,
};

module.exports = ReactFeatureFlags;

0 comments on commit 312d3ba

Please sign in to comment.