From 8baff7e01c159aeb50a16218ba62e2d918333e53 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 21 Jan 2020 17:01:54 +0000 Subject: [PATCH 1/4] Update legacy context warning Fix flow --- .../src/ReactStrictModeWarnings.js | 40 ++++++--- .../ReactIncremental-test.internal.js | 87 ++++++------------- ...tIncrementalErrorHandling-test.internal.js | 32 +++---- .../ReactIncrementalPerf-test.internal.js | 2 +- .../ReactNewContext-test.internal.js | 3 +- .../src/__tests__/ReactStrictMode-test.js | 5 +- 6 files changed, 75 insertions(+), 94 deletions(-) diff --git a/packages/react-reconciler/src/ReactStrictModeWarnings.js b/packages/react-reconciler/src/ReactStrictModeWarnings.js index 29af663d90ec..b61eb173d2e3 100644 --- a/packages/react-reconciler/src/ReactStrictModeWarnings.js +++ b/packages/react-reconciler/src/ReactStrictModeWarnings.js @@ -287,7 +287,6 @@ if (__DEV__) { // Tracks components we have already warned about. const didWarnAboutLegacyContext = new Set(); - ReactStrictModeWarnings.recordLegacyContextWarning = ( fiber: Fiber, instance: any, @@ -305,7 +304,6 @@ if (__DEV__) { if (didWarnAboutLegacyContext.has(fiber.type)) { return; } - let warningsForRoot = pendingLegacyContextWarning.get(strictRoot); if ( @@ -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' + '\n\nLearn more about this warning here: https://fb.me/react-legacy-context' + '%s', - sortedNames, - strictRootComponentStack, + (componentStacks.get(mostFrequentStack): any).name, + mostFrequentStack, ); }, ); diff --git a/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js index e39b055f79c5..c20c2e573294 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js @@ -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( @@ -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', ); }); @@ -2000,14 +1998,11 @@ describe('ReactIncremental', () => { } ReactNoop.render(); - 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 {}', @@ -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}, ); expect(ops).toEqual([ 'Recurse {}', @@ -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', ); }); @@ -2196,14 +2190,11 @@ describe('ReactIncremental', () => { , ); - 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 {}', @@ -2292,14 +2283,11 @@ describe('ReactIncremental', () => { , ); - 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 {}', @@ -2365,14 +2353,11 @@ describe('ReactIncremental', () => { // Init ReactNoop.render(); - 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 @@ -2419,14 +2404,11 @@ describe('ReactIncremental', () => { // Init ReactNoop.render(); - 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 @@ -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([ @@ -2622,14 +2604,11 @@ describe('ReactIncremental', () => { , ); - 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(); @@ -2688,14 +2667,11 @@ describe('ReactIncremental', () => { , ); - 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(); @@ -2763,14 +2739,11 @@ describe('ReactIncremental', () => { , ); - 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(); @@ -2848,14 +2821,11 @@ describe('ReactIncremental', () => { , ); - 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(); @@ -2956,10 +2926,9 @@ describe('ReactIncremental', () => { ReactNoop.render(); 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, diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index 519c2d3f3a85..8fb3c7527fab 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -1148,14 +1148,11 @@ describe('ReactIncrementalErrorHandling', () => { , ); - 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' @@ -1649,19 +1646,16 @@ describe('ReactIncrementalErrorHandling', () => { ReactNoop.render(); expect(() => { expect(Scheduler).toFlushAndThrow('Oops!'); - }).toErrorDev( - [ - 'Warning: The 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 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', + ]); }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js index a88cd8e3a89f..08c569c23950 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js @@ -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(); addComment('Update'); diff --git a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js index 03c5ad4eec66..f6e5b5b4a148 100644 --- a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js @@ -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')]); diff --git a/packages/react/src/__tests__/ReactStrictMode-test.js b/packages/react/src/__tests__/ReactStrictMode-test.js index 15b2ce719c76..b6e612da507e 100644 --- a/packages/react/src/__tests__/ReactStrictMode-test.js +++ b/packages/react/src/__tests__/ReactStrictMode-test.js @@ -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 **)', From 4cf77031138d6b02e6d3cee138dac09c0cc06619 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 21 Jan 2020 20:35:13 +0000 Subject: [PATCH 2/4] Address feedback --- .../src/ReactStrictModeWarnings.js | 50 ++++++++++--------- .../ReactIncremental-test.internal.js | 28 +++++------ ...tIncrementalErrorHandling-test.internal.js | 4 +- .../ReactNewContext-test.internal.js | 2 +- .../src/__tests__/ReactStrictMode-test.js | 4 +- 5 files changed, 46 insertions(+), 42 deletions(-) diff --git a/packages/react-reconciler/src/ReactStrictModeWarnings.js b/packages/react-reconciler/src/ReactStrictModeWarnings.js index b61eb173d2e3..34654d9c3111 100644 --- a/packages/react-reconciler/src/ReactStrictModeWarnings.js +++ b/packages/react-reconciler/src/ReactStrictModeWarnings.js @@ -287,6 +287,7 @@ if (__DEV__) { // Tracks components we have already warned about. const didWarnAboutLegacyContext = new Set(); + ReactStrictModeWarnings.recordLegacyContextWarning = ( fiber: Fiber, instance: any, @@ -304,6 +305,7 @@ if (__DEV__) { if (didWarnAboutLegacyContext.has(fiber.type)) { return; } + let warningsForRoot = pendingLegacyContextWarning.get(strictRoot); if ( @@ -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, - ); }, ); }; diff --git a/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js index c20c2e573294..1acb4e49c4b8 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js @@ -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( @@ -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', ); }); @@ -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 {}', @@ -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}, ); @@ -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', ); }); @@ -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 {}', @@ -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 {}', @@ -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 @@ -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 @@ -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}, ); @@ -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(); @@ -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(); @@ -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(); @@ -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(); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index 8fb3c7527fab..493cd79cc8bd 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -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' @@ -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', ]); }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js index f6e5b5b4a148..34d854f78cdd 100644 --- a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js @@ -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')]); diff --git a/packages/react/src/__tests__/ReactStrictMode-test.js b/packages/react/src/__tests__/ReactStrictMode-test.js index b6e612da507e..87b153ef61e6 100644 --- a/packages/react/src/__tests__/ReactStrictMode-test.js +++ b/packages/react/src/__tests__/ReactStrictMode-test.js @@ -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 **)' + From 12d95b61f45724348c1d50e8d08fc693c1f406e6 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 23 Jan 2020 11:33:40 +0000 Subject: [PATCH 3/4] Address more feedback Address more feedback Address more feedback --- .../src/ReactStrictModeWarnings.js | 91 +++++++++---------- .../ReactIncremental-test.internal.js | 27 +++--- 2 files changed, 57 insertions(+), 61 deletions(-) diff --git a/packages/react-reconciler/src/ReactStrictModeWarnings.js b/packages/react-reconciler/src/ReactStrictModeWarnings.js index 34654d9c3111..684f830df1bc 100644 --- a/packages/react-reconciler/src/ReactStrictModeWarnings.js +++ b/packages/react-reconciler/src/ReactStrictModeWarnings.js @@ -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 = ( @@ -300,17 +301,23 @@ if (__DEV__) { ); return; } + const type = fiber.type; - // Dedup strategy: Warn once per component. - if (didWarnAboutLegacyContext.has(fiber.type)) { + // Update legacy context counts + const warningCount = legacyContextCounts.get(type) || 0; + // Increase warning count by 1 + legacyContextCounts.set(type, warningCount + 1); + + // Dedup strategy: Warn once per component + if (didWarnAboutLegacyContext.has(type)) { return; } 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) { @@ -324,49 +331,41 @@ if (__DEV__) { ReactStrictModeWarnings.flushLegacyContextWarning = () => { ((pendingLegacyContextWarning: any): FiberToFiberComponentsMap).forEach( (fiberArray: FiberArray, strictRoot) => { - const componentStacks = new Map(); - - fiberArray.forEach(fiber => { - 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, - stack: componentStack, + 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; + }) + .forEach((fiber, i) => { + const type = fiber.type; + if (didWarnAboutLegacyContext.has(type)) { + return; + } + didWarnAboutLegacyContext.add(type); + // Build up the string of comma separated component names + componentNames += + (getComponentName(fiber.type) || 'Component') + + (i === fibers - 1 ? '' : ', '); }); - didWarnAboutLegacyContext.add(fiber.type); - }); - - // 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, - ); - } + const mostFrequentFiber = fiberArray[0]; + const stack = mostFrequentFiber + ? getStackByFiberInDevAndProd(mostFrequentFiber) + : ''; + + 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, + stack, + ); }, ); }; diff --git a/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js index 1acb4e49c4b8..0dcca2ae2f38 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js @@ -2036,20 +2036,17 @@ describe('ReactIncremental', () => { }; ReactNoop.render(); - expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev( - [ - 'Warning: The 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: 0}, - ); + expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev([ + 'Warning: The 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}', @@ -2114,7 +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', + 'Please update the following components: ShowLocale, Intl', ); }); From 32cdbdafc3111c3de9e5e66019a9c50706399a87 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Fri, 24 Jan 2020 17:50:51 +0000 Subject: [PATCH 4/4] Address more feedback --- .../react-reconciler/src/ReactStrictModeWarnings.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/react-reconciler/src/ReactStrictModeWarnings.js b/packages/react-reconciler/src/ReactStrictModeWarnings.js index 684f830df1bc..d7421bd15409 100644 --- a/packages/react-reconciler/src/ReactStrictModeWarnings.js +++ b/packages/react-reconciler/src/ReactStrictModeWarnings.js @@ -303,16 +303,16 @@ if (__DEV__) { } const type = fiber.type; - // Update legacy context counts - const warningCount = legacyContextCounts.get(type) || 0; - // Increase warning count by 1 - legacyContextCounts.set(type, warningCount + 1); - // Dedup strategy: Warn once per component if (didWarnAboutLegacyContext.has(type)) { 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 (