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 all commits
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
53 changes: 37 additions & 16 deletions packages/react-reconciler/src/ReactStrictModeWarnings.js
View file
Open in desktop
Expand Up @@ -286,6 +286,7 @@ if (__DEV__) {
let pendingLegacyContextWarning: FiberToFiberComponentsMap = new Map();

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

ReactStrictModeWarnings.recordLegacyContextWarning = (
Expand All @@ -300,17 +301,23 @@ if (__DEV__) {
);
return;
}
const type = fiber.type;

// Dedup strategy: Warn once per component.
if (didWarnAboutLegacyContext.has(fiber.type)) {
// Dedup strategy: Warn once per component
if (didWarnAboutLegacyContext.has(type)) {
trueadm marked this conversation as resolved.
Show resolved Hide resolved
return;
}

// Update legacy context counts
const warningCount = legacyContextCounts.get(type) || 0;
// Increase warning count by 1
legacyContextCounts.set(type, warningCount + 1);

let warningsForRoot = pendingLegacyContextWarning.get(strictRoot);

if (
fiber.type.contextTypes != null ||
fiber.type.childContextTypes != null ||
type.contextTypes != null ||
type.childContextTypes != null ||
(instance !== null && typeof instance.getChildContext === 'function')
) {
if (warningsForRoot === undefined) {
Expand All @@ -324,16 +331,30 @@ if (__DEV__) {
ReactStrictModeWarnings.flushLegacyContextWarning = () => {
((pendingLegacyContextWarning: any): FiberToFiberComponentsMap).forEach(
(fiberArray: FiberArray, strictRoot) => {
const uniqueNames = new Set();
fiberArray.forEach(fiber => {
uniqueNames.add(getComponentName(fiber.type) || 'Component');
didWarnAboutLegacyContext.add(fiber.type);
});

const sortedNames = setToSortedString(uniqueNames);
const strictRootComponentStack = getStackByFiberInDevAndProd(
strictRoot,
);
const fibers = fiberArray.length;
let componentNames = '';

fiberArray
.sort((a, b) => {
const aCount = legacyContextCounts.get(a.type) || 0;
const bCount = legacyContextCounts.get(b.type) || 0;
return bCount - aCount;
})
trueadm marked this conversation as resolved.
Show resolved Hide resolved
.forEach((fiber, i) => {
const type = fiber.type;
if (didWarnAboutLegacyContext.has(type)) {
trueadm marked this conversation as resolved.
Show resolved Hide resolved
return;
}
didWarnAboutLegacyContext.add(type);
// Build up the string of comma separated component names
componentNames +=
(getComponentName(fiber.type) || 'Component') +
(i === fibers - 1 ? '' : ', ');
});
const mostFrequentFiber = fiberArray[0];
const stack = mostFrequentFiber
? getStackByFiberInDevAndProd(mostFrequentFiber)
: '';

console.error(
'Legacy context API has been detected within a strict-mode tree.' +
Expand All @@ -342,8 +363,8 @@ if (__DEV__) {
'\n\nPlease update the following components: %s' +
'\n\nLearn more about this warning here: https://fb.me/react-legacy-context' +
'%s',
sortedNames,
strictRootComponentStack,
componentNames,
stack,
);
},
);
Expand Down
94 changes: 30 additions & 64 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 components: Intl, ShowLocale, ShowBoth',
trueadm marked this conversation as resolved.
Show resolved Hide resolved
);

ReactNoop.render(
Expand Down Expand Up @@ -1974,7 +1973,6 @@ describe('ReactIncremental', () => {
'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},
);
});

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},
);
expect(ops).toEqual([
'Recurse {}',
Expand Down Expand Up @@ -2041,20 +2036,17 @@ describe('ReactIncremental', () => {
};

ReactNoop.render(<Recurse />);
expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev(
[
'Warning: The <Recurse /> component appears to be a function component that returns a class instance. ' +
'Change Recurse 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. " +
'`Recurse.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: Recurse',
],
{withoutStack: 1},
);
expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev([
'Warning: The <Recurse /> component appears to be a function component that returns a class instance. ' +
'Change Recurse 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. " +
'`Recurse.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: Recurse',
]);
expect(ops).toEqual([
'Recurse {}',
'Recurse {"n":2}',
Expand Down Expand Up @@ -2119,8 +2111,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 components: ShowLocale, Intl',
);
});

Expand Down Expand Up @@ -2196,14 +2187,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},
);
expect(ops).toEqual([
'Intl:read {}',
Expand Down Expand Up @@ -2292,14 +2280,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},
);
expect(ops).toEqual([
'Intl:read {}',
Expand Down Expand Up @@ -2365,14 +2350,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},
);

// Trigger an update in the middle of the tree
Expand Down Expand Up @@ -2419,14 +2401,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},
);

// Trigger an update in the middle of the tree
Expand Down Expand Up @@ -2479,7 +2458,7 @@ describe('ReactIncremental', () => {
'using it should migrate to the new version.\n\n' +
'Please update the following components: MyComponent',
],
{withoutStack: true},
{withoutStack: 1},
);

expect(ops).toEqual([
Expand Down Expand Up @@ -2622,14 +2601,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 components: TopContextProvider, Child',
);
expect(rendered).toEqual(['count:0']);
instance.updateCount();
Expand Down Expand Up @@ -2688,14 +2664,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 components: TopContextProvider, MiddleContextProvider, Child',
);
expect(rendered).toEqual(['count:0']);
instance.updateCount();
Expand Down Expand Up @@ -2763,14 +2736,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 components: TopContextProvider, MiddleContextProvider, Child',
);
expect(rendered).toEqual(['count:0']);
instance.updateCount();
Expand Down Expand Up @@ -2848,14 +2818,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 components: TopContextProvider, MiddleContextProvider, Child',
);
expect(rendered).toEqual(['count:0, name:brian']);
topInstance.updateCount();
Expand Down Expand Up @@ -2956,10 +2923,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 components: Provider, Connector',
);

// 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 components: 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
1 change: 0 additions & 1 deletion packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js
View file
Open in desktop
Expand Up @@ -1198,7 +1198,6 @@ describe('ReactNewContext', () => {
'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},
);
expect(ReactNoop.getChildren()).toEqual([span('Child')]);

Expand Down
3 changes: 2 additions & 1 deletion packages/react/src/__tests__/ReactStrictMode-test.js
View file
Open in desktop
Expand Up @@ -875,9 +875,10 @@ describe('context legacy', () => {
'\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' +
'LegacyContextProvider, LegacyContextConsumer, FunctionalLegacyContextConsumer' +
'\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