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

Jump to bottom

Update legacy context warning message #17882

Closed
wants to merge 4 commits into from
Closed

Update legacy context warning message #17882

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8baff7e
Update legacy context warning
trueadm Jan 21, 2020
4cf7703
Address feedback
trueadm Jan 21, 2020
12d95b6
Address more feedback
trueadm Jan 23, 2020
32cdbda
Address more feedback
trueadm Jan 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 29 additions & 11 deletions packages/react-reconciler/src/ReactStrictModeWarnings.js
View file
Open in desktop
Expand Up @@ -287,7 +287,6 @@ if (__DEV__) {

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

ReactStrictModeWarnings.recordLegacyContextWarning = (
fiber: Fiber,
instance: any,
Expand All @@ -305,7 +304,6 @@ if (__DEV__) {
if (didWarnAboutLegacyContext.has(fiber.type)) {
return;
}

let warningsForRoot = pendingLegacyContextWarning.get(strictRoot);

if (
Expand All @@ -324,26 +322,46 @@ if (__DEV__) {
ReactStrictModeWarnings.flushLegacyContextWarning = () => {
((pendingLegacyContextWarning: any): FiberToFiberComponentsMap).forEach(
(fiberArray: FiberArray, strictRoot) => {
const uniqueNames = new Set();
const componentStacks = new Map();

fiberArray.forEach(fiber => {
uniqueNames.add(getComponentName(fiber.type) || 'Component');
const componentName = getComponentName(fiber.type) || 'Component';
const componentStack = getStackByFiberInDevAndProd(fiber);
let count = 0;
if (componentStacks.has(componentStack)) {
({count} = (componentStacks.get(componentStack): any));
}
// Increase count by 1
componentStacks.set(componentStack, {
count: count + 1,
name: componentName,
});
didWarnAboutLegacyContext.add(fiber.type);
});

const sortedNames = setToSortedString(uniqueNames);
const strictRootComponentStack = getStackByFiberInDevAndProd(
strictRoot,
);
const stacks = Array.from(componentStacks.keys());
// Find the most frequently found component stack and use that
let currentCount = 0;
let mostFrequentStack = null;

for (let i = 0; i < stacks.length; i++) {
const stack = stacks[i];
let {count} = (componentStacks.get(stack): any);
if (count > currentCount || mostFrequentStack === null) {
currentCount = count;
mostFrequentStack = stack;
}
}

console.error(
'Legacy context API has been detected within a strict-mode tree.' +
'\n\nThe old API will be supported in all 16.x releases, but applications ' +
'using it should migrate to the new version.' +
'\n\nPlease update the following components: %s' +
'\n\nPlease update the following component: %s' +
trueadm marked this conversation as resolved.
Show resolved Hide resolved
'\n\nLearn more about this warning here: https://fb.me/react-legacy-context' +
'%s',
sortedNames,
strictRootComponentStack,
(componentStacks.get(mostFrequentStack): any).name,
mostFrequentStack,
);
},
);
Expand Down
87 changes: 28 additions & 59 deletions packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js
View file
Open in desktop
Expand Up @@ -1916,8 +1916,7 @@ describe('ReactIncremental', () => {
'Legacy context API has been detected within a strict-mode tree.\n\n' +
'The old API will be supported in all 16.x releases, but applications ' +
'using it should migrate to the new version.\n\n' +
'Please update the following components: Intl, ShowBoth, ShowLocale',
{withoutStack: true},
'Please update the following component: Intl',
);

ReactNoop.render(
Expand Down Expand Up @@ -1973,8 +1972,7 @@ describe('ReactIncremental', () => {
'Legacy context API has been detected within a strict-mode tree.\n\n' +
'The old API will be supported in all 16.x releases, but applications ' +
'using it should migrate to the new version.\n\n' +
'Please update the following components: Router, ShowRoute',
{withoutStack: true},
'Please update the following component: Router',
);
});

Expand All @@ -2000,14 +1998,11 @@ describe('ReactIncremental', () => {
}

ReactNoop.render(<Recurse />);
expect(() =>
expect(Scheduler).toFlushWithoutYielding(),
).toErrorDev(
expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev(
'Legacy context API has been detected within a strict-mode tree.\n\n' +
'The old API will be supported in all 16.x releases, but applications ' +
'using it should migrate to the new version.\n\n' +
'Please update the following components: Recurse',
{withoutStack: true},
'Please update the following component: Recurse',
);
expect(ops).toEqual([
'Recurse {}',
Expand Down Expand Up @@ -2051,9 +2046,9 @@ describe('ReactIncremental', () => {
'Legacy context API has been detected within a strict-mode tree.\n\n' +
'The old API will be supported in all 16.x releases, but applications ' +
'using it should migrate to the new version.\n\n' +
'Please update the following components: Recurse',
'Please update the following component: Recurse',
],
{withoutStack: 1},
{withoutStack: 0},
trueadm marked this conversation as resolved.
Show resolved Hide resolved
);
expect(ops).toEqual([
'Recurse {}',
Expand Down Expand Up @@ -2119,8 +2114,7 @@ describe('ReactIncremental', () => {
'Legacy context API has been detected within a strict-mode tree.\n\n' +
'The old API will be supported in all 16.x releases, but applications ' +
'using it should migrate to the new version.\n\n' +
'Please update the following components: Intl, ShowLocale',
{withoutStack: true},
'Please update the following component: Intl',
);
});

Expand Down Expand Up @@ -2196,14 +2190,11 @@ describe('ReactIncremental', () => {
</IndirectionFn>
</Intl>,
);
expect(() =>
expect(Scheduler).toFlushWithoutYielding(),
).toErrorDev(
expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev(
'Legacy context API has been detected within a strict-mode tree.\n\n' +
'The old API will be supported in all 16.x releases, but applications ' +
'using it should migrate to the new version.\n\n' +
'Please update the following components: Intl, ShowLocaleClass, ShowLocaleFn',
{withoutStack: true},
'Please update the following component: Intl',
);
expect(ops).toEqual([
'Intl:read {}',
Expand Down Expand Up @@ -2292,14 +2283,11 @@ describe('ReactIncremental', () => {
</IndirectionFn>
</Stateful>,
);
expect(() =>
expect(Scheduler).toFlushWithoutYielding(),
).toErrorDev(
expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev(
'Legacy context API has been detected within a strict-mode tree.\n\n' +
'The old API will be supported in all 16.x releases, but applications ' +
'using it should migrate to the new version.\n\n' +
'Please update the following components: Intl, ShowLocaleClass, ShowLocaleFn',
{withoutStack: true},
'Please update the following component: Intl',
);
expect(ops).toEqual([
'Intl:read {}',
Expand Down Expand Up @@ -2365,14 +2353,11 @@ describe('ReactIncremental', () => {

// Init
ReactNoop.render(<Root />);
expect(() =>
expect(Scheduler).toFlushWithoutYielding(),
).toErrorDev(
expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev(
'Legacy context API has been detected within a strict-mode tree.\n\n' +
'The old API will be supported in all 16.x releases, but applications ' +
'using it should migrate to the new version.\n\n' +
'Please update the following components: Child',
{withoutStack: true},
'Please update the following component: Child',
);

// Trigger an update in the middle of the tree
Expand Down Expand Up @@ -2419,14 +2404,11 @@ describe('ReactIncremental', () => {

// Init
ReactNoop.render(<Root />);
expect(() =>
expect(Scheduler).toFlushWithoutYielding(),
).toErrorDev(
expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev(
'Legacy context API has been detected within a strict-mode tree.\n\n' +
'The old API will be supported in all 16.x releases, but applications ' +
'using it should migrate to the new version.\n\n' +
'Please update the following components: ContextProvider',
{withoutStack: true},
'Please update the following component: ContextProvider',
);

// Trigger an update in the middle of the tree
Expand Down Expand Up @@ -2477,9 +2459,9 @@ describe('ReactIncremental', () => {
'Legacy context API has been detected within a strict-mode tree.\n\n' +
'The old API will be supported in all 16.x releases, but applications ' +
'using it should migrate to the new version.\n\n' +
'Please update the following components: MyComponent',
'Please update the following component: MyComponent',
],
{withoutStack: true},
{withoutStack: 1},
);

expect(ops).toEqual([
Expand Down Expand Up @@ -2622,14 +2604,11 @@ describe('ReactIncremental', () => {
</TopContextProvider>,
);

expect(() =>
expect(Scheduler).toFlushWithoutYielding(),
).toErrorDev(
expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev(
'Legacy context API has been detected within a strict-mode tree.\n\n' +
'The old API will be supported in all 16.x releases, but applications ' +
'using it should migrate to the new version.\n\n' +
'Please update the following components: Child, TopContextProvider',
{withoutStack: true},
'Please update the following component: TopContextProvider',
);
expect(rendered).toEqual(['count:0']);
instance.updateCount();
Expand Down Expand Up @@ -2688,14 +2667,11 @@ describe('ReactIncremental', () => {
</TopContextProvider>,
);

expect(() =>
expect(Scheduler).toFlushWithoutYielding(),
).toErrorDev(
expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev(
'Legacy context API has been detected within a strict-mode tree.\n\n' +
'The old API will be supported in all 16.x releases, but applications ' +
'using it should migrate to the new version.\n\n' +
'Please update the following components: Child, MiddleContextProvider, TopContextProvider',
{withoutStack: true},
'Please update the following component: TopContextProvider',
);
expect(rendered).toEqual(['count:0']);
instance.updateCount();
Expand Down Expand Up @@ -2763,14 +2739,11 @@ describe('ReactIncremental', () => {
</TopContextProvider>,
);

expect(() =>
expect(Scheduler).toFlushWithoutYielding(),
).toErrorDev(
expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev(
'Legacy context API has been detected within a strict-mode tree.\n\n' +
'The old API will be supported in all 16.x releases, but applications ' +
'using it should migrate to the new version.\n\n' +
'Please update the following components: Child, MiddleContextProvider, TopContextProvider',
{withoutStack: true},
'Please update the following component: TopContextProvider',
);
expect(rendered).toEqual(['count:0']);
instance.updateCount();
Expand Down Expand Up @@ -2848,14 +2821,11 @@ describe('ReactIncremental', () => {
</TopContextProvider>,
);

expect(() =>
expect(Scheduler).toFlushWithoutYielding(),
).toErrorDev(
expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev(
'Legacy context API has been detected within a strict-mode tree.\n\n' +
'The old API will be supported in all 16.x releases, but applications ' +
'using it should migrate to the new version.\n\n' +
'Please update the following components: Child, MiddleContextProvider, TopContextProvider',
{withoutStack: true},
'Please update the following component: TopContextProvider',
);
expect(rendered).toEqual(['count:0, name:brian']);
topInstance.updateCount();
Expand Down Expand Up @@ -2956,10 +2926,9 @@ describe('ReactIncremental', () => {
ReactNoop.render(<Boundary />);
expect(() => {
expect(Scheduler).toFlushWithoutYielding();
}).toErrorDev(
['Legacy context API has been detected within a strict-mode tree'],
{withoutStack: true},
);
}).toErrorDev([
'Legacy context API has been detected within a strict-mode tree',
]);
}

// First, verify that this code path normally receives Fibers as keys,
Expand Down
32 changes: 13 additions & 19 deletions packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js
View file
Open in desktop
Expand Up @@ -1148,14 +1148,11 @@ describe('ReactIncrementalErrorHandling', () => {
<Connector />
</Provider>,
);
expect(() =>
expect(Scheduler).toFlushWithoutYielding(),
).toErrorDev(
expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev(
'Legacy context API has been detected within a strict-mode tree.\n\n' +
'The old API will be supported in all 16.x releases, but ' +
'applications using it should migrate to the new version.\n\n' +
'Please update the following components: Connector, Provider',
{withoutStack: true},
'Please update the following component: Provider',
);

// If the context stack does not unwind, span will get 'abcde'
Expand Down Expand Up @@ -1649,19 +1646,16 @@ describe('ReactIncrementalErrorHandling', () => {
ReactNoop.render(<Provider />);
expect(() => {
expect(Scheduler).toFlushAndThrow('Oops!');
}).toErrorDev(
[
'Warning: The <Provider /> component appears to be a function component that returns a class instance. ' +
'Change Provider to a class that extends React.Component instead. ' +
"If you can't use a class try assigning the prototype on the function as a workaround. " +
'`Provider.prototype = React.Component.prototype`. ' +
"Don't use an arrow function since it cannot be called with `new` by React.",
'Legacy context API has been detected within a strict-mode tree.\n\n' +
'The old API will be supported in all 16.x releases, but ' +
'applications using it should migrate to the new version.\n\n' +
'Please update the following components: Provider',
],
{withoutStack: 1},
);
}).toErrorDev([
'Warning: The <Provider /> component appears to be a function component that returns a class instance. ' +
'Change Provider to a class that extends React.Component instead. ' +
"If you can't use a class try assigning the prototype on the function as a workaround. " +
'`Provider.prototype = React.Component.prototype`. ' +
"Don't use an arrow function since it cannot be called with `new` by React.",
'Legacy context API has been detected within a strict-mode tree.\n\n' +
'The old API will be supported in all 16.x releases, but ' +
'applications using it should migrate to the new version.\n\n' +
'Please update the following component: Provider',
]);
});
});
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js
View file
Open in desktop
Expand Up @@ -371,7 +371,7 @@ describe('ReactDebugFiberPerf', () => {
'Using UNSAFE_componentWillUpdate in strict mode is not recommended',
'Legacy context API has been detected within a strict-mode tree',
],
{withoutStack: true},
{withoutStack: 3},
);
ReactNoop.render(<AllLifecycles />);
addComment('Update');
Expand Down
3 changes: 1 addition & 2 deletions packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js
View file
Open in desktop
Expand Up @@ -1197,8 +1197,7 @@ describe('ReactNewContext', () => {
'Legacy context API has been detected within a strict-mode tree.\n\n' +
'The old API will be supported in all 16.x releases, but applications ' +
'using it should migrate to the new version.\n\n' +
'Please update the following components: LegacyProvider',
{withoutStack: true},
'Please update the following component: LegacyProvider',
);
expect(ReactNoop.getChildren()).toEqual([span('Child')]);

Expand Down
5 changes: 3 additions & 2 deletions packages/react/src/__tests__/ReactStrictMode-test.js
View file
Open in desktop
Expand Up @@ -874,10 +874,11 @@ describe('context legacy', () => {
'Warning: Legacy context API has been detected within a strict-mode tree.' +
'\n\nThe old API will be supported in all 16.x releases, but applications ' +
'using it should migrate to the new version.' +
'\n\nPlease update the following components: ' +
'FunctionalLegacyContextConsumer, LegacyContextConsumer, LegacyContextProvider' +
'\n\nPlease update the following component: ' +
'LegacyContextProvider' +
'\n\nLearn more about this warning here: ' +
'https://fb.me/react-legacy-context' +
'\n in LegacyContextProvider (at **)' +
'\n in StrictMode (at **)' +
'\n in div (at **)' +
'\n in Root (at **)',
Expand Down