Skip to content

Commit

Permalink
Address feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@trueadm
trueadm committed Jan 21, 2020
1 parent 8baff7e commit 4cf7703
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 42 deletions.
50 changes: 27 additions & 23 deletions packages/react-reconciler/src/ReactStrictModeWarnings.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ if (__DEV__) {

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

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

let warningsForRoot = pendingLegacyContextWarning.get(strictRoot);

if (
Expand Down Expand Up @@ -335,34 +337,36 @@ if (__DEV__) {
componentStacks.set(componentStack, {
count: count + 1,
name: componentName,
stack: componentStack,
});
didWarnAboutLegacyContext.add(fiber.type);
});

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;
}
// Get the stacks from our componentStacks Map
const stacks = Array.from(componentStacks.values());

// Sort the stacks by their counts
stacks.sort((a, b) => b.count - a.count);

const mostFrequentStack = stacks[0];

if (mostFrequentStack) {
// We map to a Set to remove duplicate component names
const componentNames = Array.from(
new Set(stacks.map(stack => stack.name)),
).join(', ');

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\nLearn more about this warning here: https://fb.me/react-legacy-context' +
'%s',
componentNames,
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 component: %s' +
'\n\nLearn more about this warning here: https://fb.me/react-legacy-context' +
'%s',
(componentStacks.get(mostFrequentStack): any).name,
mostFrequentStack,
);
},
);
};
Expand Down
28 changes: 14 additions & 14 deletions packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1916,7 +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 component: Intl',
'Please update the following components: Intl, ShowLocale, ShowBoth',
);

ReactNoop.render(
Expand Down Expand Up @@ -1972,7 +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 component: Router',
'Please update the following components: Router, ShowRoute',
);
});

Expand Down Expand Up @@ -2002,7 +2002,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 component: Recurse',
'Please update the following components: Recurse',
);
expect(ops).toEqual([
'Recurse {}',
Expand Down Expand Up @@ -2046,7 +2046,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 component: Recurse',
'Please update the following components: Recurse',
],
{withoutStack: 0},
);
Expand Down Expand Up @@ -2114,7 +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 component: Intl',
'Please update the following components: Intl, ShowLocale',
);
});

Expand Down Expand Up @@ -2194,7 +2194,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 component: Intl',
'Please update the following components: Intl, ShowLocaleClass, ShowLocaleFn',
);
expect(ops).toEqual([
'Intl:read {}',
Expand Down Expand Up @@ -2287,7 +2287,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 component: Intl',
'Please update the following components: Intl, ShowLocaleClass, ShowLocaleFn',
);
expect(ops).toEqual([
'Intl:read {}',
Expand Down Expand Up @@ -2357,7 +2357,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 component: Child',
'Please update the following components: Child',
);

// Trigger an update in the middle of the tree
Expand Down Expand Up @@ -2408,7 +2408,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 component: ContextProvider',
'Please update the following components: ContextProvider',
);

// Trigger an update in the middle of the tree
Expand Down Expand Up @@ -2459,7 +2459,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 component: MyComponent',
'Please update the following components: MyComponent',
],
{withoutStack: 1},
);
Expand Down Expand Up @@ -2608,7 +2608,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 component: TopContextProvider',
'Please update the following components: TopContextProvider, Child',
);
expect(rendered).toEqual(['count:0']);
instance.updateCount();
Expand Down Expand Up @@ -2671,7 +2671,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 component: TopContextProvider',
'Please update the following components: TopContextProvider, MiddleContextProvider, Child',
);
expect(rendered).toEqual(['count:0']);
instance.updateCount();
Expand Down Expand Up @@ -2743,7 +2743,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 component: TopContextProvider',
'Please update the following components: TopContextProvider, MiddleContextProvider, Child',
);
expect(rendered).toEqual(['count:0']);
instance.updateCount();
Expand Down Expand Up @@ -2825,7 +2825,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 component: TopContextProvider',
'Please update the following components: TopContextProvider, MiddleContextProvider, Child',
);
expect(rendered).toEqual(['count:0, name:brian']);
topInstance.updateCount();
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1152,7 +1152,7 @@ describe('ReactIncrementalErrorHandling', () => {
'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',
'Please update the following components: Provider, Connector',
);

// If the context stack does not unwind, span will get 'abcde'
Expand Down Expand Up @@ -1655,7 +1655,7 @@ describe('ReactIncrementalErrorHandling', () => {
'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',
'Please update the following components: Provider',
]);
});
});
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1197,7 +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 component: LegacyProvider',
'Please update the following components: LegacyProvider',
);
expect(ReactNoop.getChildren()).toEqual([span('Child')]);

Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/__tests__/ReactStrictMode-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -874,8 +874,8 @@ 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 component: ' +
'LegacyContextProvider' +
'\n\nPlease update the following components: ' +
'LegacyContextProvider, LegacyContextConsumer, FunctionalLegacyContextConsumer' +
'\n\nLearn more about this warning here: ' +
'https://fb.me/react-legacy-context' +
'\n in LegacyContextProvider (at **)' +
Expand Down

0 comments on commit 4cf7703

Please sign in to comment.