From 5b57bc6e31689ad715161cdcfa1b1d31eac2ba3b Mon Sep 17 00:00:00 2001 From: Luna Ruan Date: Tue, 21 Sep 2021 15:00:11 -0700 Subject: [PATCH] [Draft] don't patch console during first render (#22308) Previously, DevTools always overrode the native console to dim or supress StrictMode double logging. It also overrode console.log (in addition to console.error and console.warn). However, this changes the location shown by the browser console, which causes a bad developer experience. There is currently a TC39 proposal that would allow us to extend console without breaking developer experience, but in the meantime this PR changes the StrictMode console override behavior so that we only patch the console during the StrictMode double render so that, during the first render, the location points to developer code rather than our DevTools console code. --- .../src/__tests__/console-test.js | 222 ++++++++--- .../src/backend/console.js | 337 ++++++++++------- .../src/backend/legacy/renderer.js | 6 + .../src/backend/renderer.js | 4 + .../src/backend/types.js | 7 +- packages/react-devtools-shared/src/hook.js | 350 ++++++++++-------- .../react-devtools-shell/src/app/console.js | 13 +- .../src/ReactFiberBeginWork.new.js | 7 +- .../src/ReactFiberBeginWork.old.js | 7 +- .../src/ReactFiberClassComponent.new.js | 2 +- .../src/ReactFiberClassComponent.old.js | 2 +- .../src/ReactFiberDevToolsHook.new.js | 43 ++- .../src/ReactFiberDevToolsHook.old.js | 43 ++- .../src/ReactFiberReconciler.js | 11 - .../src/ReactFiberReconciler.new.js | 36 +- .../src/ReactFiberReconciler.old.js | 36 +- .../src/ReactUpdateQueue.new.js | 2 +- .../src/ReactUpdateQueue.old.js | 2 +- 18 files changed, 676 insertions(+), 454 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/console-test.js b/packages/react-devtools-shared/src/__tests__/console-test.js index 4c4bbb17a233b..fef2d9674b6fd 100644 --- a/packages/react-devtools-shared/src/__tests__/console-test.js +++ b/packages/react-devtools-shared/src/__tests__/console-test.js @@ -17,11 +17,10 @@ let mockLog; let mockWarn; let patchConsole; let unpatchConsole; +let rendererID; describe('console', () => { beforeEach(() => { - jest.resetModules(); - const Console = require('react-devtools-shared/src/backend/console'); patchConsole = Console.patch; unpatchConsole = Console.unpatch; @@ -41,21 +40,16 @@ describe('console', () => { }; Console.dangerous_setTargetConsoleForTesting(fakeConsole); - - // Note the Console module only patches once, - // so it's important to patch the test console before injection. - patchConsole({ - appendComponentStack: true, - breakOnWarn: false, - showInlineWarningsAndErrors: false, - hideDoubleLogsInStrictLegacy: false, - }); + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.dangerous_setTargetConsoleForTesting( + fakeConsole, + ); const inject = global.__REACT_DEVTOOLS_GLOBAL_HOOK__.inject; global.__REACT_DEVTOOLS_GLOBAL_HOOK__.inject = internals => { - inject(internals); + rendererID = inject(internals); Console.registerRenderer(internals); + return rendererID; }; React = require('react'); @@ -78,7 +72,7 @@ describe('console', () => { it('should not patch console methods that are not explicitly overriden', () => { expect(fakeConsole.error).not.toBe(mockError); expect(fakeConsole.info).toBe(mockInfo); - expect(fakeConsole.log).not.toBe(mockLog); + expect(fakeConsole.log).toBe(mockLog); expect(fakeConsole.warn).not.toBe(mockWarn); }); @@ -90,7 +84,7 @@ describe('console', () => { patchConsole({ appendComponentStack: true, - breakOnWarn: false, + breakOnConsoleErrors: false, showInlineWarningsAndErrors: false, }); @@ -98,7 +92,7 @@ describe('console', () => { expect(fakeConsole.warn).not.toBe(mockWarn); }); - it('should patch the console when breakOnWarn is enabled', () => { + it('should patch the console when breakOnConsoleErrors is enabled', () => { unpatchConsole(); expect(fakeConsole.error).toBe(mockError); @@ -106,7 +100,7 @@ describe('console', () => { patchConsole({ appendComponentStack: false, - breakOnWarn: true, + breakOnConsoleErrors: true, showInlineWarningsAndErrors: false, }); @@ -122,7 +116,7 @@ describe('console', () => { patchConsole({ appendComponentStack: false, - breakOnWarn: false, + breakOnConsoleErrors: false, showInlineWarningsAndErrors: true, }); @@ -135,7 +129,7 @@ describe('console', () => { patchConsole({ appendComponentStack: true, - breakOnWarn: false, + breakOnConsoleErrors: false, showInlineWarningsAndErrors: false, }); @@ -172,6 +166,8 @@ describe('console', () => { }); it('should not append multiple stacks', () => { + global.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ = true; + const Child = ({children}) => { fakeConsole.warn('warn\n in Child (at fake.js:123)'); fakeConsole.error('error', '\n in Child (at fake.js:123)'); @@ -192,6 +188,8 @@ describe('console', () => { }); it('should append component stacks to errors and warnings logged during render', () => { + global.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ = true; + const Intermediate = ({children}) => children; const Parent = ({children}) => ( @@ -277,6 +275,8 @@ describe('console', () => { }); it('should append component stacks to errors and warnings logged from commit hooks', () => { + global.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ = true; + const Intermediate = ({children}) => children; const Parent = ({children}) => ( @@ -372,13 +372,14 @@ describe('console', () => { }); it('should append stacks after being uninstalled and reinstalled', () => { + global.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ = false; + const Child = ({children}) => { fakeConsole.warn('warn'); fakeConsole.error('error'); return null; }; - unpatchConsole(); act(() => legacyRender(, document.createElement('div'))); expect(mockWarn).toHaveBeenCalledTimes(1); @@ -390,7 +391,7 @@ describe('console', () => { patchConsole({ appendComponentStack: true, - breakOnWarn: false, + breakOnConsoleErrors: false, showInlineWarningsAndErrors: false, }); act(() => legacyRender(, document.createElement('div'))); @@ -410,6 +411,8 @@ describe('console', () => { }); it('should be resilient to prepareStackTrace', () => { + global.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ = true; + Error.prepareStackTrace = function(error, callsites) { const stack = ['An error occurred:', error.message]; for (let i = 0; i < callsites.length; i++) { @@ -469,6 +472,9 @@ describe('console', () => { }); it('should double log if hideConsoleLogsInStrictMode is disabled in Strict mode', () => { + global.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ = false; + global.__REACT_DEVTOOLS_HIDE_CONSOLE_LOGS_IN_STRICT_MODE__ = false; + const container = document.createElement('div'); const root = ReactDOM.createRoot(container); @@ -479,13 +485,6 @@ describe('console', () => { return
; } - patchConsole({ - appendComponentStack: false, - breakOnWarn: false, - showInlineWarningsAndErrors: false, - hideConsoleLogsInStrictMode: false, - }); - act(() => root.render( @@ -493,8 +492,6 @@ describe('console', () => { , ), ); - - expect(mockLog).toHaveBeenCalledTimes(2); expect(mockLog.mock.calls[0]).toHaveLength(1); expect(mockLog.mock.calls[0][0]).toBe('log'); expect(mockLog.mock.calls[1]).toHaveLength(2); @@ -514,6 +511,9 @@ describe('console', () => { }); it('should not double log if hideConsoleLogsInStrictMode is enabled in Strict mode', () => { + global.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ = false; + global.__REACT_DEVTOOLS_HIDE_CONSOLE_LOGS_IN_STRICT_MODE__ = true; + const container = document.createElement('div'); const root = ReactDOM.createRoot(container); @@ -524,12 +524,47 @@ describe('console', () => { return
; } - patchConsole({ - appendComponentStack: false, - breakOnWarn: false, - showInlineWarningsAndErrors: false, - hideConsoleLogsInStrictMode: true, - }); + act(() => + root.render( + + + , + ), + ); + + expect(mockLog).toHaveBeenCalledTimes(1); + expect(mockLog.mock.calls[0]).toHaveLength(1); + expect(mockLog.mock.calls[0][0]).toBe('log'); + + expect(mockWarn).toHaveBeenCalledTimes(1); + expect(mockWarn.mock.calls[0]).toHaveLength(1); + expect(mockWarn.mock.calls[0][0]).toBe('warn'); + + expect(mockError).toHaveBeenCalledTimes(1); + expect(mockError.mock.calls[0]).toHaveLength(1); + expect(mockError.mock.calls[0][0]).toBe('error'); + }); + + it('should double log in Strict mode initial render for extension', () => { + global.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ = false; + global.__REACT_DEVTOOLS_HIDE_CONSOLE_LOGS_IN_STRICT_MODE__ = false; + + // This simulates a render that happens before React DevTools have finished + // their handshake to attach the React DOM renderer functions to DevTools + // In this case, we should still be able to mock the console in Strict mode + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.rendererInterfaces.set( + rendererID, + null, + ); + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container); + + function App() { + fakeConsole.log('log'); + fakeConsole.warn('warn'); + fakeConsole.error('error'); + return
; + } act(() => root.render( @@ -539,6 +574,53 @@ describe('console', () => { ), ); + expect(mockLog).toHaveBeenCalledTimes(2); + expect(mockLog.mock.calls[0]).toHaveLength(1); + expect(mockLog.mock.calls[0][0]).toBe('log'); + expect(mockLog.mock.calls[1]).toHaveLength(2); + expect(mockLog.mock.calls[1][0]).toBe('%clog'); + + expect(mockWarn).toHaveBeenCalledTimes(2); + expect(mockWarn.mock.calls[0]).toHaveLength(1); + expect(mockWarn.mock.calls[0][0]).toBe('warn'); + expect(mockWarn.mock.calls[1]).toHaveLength(2); + expect(mockWarn.mock.calls[1][0]).toBe('%cwarn'); + + expect(mockError).toHaveBeenCalledTimes(2); + expect(mockError.mock.calls[0]).toHaveLength(1); + expect(mockError.mock.calls[0][0]).toBe('error'); + expect(mockError.mock.calls[1]).toHaveLength(2); + expect(mockError.mock.calls[1][0]).toBe('%cerror'); + }); + + it('should not double log in Strict mode initial render for extension', () => { + global.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ = false; + global.__REACT_DEVTOOLS_HIDE_CONSOLE_LOGS_IN_STRICT_MODE__ = true; + + // This simulates a render that happens before React DevTools have finished + // their handshake to attach the React DOM renderer functions to DevTools + // In this case, we should still be able to mock the console in Strict mode + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.rendererInterfaces.set( + rendererID, + null, + ); + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container); + + function App() { + fakeConsole.log('log'); + fakeConsole.warn('warn'); + fakeConsole.error('error'); + return
; + } + + act(() => + root.render( + + + , + ), + ); expect(mockLog).toHaveBeenCalledTimes(1); expect(mockLog.mock.calls[0]).toHaveLength(1); expect(mockLog.mock.calls[0][0]).toBe('log'); @@ -551,6 +633,56 @@ describe('console', () => { expect(mockError.mock.calls[0]).toHaveLength(1); expect(mockError.mock.calls[0][0]).toBe('error'); }); + + it('should properly dim component stacks during strict mode double log', () => { + global.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ = true; + global.__REACT_DEVTOOLS_HIDE_CONSOLE_LOGS_IN_STRICT_MODE__ = false; + + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container); + + const Intermediate = ({children}) => children; + const Parent = ({children}) => ( + + + + ); + const Child = ({children}) => { + fakeConsole.error('error'); + fakeConsole.warn('warn'); + return null; + }; + + act(() => + root.render( + + + , + ), + ); + + expect(mockWarn).toHaveBeenCalledTimes(2); + expect(mockWarn.mock.calls[0]).toHaveLength(2); + expect(normalizeCodeLocInfo(mockWarn.mock.calls[0][1])).toEqual( + '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)', + ); + expect(mockWarn.mock.calls[1]).toHaveLength(2); + expect(normalizeCodeLocInfo(mockWarn.mock.calls[1][0])).toEqual( + '%cwarn \n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)', + ); + expect(mockWarn.mock.calls[1][1]).toMatch('color: rgba('); + + expect(mockError).toHaveBeenCalledTimes(2); + expect(mockError.mock.calls[0]).toHaveLength(2); + expect(normalizeCodeLocInfo(mockError.mock.calls[0][1])).toEqual( + '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)', + ); + expect(mockError.mock.calls[1]).toHaveLength(2); + expect(normalizeCodeLocInfo(mockError.mock.calls[1][0])).toEqual( + '%cerror \n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)', + ); + expect(mockError.mock.calls[1][1]).toMatch('color: rgba('); + }); }); describe('console error', () => { @@ -577,23 +709,13 @@ describe('console error', () => { Console.dangerous_setTargetConsoleForTesting(fakeConsole); - // Note the Console module only patches once, - // so it's important to patch the test console before injection. - patchConsole({ - appendComponentStack: true, - breakOnWarn: false, - showInlineWarningsAndErrors: false, - hideDoubleLogsInStrictLegacy: false, - }); - const inject = global.__REACT_DEVTOOLS_GLOBAL_HOOK__.inject; global.__REACT_DEVTOOLS_GLOBAL_HOOK__.inject = internals => { - internals.getIsStrictMode = () => { - throw Error('foo'); - }; inject(internals); - Console.registerRenderer(internals); + Console.registerRenderer(internals, () => { + throw Error('foo'); + }); }; React = require('react'); @@ -617,8 +739,8 @@ describe('console error', () => { patchConsole({ appendComponentStack: true, - breakOnWarn: false, - showInlineWarningsAndErrors: false, + breakOnConsoleErrors: false, + showInlineWarningsAndErrors: true, hideConsoleLogsInStrictMode: false, }); diff --git a/packages/react-devtools-shared/src/backend/console.js b/packages/react-devtools-shared/src/backend/console.js index 26168f8d3d463..95d1f4d103329 100644 --- a/packages/react-devtools-shared/src/backend/console.js +++ b/packages/react-devtools-shared/src/backend/console.js @@ -16,7 +16,7 @@ import {getInternalReactConstants} from './renderer'; import {getStackByFiberInDevAndProd} from './DevToolsFiberComponentStack'; import {consoleManagedByDevToolsDuringStrictMode} from 'react-devtools-feature-flags'; -const OVERRIDE_CONSOLE_METHODS = ['error', 'trace', 'warn', 'log']; +const OVERRIDE_CONSOLE_METHODS = ['error', 'trace', 'warn']; const DIMMED_NODE_CONSOLE_COLOR = '\x1b[2m%s\x1b[0m'; // React's custom built component stack strings match "\s{4}in" @@ -30,6 +30,37 @@ export function isStringComponentStack(text: string): boolean { return PREFIX_REGEX.test(text) || ROW_COLUMN_NUMBER_REGEX.test(text); } +const STYLE_DIRECTIVE_REGEX = /^%c/; + +// This function tells whether or not the arguments for a console +// method has been overridden by the patchForStrictMode function. +// If it has we'll need to do some special formatting of the arguments +// so the console color stays consistent +function isStrictModeOverride(args: Array, method: string): boolean { + return ( + args.length === 2 && + STYLE_DIRECTIVE_REGEX.test(args[0]) && + args[1] === `color: ${getConsoleColor(method) || ''}` + ); +} + +function getConsoleColor(method: string): ?string { + switch (method) { + case 'warn': + return consoleSettingsRef.browserTheme === 'light' + ? process.env.LIGHT_MODE_DIMMED_WARNING_COLOR + : process.env.DARK_MODE_DIMMED_WARNING_COLOR; + case 'error': + return consoleSettingsRef.browserTheme === 'light' + ? process.env.LIGHT_MODE_DIMMED_ERROR_COLOR + : process.env.DARK_MODE_DIMMED_ERROR_COLOR; + case 'log': + default: + return consoleSettingsRef.browserTheme === 'light' + ? process.env.LIGHT_MODE_DIMMED_LOG_COLOR + : process.env.DARK_MODE_DIMMED_LOG_COLOR; + } +} type OnErrorOrWarning = ( fiber: Fiber, type: 'error' | 'warn', @@ -43,7 +74,6 @@ const injectedRenderers: Map< getCurrentFiber: () => Fiber | null, onErrorOrWarning: ?OnErrorOrWarning, workTagMap: WorkTagMap, - getIsStrictMode: ?() => boolean, |}, > = new Map(); @@ -82,7 +112,6 @@ export function registerRenderer( const { currentDispatcherRef, getCurrentFiber, - getIsStrictMode, findFiberByHostInstance, version, } = renderer; @@ -100,7 +129,6 @@ export function registerRenderer( injectedRenderers.set(renderer, { currentDispatcherRef, getCurrentFiber, - getIsStrictMode, workTagMap: ReactTypeOfWork, onErrorOrWarning, }); @@ -112,11 +140,11 @@ const consoleSettingsRef = { breakOnConsoleErrors: false, showInlineWarningsAndErrors: false, hideConsoleLogsInStrictMode: false, + browserTheme: 'dark', }; // Patches console methods to append component stack for the current fiber. // Call unpatch() to remove the injected behavior. -// NOTE: KEEP IN SYNC with src/hook.js:patchConsoleForInitialRenderInExtension export function patch({ appendComponentStack, breakOnConsoleErrors, @@ -136,141 +164,176 @@ export function patch({ consoleSettingsRef.breakOnConsoleErrors = breakOnConsoleErrors; consoleSettingsRef.showInlineWarningsAndErrors = showInlineWarningsAndErrors; consoleSettingsRef.hideConsoleLogsInStrictMode = hideConsoleLogsInStrictMode; + consoleSettingsRef.browserTheme = browserTheme; + + if ( + appendComponentStack || + breakOnConsoleErrors || + showInlineWarningsAndErrors + ) { + if (unpatchFn !== null) { + // Don't patch twice. + return; + } - if (unpatchFn !== null) { - // Don't patch twice. - return; - } + const originalConsoleMethods = {}; - const originalConsoleMethods = {}; + unpatchFn = () => { + for (const method in originalConsoleMethods) { + try { + // $FlowFixMe property error|warn is not writable. + targetConsole[method] = originalConsoleMethods[method]; + } catch (error) {} + } + }; - unpatchFn = () => { - for (const method in originalConsoleMethods) { + OVERRIDE_CONSOLE_METHODS.forEach(method => { try { - // $FlowFixMe property error|warn is not writable. - targetConsole[method] = originalConsoleMethods[method]; - } catch (error) {} - } - }; - - OVERRIDE_CONSOLE_METHODS.forEach(method => { - try { - const originalMethod = (originalConsoleMethods[method] = targetConsole[ - method - ].__REACT_DEVTOOLS_ORIGINAL_METHOD__ - ? targetConsole[method].__REACT_DEVTOOLS_ORIGINAL_METHOD__ - : targetConsole[method]); - - const overrideMethod = (...args) => { - let shouldAppendWarningStack = false; - if (method !== 'log') { - if (consoleSettingsRef.appendComponentStack) { - const lastArg = args.length > 0 ? args[args.length - 1] : null; - const alreadyHasComponentStack = - typeof lastArg === 'string' && isStringComponentStack(lastArg); - - // If we are ever called with a string that already has a component stack, - // e.g. a React error/warning, don't append a second stack. - shouldAppendWarningStack = !alreadyHasComponentStack; + const originalMethod = (originalConsoleMethods[method] = targetConsole[ + method + ].__REACT_DEVTOOLS_ORIGINAL_METHOD__ + ? targetConsole[method].__REACT_DEVTOOLS_ORIGINAL_METHOD__ + : targetConsole[method]); + + const overrideMethod = (...args) => { + let shouldAppendWarningStack = false; + if (method !== 'log') { + if (consoleSettingsRef.appendComponentStack) { + const lastArg = args.length > 0 ? args[args.length - 1] : null; + const alreadyHasComponentStack = + typeof lastArg === 'string' && isStringComponentStack(lastArg); + + // If we are ever called with a string that already has a component stack, + // e.g. a React error/warning, don't append a second stack. + shouldAppendWarningStack = !alreadyHasComponentStack; + } } - } - - const shouldShowInlineWarningsAndErrors = - consoleSettingsRef.showInlineWarningsAndErrors && - (method === 'error' || method === 'warn'); - - let isInStrictMode = false; - - // Search for the first renderer that has a current Fiber. - // We don't handle the edge case of stacks for more than one (e.g. interleaved renderers?) - // eslint-disable-next-line no-for-of-loops/no-for-of-loops - for (const { - currentDispatcherRef, - getCurrentFiber, - onErrorOrWarning, - workTagMap, - getIsStrictMode, - } of injectedRenderers.values()) { - const current: ?Fiber = getCurrentFiber(); - if (current != null) { - try { - if (typeof getIsStrictMode === 'function' && getIsStrictMode()) { - isInStrictMode = true; - } - if (shouldShowInlineWarningsAndErrors) { - // patch() is called by two places: (1) the hook and (2) the renderer backend. - // The backend is what implements a message queue, so it's the only one that injects onErrorOrWarning. - if (typeof onErrorOrWarning === 'function') { - onErrorOrWarning( - current, - ((method: any): 'error' | 'warn'), - // Copy args before we mutate them (e.g. adding the component stack) - args.slice(), - ); + const shouldShowInlineWarningsAndErrors = + consoleSettingsRef.showInlineWarningsAndErrors && + (method === 'error' || method === 'warn'); + + // Search for the first renderer that has a current Fiber. + // We don't handle the edge case of stacks for more than one (e.g. interleaved renderers?) + // eslint-disable-next-line no-for-of-loops/no-for-of-loops + for (const { + currentDispatcherRef, + getCurrentFiber, + onErrorOrWarning, + workTagMap, + } of injectedRenderers.values()) { + const current: ?Fiber = getCurrentFiber(); + if (current != null) { + try { + if (shouldShowInlineWarningsAndErrors) { + // patch() is called by two places: (1) the hook and (2) the renderer backend. + // The backend is what implements a message queue, so it's the only one that injects onErrorOrWarning. + if (typeof onErrorOrWarning === 'function') { + onErrorOrWarning( + current, + ((method: any): 'error' | 'warn'), + // Copy args before we mutate them (e.g. adding the component stack) + args.slice(), + ); + } } - } - if (shouldAppendWarningStack) { - const componentStack = getStackByFiberInDevAndProd( - workTagMap, - current, - currentDispatcherRef, - ); - if (componentStack !== '') { - args.push(componentStack); + if (shouldAppendWarningStack) { + const componentStack = getStackByFiberInDevAndProd( + workTagMap, + current, + currentDispatcherRef, + ); + if (componentStack !== '') { + if (isStrictModeOverride(args, method)) { + args[0] = format(args[0], componentStack); + } else { + args.push(componentStack); + } + } } + } catch (error) { + // Don't let a DevTools or React internal error interfere with logging. + setTimeout(() => { + throw error; + }, 0); + } finally { + break; } - } catch (error) { - // Don't let a DevTools or React internal error interfere with logging. - setTimeout(() => { - throw error; - }, 0); - } finally { - break; } } - } - - if (consoleSettingsRef.breakOnConsoleErrors) { - // --- Welcome to debugging with React DevTools --- - // This debugger statement means that you've enabled the "break on warnings" feature. - // Use the browser's Call Stack panel to step out of this override function- - // to where the original warning or error was logged. - // eslint-disable-next-line no-debugger - debugger; - } - - if (consoleManagedByDevToolsDuringStrictMode && isInStrictMode) { + + if (consoleSettingsRef.breakOnConsoleErrors) { + // --- Welcome to debugging with React DevTools --- + // This debugger statement means that you've enabled the "break on warnings" feature. + // Use the browser's Call Stack panel to step out of this override function- + // to where the original warning or error was logged. + // eslint-disable-next-line no-debugger + debugger; + } + + originalMethod(...args); + }; + + overrideMethod.__REACT_DEVTOOLS_ORIGINAL_METHOD__ = originalMethod; + originalMethod.__REACT_DEVTOOLS_OVERRIDE_METHOD__ = overrideMethod; + + // $FlowFixMe property error|warn is not writable. + targetConsole[method] = overrideMethod; + } catch (error) {} + }); + } else { + unpatch(); + } +} + +// Removed component stack patch from console methods. +export function unpatch(): void { + if (unpatchFn !== null) { + unpatchFn(); + unpatchFn = null; + } +} + +let unpatchForStrictModeFn: null | (() => void) = null; + +// NOTE: KEEP IN SYNC with src/hook.js:patchConsoleForInitialRenderInStrictMode +export function patchForStrictMode() { + if (consoleManagedByDevToolsDuringStrictMode) { + const overrideConsoleMethods = ['error', 'trace', 'warn', 'log']; + + if (unpatchForStrictModeFn !== null) { + // Don't patch twice. + return; + } + + const originalConsoleMethods = {}; + + unpatchForStrictModeFn = () => { + for (const method in originalConsoleMethods) { + try { + // $FlowFixMe property error|warn is not writable. + targetConsole[method] = originalConsoleMethods[method]; + } catch (error) {} + } + }; + + overrideConsoleMethods.forEach(method => { + try { + const originalMethod = (originalConsoleMethods[method] = targetConsole[ + method + ].__REACT_DEVTOOLS_STRICT_MODE_ORIGINAL_METHOD__ + ? targetConsole[method].__REACT_DEVTOOLS_STRICT_MODE_ORIGINAL_METHOD__ + : targetConsole[method]); + + const overrideMethod = (...args) => { if (!consoleSettingsRef.hideConsoleLogsInStrictMode) { // Dim the text color of the double logs if we're not // hiding them. if (isNode) { originalMethod(DIMMED_NODE_CONSOLE_COLOR, format(...args)); } else { - let color; - switch (method) { - case 'warn': - color = - browserTheme === 'light' - ? process.env.LIGHT_MODE_DIMMED_WARNING_COLOR - : process.env.DARK_MODE_DIMMED_WARNING_COLOR; - break; - case 'error': - color = - browserTheme === 'light' - ? process.env.LIGHT_MODE_DIMMED_ERROR_COLOR - : process.env.DARK_MODE_DIMMED_ERROR_COLOR; - break; - case 'log': - default: - color = - browserTheme === 'light' - ? process.env.LIGHT_MODE_DIMMED_LOG_COLOR - : process.env.DARK_MODE_DIMMED_LOG_COLOR; - break; - } - + const color = getConsoleColor(method); if (color) { originalMethod(`%c${format(...args)}`, `color: ${color}`); } else { @@ -278,24 +341,24 @@ export function patch({ } } } - } else { - originalMethod(...args); - } - }; + }; - overrideMethod.__REACT_DEVTOOLS_ORIGINAL_METHOD__ = originalMethod; - originalMethod.__REACT_DEVTOOLS_OVERRIDE_METHOD__ = overrideMethod; + overrideMethod.__REACT_DEVTOOLS_STRICT_MODE_ORIGINAL_METHOD__ = originalMethod; + originalMethod.__REACT_DEVTOOLS_STRICT_MODE_OVERRIDE_METHOD__ = overrideMethod; - // $FlowFixMe property error|warn is not writable. - targetConsole[method] = overrideMethod; - } catch (error) {} - }); + // $FlowFixMe property error|warn is not writable. + targetConsole[method] = overrideMethod; + } catch (error) {} + }); + } } -// Removed component stack patch from console methods. -export function unpatch(): void { - if (unpatchFn !== null) { - unpatchFn(); - unpatchFn = null; +// NOTE: KEEP IN SYNC with src/hook.js:unpatchConsoleForInitialRenderInStrictMode +export function unpatchForStrictMode(): void { + if (consoleManagedByDevToolsDuringStrictMode) { + if (unpatchForStrictModeFn !== null) { + unpatchForStrictModeFn(); + unpatchForStrictModeFn = null; + } } } diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index 7b612ff346171..24f30b87bc78a 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js @@ -1073,6 +1073,10 @@ export function attach( // Not implemented } + function patchConsoleForStrictMode() {} + + function unpatchConsoleForStrictMode() {} + return { clearErrorsAndWarnings, clearErrorsForFiberID, @@ -1101,6 +1105,7 @@ export function attach( overrideSuspense, overrideValueAtPath, renamePath, + patchConsoleForStrictMode, prepareViewAttributeSource, prepareViewElementSource, renderer, @@ -1109,6 +1114,7 @@ export function attach( startProfiling, stopProfiling, storeAsGlobal, + unpatchConsoleForStrictMode, updateComponentFilters, }; } diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 1cfcaaa6a866b..19279cbaf565e 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -59,6 +59,8 @@ import {inspectHooksOfFiber} from 'react-debug-tools'; import { patch as patchConsole, registerRenderer as registerRendererWithConsole, + patchForStrictMode as patchConsoleForStrictMode, + unpatchForStrictMode as unpatchConsoleForStrictMode, } from './console'; import { CONCURRENT_MODE_NUMBER, @@ -4249,6 +4251,7 @@ export function attach( handlePostCommitFiberRoot, inspectElement, logElementToConsole, + patchConsoleForStrictMode, prepareViewAttributeSource, prepareViewElementSource, overrideError, @@ -4261,6 +4264,7 @@ export function attach( startProfiling, stopProfiling, storeAsGlobal, + unpatchConsoleForStrictMode, updateComponentFilters, }; } diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 0a7166d040983..2206dd94fb124 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -136,8 +136,6 @@ export type ReactRenderer = { // Only injected by React v16.9+ in DEV mode. // Enables DevTools to append owners-only component stack to error messages. getCurrentFiber?: () => Fiber | null, - - getIsStrictMode?: () => boolean, // 17.0.2+ reconcilerVersion?: string, // Uniquely identifies React DOM v15. @@ -352,6 +350,7 @@ export type RendererInterface = { path: Array, value: any, ) => void, + patchConsoleForStrictMode: () => void, prepareViewAttributeSource: ( id: number, path: Array, @@ -374,6 +373,7 @@ export type RendererInterface = { path: Array, count: number, ) => void, + unpatchConsoleForStrictMode: () => void, updateComponentFilters: (componentFilters: Array) => void, ... }; @@ -408,5 +408,8 @@ export type DevToolsHook = { // Added in v16.9 to support Fast Refresh didError?: boolean, ) => void, + + // Testing + dangerous_setTargetConsoleForTesting?: (fakeConsole: Object) => void, ... }; diff --git a/packages/react-devtools-shared/src/hook.js b/packages/react-devtools-shared/src/hook.js index 28cfd027afe43..152b1dce8f8e4 100644 --- a/packages/react-devtools-shared/src/hook.js +++ b/packages/react-devtools-shared/src/hook.js @@ -8,15 +8,12 @@ * @flow */ -import type {Fiber} from 'react-reconciler/src/ReactInternalTypes'; -import type {ReactRenderer} from './backend/types'; import type {BrowserTheme} from 'react-devtools-shared/src/devtools/views/DevTools'; import { patch as patchConsole, registerRenderer as registerRendererWithConsole, } from './backend/console'; -import {consoleManagedByDevToolsDuringStrictMode} from 'react-devtools-feature-flags'; import type {DevToolsHook} from 'react-devtools-shared/src/backend/types'; @@ -27,6 +24,23 @@ export function installHook(target: any): DevToolsHook | null { return null; } + let targetConsole: Object = console; + let targetConsoleMethods = {}; + for (const method in console) { + targetConsoleMethods[method] = console[method]; + } + + function dangerous_setTargetConsoleForTesting( + targetConsoleForTesting: Object, + ): void { + targetConsole = targetConsoleForTesting; + + targetConsoleMethods = {}; + for (const method in targetConsole) { + targetConsoleMethods[method] = console[method]; + } + } + function detectReactBuildType(renderer) { try { if (typeof renderer.version === 'string') { @@ -163,156 +177,148 @@ export function installHook(target: any): DevToolsHook | null { maybeMessage: any, ...inputArgs: $ReadOnlyArray ): string { - if (consoleManagedByDevToolsDuringStrictMode) { - const args = inputArgs.slice(); - - // Symbols cannot be concatenated with Strings. - let formatted: string = - typeof maybeMessage === 'symbol' - ? maybeMessage.toString() - : '' + maybeMessage; - - // If the first argument is a string, check for substitutions. - if (typeof maybeMessage === 'string') { - if (args.length) { - const REGEXP = /(%?)(%([jds]))/g; - - formatted = formatted.replace(REGEXP, (match, escaped, ptn, flag) => { - let arg = args.shift(); - switch (flag) { - case 's': - arg += ''; - break; - case 'd': - case 'i': - arg = parseInt(arg, 10).toString(); - break; - case 'f': - arg = parseFloat(arg).toString(); - break; - } - if (!escaped) { - return arg; - } - args.unshift(arg); - return match; - }); - } - } + const args = inputArgs.slice(); - // Arguments that remain after formatting. - if (args.length) { - for (let i = 0; i < args.length; i++) { - const arg = args[i]; + // Symbols cannot be concatenated with Strings. + let formatted: string = + typeof maybeMessage === 'symbol' + ? maybeMessage.toString() + : '' + maybeMessage; - // Symbols cannot be concatenated with Strings. - formatted += ' ' + (typeof arg === 'symbol' ? arg.toString() : arg); - } + // If the first argument is a string, check for substitutions. + if (typeof maybeMessage === 'string') { + if (args.length) { + const REGEXP = /(%?)(%([jds]))/g; + + formatted = formatted.replace(REGEXP, (match, escaped, ptn, flag) => { + let arg = args.shift(); + switch (flag) { + case 's': + arg += ''; + break; + case 'd': + case 'i': + arg = parseInt(arg, 10).toString(); + break; + case 'f': + arg = parseFloat(arg).toString(); + break; + } + if (!escaped) { + return arg; + } + args.unshift(arg); + return match; + }); } + } - // Update escaped %% values. - formatted = formatted.replace(/%{2,2}/g, '%'); + // Arguments that remain after formatting. + if (args.length) { + for (let i = 0; i < args.length; i++) { + const arg = args[i]; - return '' + formatted; + // Symbols cannot be concatenated with Strings. + formatted += ' ' + (typeof arg === 'symbol' ? arg.toString() : arg); + } } - return ''; + // Update escaped %% values. + formatted = formatted.replace(/%{2,2}/g, '%'); + + return '' + formatted; } - // NOTE: KEEP IN SYNC with src/backend/console.js:patch - function patchConsoleForInitialRenderInExtension( - renderer: ReactRenderer, - { - hideConsoleLogsInStrictMode, - browserTheme, - }: {hideConsoleLogsInStrictMode: boolean, browserTheme: BrowserTheme}, - ): void { - if (consoleManagedByDevToolsDuringStrictMode) { - const overrideConsoleMethods = ['error', 'trace', 'warn', 'log']; - - if (__EXTENSION__) { - const targetConsole = console; - - const originalConsoleMethods = {}; - - overrideConsoleMethods.forEach(method => { - try { - const originalMethod = (originalConsoleMethods[ - method - ] = targetConsole[method].__REACT_DEVTOOLS_ORIGINAL_METHOD__ - ? targetConsole[method].__REACT_DEVTOOLS_ORIGINAL_METHOD__ - : targetConsole[method]); - - const overrideMethod = (...args) => { - let isInStrictMode = false; - - // Search for the first renderer that has a current Fiber. - // We don't handle the edge case of stacks for more than one (e.g. interleaved renderers?) - const {getCurrentFiber, getIsStrictMode} = renderer; - if (typeof getCurrentFiber !== 'function') { - return; - } - - const current: ?Fiber = getCurrentFiber(); - if (current != null) { - try { - if ( - typeof getIsStrictMode === 'function' && - getIsStrictMode() - ) { - isInStrictMode = true; - } - } catch (error) { - // Don't let a DevTools or React internal error interfere with logging. - } - } - - if (isInStrictMode) { - if (!hideConsoleLogsInStrictMode) { - // Dim the text color of the double logs if we're not - // hiding them. - let color; - switch (method) { - case 'warn': - color = - browserTheme === 'light' - ? process.env.LIGHT_MODE_DIMMED_WARNING_COLOR - : process.env.DARK_MODE_DIMMED_WARNING_COLOR; - break; - case 'error': - color = - browserTheme === 'light' - ? process.env.LIGHT_MODE_DIMMED_ERROR_COLOR - : process.env.DARK_MODE_DIMMED_ERROR_COLOR; - break; - case 'log': - default: - color = - browserTheme === 'light' - ? process.env.LIGHT_MODE_DIMMED_LOG_COLOR - : process.env.DARK_MODE_DIMMED_LOG_COLOR; - break; - } - - if (color) { - originalMethod(`%c${format(...args)}`, `color: ${color}`); - } else { - throw Error('Console color is not defined'); - } - } - } else { - originalMethod(...args); - } - }; - - overrideMethod.__REACT_DEVTOOLS_ORIGINAL_METHOD__ = originalMethod; - originalMethod.__REACT_DEVTOOLS_OVERRIDE_METHOD__ = overrideMethod; - - // $FlowFixMe property error|warn is not writable. - targetConsole[method] = overrideMethod; - } catch (error) {} - }); + let unpatchFn = null; + + // NOTE: KEEP IN SYNC with src/backend/console.js:patchForStrictMode + // This function hides or dims console logs during the initial double renderer + // in Strict Mode. We need this function because during initial render, + // React and DevTools are connecting and the renderer interface isn't avaiable + // and we want to be able to have consistent logging behavior for double logs + // during the initial renderer. + function patchConsoleForInitialRenderInStrictMode({ + hideConsoleLogsInStrictMode, + browserTheme, + }: { + hideConsoleLogsInStrictMode: boolean, + browserTheme: BrowserTheme, + }) { + const overrideConsoleMethods = ['error', 'trace', 'warn', 'log']; + + if (unpatchFn !== null) { + // Don't patch twice. + return; + } + + const originalConsoleMethods = {}; + + unpatchFn = () => { + for (const method in originalConsoleMethods) { + try { + // $FlowFixMe property error|warn is not writable. + targetConsole[method] = originalConsoleMethods[method]; + } catch (error) {} } + }; + + overrideConsoleMethods.forEach(method => { + try { + const originalMethod = (originalConsoleMethods[method] = targetConsole[ + method + ].__REACT_DEVTOOLS_STRICT_MODE_ORIGINAL_METHOD__ + ? targetConsole[method].__REACT_DEVTOOLS_STRICT_MODE_ORIGINAL_METHOD__ + : targetConsole[method]); + + const overrideMethod = (...args) => { + if (!hideConsoleLogsInStrictMode) { + // Dim the text color of the double logs if we're not + // hiding them. + let color; + switch (method) { + case 'warn': + color = + browserTheme === 'light' + ? process.env.LIGHT_MODE_DIMMED_WARNING_COLOR + : process.env.DARK_MODE_DIMMED_WARNING_COLOR; + break; + case 'error': + color = + browserTheme === 'light' + ? process.env.LIGHT_MODE_DIMMED_ERROR_COLOR + : process.env.DARK_MODE_DIMMED_ERROR_COLOR; + break; + case 'log': + default: + color = + browserTheme === 'light' + ? process.env.LIGHT_MODE_DIMMED_LOG_COLOR + : process.env.DARK_MODE_DIMMED_LOG_COLOR; + break; + } + + if (color) { + originalMethod(`%c${format(...args)}`, `color: ${color}`); + } else { + throw Error('Console color is not defined'); + } + } + }; + + overrideMethod.__REACT_DEVTOOLS_STRICT_MODE_ORIGINAL_METHOD__ = originalMethod; + originalMethod.__REACT_DEVTOOLS_STRICT_MODE_OVERRIDE_METHOD__ = overrideMethod; + + // $FlowFixMe property error|warn is not writable. + targetConsole[method] = overrideMethod; + } catch (error) {} + }); + } + + // NOTE: KEEP IN SYNC with src/backend/console.js:unpatchForStrictMode + function unpatchConsoleForInitialRenderInStrictMode() { + if (unpatchFn !== null) { + unpatchFn(); + unpatchFn = null; } } @@ -343,7 +349,7 @@ export function installHook(target: any): DevToolsHook | null { // Note that because this function is inlined, this conditional check must only use static booleans. // Otherwise the extension will throw with an undefined error. // (See comments in the try/catch below for more context on inlining.) - if (!__TEST__) { + if (!__TEST__ && !__EXTENSION__) { try { const appendComponentStack = window.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ !== false; @@ -362,23 +368,14 @@ export function installHook(target: any): DevToolsHook | null { // but Webpack wraps imports with an object (e.g. _backend_console__WEBPACK_IMPORTED_MODULE_0__) // and the object itself will be undefined as well for the reasons mentioned above, // so we use try/catch instead. - if (!__EXTENSION__) { - registerRendererWithConsole(renderer); - patchConsole({ - appendComponentStack, - breakOnConsoleErrors, - showInlineWarningsAndErrors, - hideConsoleLogsInStrictMode, - browserTheme, - }); - } else { - if (consoleManagedByDevToolsDuringStrictMode) { - patchConsoleForInitialRenderInExtension(renderer, { - hideConsoleLogsInStrictMode, - browserTheme, - }); - } - } + registerRendererWithConsole(renderer); + patchConsole({ + appendComponentStack, + breakOnConsoleErrors, + showInlineWarningsAndErrors, + hideConsoleLogsInStrictMode, + browserTheme, + }); } catch (error) {} } @@ -473,6 +470,32 @@ export function installHook(target: any): DevToolsHook | null { } } + function setStrictMode(rendererID, isStrictMode) { + const rendererInterface = rendererInterfaces.get(rendererID); + if (rendererInterface != null) { + if (isStrictMode) { + rendererInterface.patchConsoleForStrictMode(); + } else { + rendererInterface.unpatchConsoleForStrictMode(); + } + } else { + // This should only happen during initial render in the extension before DevTools + // finishes its handshake with the injected renderer + if (isStrictMode) { + const hideConsoleLogsInStrictMode = + window.__REACT_DEVTOOLS_HIDE_CONSOLE_LOGS_IN_STRICT_MODE__ === true; + const browserTheme = window.__REACT_DEVTOOLS_BROWSER_THEME__; + + patchConsoleForInitialRenderInStrictMode({ + hideConsoleLogsInStrictMode, + browserTheme, + }); + } else { + unpatchConsoleForInitialRenderInStrictMode(); + } + } + } + // TODO: More meaningful names for "rendererInterfaces" and "renderers". const fiberRoots = {}; const rendererInterfaces = new Map(); @@ -502,8 +525,13 @@ export function installHook(target: any): DevToolsHook | null { onCommitFiberUnmount, onCommitFiberRoot, onPostCommitFiberRoot, + setStrictMode, }; + if (__TEST__) { + hook.dangerous_setTargetConsoleForTesting = dangerous_setTargetConsoleForTesting; + } + Object.defineProperty( target, '__REACT_DEVTOOLS_GLOBAL_HOOK__', diff --git a/packages/react-devtools-shell/src/app/console.js b/packages/react-devtools-shell/src/app/console.js index 9f1244bf164e1..22338f28844dc 100644 --- a/packages/react-devtools-shell/src/app/console.js +++ b/packages/react-devtools-shell/src/app/console.js @@ -11,12 +11,6 @@ function ignoreStrings( methodName: string, stringsToIgnore: Array, ): void { - // HACKY In the test harness, DevTools overrides the parent window's console. - // Our test app code uses the iframe's console though. - // To simulate a more accurate end-to-end environment, - // the shell's console patching should pass through to the parent override methods. - const originalMethod = window.parent.console[methodName]; - console[methodName] = (...args) => { const maybeString = args[0]; if (typeof maybeString === 'string') { @@ -26,7 +20,12 @@ function ignoreStrings( } } } - originalMethod(...args); + + // HACKY In the test harness, DevTools overrides the parent window's console. + // Our test app code uses the iframe's console though. + // To simulate a more accurate end-to-end environment, + // the shell's console patching should pass through to the parent override methods. + window.parent.console[methodName](...args); }; } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 894bc523e4b58..8c89c369a5a66 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -149,11 +149,7 @@ import { getOffscreenContainerProps, } from './ReactFiberHostConfig'; import type {SuspenseInstance} from './ReactFiberHostConfig'; -import { - shouldError, - shouldSuspend, - setIsStrictModeForDevtools, -} from './ReactFiberReconciler'; +import {shouldError, shouldSuspend} from './ReactFiberReconciler'; import {pushHostContext, pushHostContainer} from './ReactFiberHostContext.new'; import { suspenseStackCursor, @@ -235,6 +231,7 @@ import {createCapturedValue} from './ReactCapturedValue'; import {createClassErrorUpdate} from './ReactFiberThrow.new'; import {completeSuspendedOffscreenHostContainer} from './ReactFiberCompleteWork.new'; import is from 'shared/objectIs'; +import {setIsStrictModeForDevtools} from './ReactFiberDevToolsHook.new'; const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index 264e723b28973..d56e2a2b5ff18 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -149,11 +149,7 @@ import { getOffscreenContainerProps, } from './ReactFiberHostConfig'; import type {SuspenseInstance} from './ReactFiberHostConfig'; -import { - shouldError, - shouldSuspend, - setIsStrictModeForDevtools, -} from './ReactFiberReconciler'; +import {shouldError, shouldSuspend} from './ReactFiberReconciler'; import {pushHostContext, pushHostContainer} from './ReactFiberHostContext.old'; import { suspenseStackCursor, @@ -235,6 +231,7 @@ import {createCapturedValue} from './ReactCapturedValue'; import {createClassErrorUpdate} from './ReactFiberThrow.old'; import {completeSuspendedOffscreenHostContainer} from './ReactFiberCompleteWork.old'; import is from 'shared/objectIs'; +import {setIsStrictModeForDevtools} from './ReactFiberDevToolsHook.old'; const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.new.js b/packages/react-reconciler/src/ReactFiberClassComponent.new.js index 490fa5f430f21..3cb67d045d410 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.new.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.new.js @@ -38,7 +38,7 @@ import getComponentNameFromType from 'shared/getComponentNameFromType'; import invariant from 'shared/invariant'; import isArray from 'shared/isArray'; import {REACT_CONTEXT_TYPE, REACT_PROVIDER_TYPE} from 'shared/ReactSymbols'; -import {setIsStrictModeForDevtools} from './ReactFiberReconciler'; +import {setIsStrictModeForDevtools} from './ReactFiberDevToolsHook.new'; import {resolveDefaultProps} from './ReactFiberLazyComponent.new'; import { diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.old.js b/packages/react-reconciler/src/ReactFiberClassComponent.old.js index 8c08eb581892b..0e30466bf82be 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.old.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.old.js @@ -38,7 +38,7 @@ import getComponentNameFromType from 'shared/getComponentNameFromType'; import invariant from 'shared/invariant'; import isArray from 'shared/isArray'; import {REACT_CONTEXT_TYPE, REACT_PROVIDER_TYPE} from 'shared/ReactSymbols'; -import {setIsStrictModeForDevtools} from './ReactFiberReconciler'; +import {setIsStrictModeForDevtools} from './ReactFiberDevToolsHook.old'; import {resolveDefaultProps} from './ReactFiberLazyComponent.old'; import { diff --git a/packages/react-reconciler/src/ReactFiberDevToolsHook.new.js b/packages/react-reconciler/src/ReactFiberDevToolsHook.new.js index 52d1c68730c61..d1748dd8f2285 100644 --- a/packages/react-reconciler/src/ReactFiberDevToolsHook.new.js +++ b/packages/react-reconciler/src/ReactFiberDevToolsHook.new.js @@ -7,7 +7,10 @@ * @flow */ -import {enableProfilerTimer} from 'shared/ReactFeatureFlags'; +import { + consoleManagedByDevToolsDuringStrictMode, + enableProfilerTimer, +} from 'shared/ReactFeatureFlags'; import type {Fiber, FiberRoot} from './ReactInternalTypes'; import type {ReactNodeList} from 'shared/ReactTypes'; @@ -25,7 +28,11 @@ import { UserBlockingPriority as UserBlockingSchedulerPriority, NormalPriority as NormalSchedulerPriority, IdlePriority as IdleSchedulerPriority, + unstable_yieldValue, + unstable_setDisableYieldValue, } from './Scheduler'; +import {setSuppressWarning} from 'shared/consoleWithStackDev'; +import {disableLogs, reenableLogs} from 'shared/ConsolePatchingDev'; declare var __REACT_DEVTOOLS_GLOBAL_HOOK__: Object | void; @@ -171,3 +178,37 @@ export function onCommitUnmount(fiber: Fiber) { } } } + +export function setIsStrictModeForDevtools(newIsStrictMode: boolean) { + if (consoleManagedByDevToolsDuringStrictMode) { + if (typeof unstable_yieldValue === 'function') { + // We're in a test because Scheduler.unstable_yieldValue only exists + // in SchedulerMock. To reduce the noise in strict mode tests, + // suppress warnings and disable scheduler yielding during the double render + unstable_setDisableYieldValue(newIsStrictMode); + setSuppressWarning(newIsStrictMode); + } + + if (injectedHook && typeof injectedHook.setStrictMode === 'function') { + try { + injectedHook.setStrictMode(rendererID, newIsStrictMode); + } catch (err) { + if (__DEV__) { + if (!hasLoggedError) { + hasLoggedError = true; + console.error( + 'React instrumentation encountered an error: %s', + err, + ); + } + } + } + } + } else { + if (newIsStrictMode) { + disableLogs(); + } else { + reenableLogs(); + } + } +} diff --git a/packages/react-reconciler/src/ReactFiberDevToolsHook.old.js b/packages/react-reconciler/src/ReactFiberDevToolsHook.old.js index 42fd68a93a8bc..aeaadc84b0193 100644 --- a/packages/react-reconciler/src/ReactFiberDevToolsHook.old.js +++ b/packages/react-reconciler/src/ReactFiberDevToolsHook.old.js @@ -7,7 +7,10 @@ * @flow */ -import {enableProfilerTimer} from 'shared/ReactFeatureFlags'; +import { + consoleManagedByDevToolsDuringStrictMode, + enableProfilerTimer, +} from 'shared/ReactFeatureFlags'; import type {Fiber, FiberRoot} from './ReactInternalTypes'; import type {ReactNodeList} from 'shared/ReactTypes'; @@ -25,7 +28,11 @@ import { UserBlockingPriority as UserBlockingSchedulerPriority, NormalPriority as NormalSchedulerPriority, IdlePriority as IdleSchedulerPriority, + unstable_yieldValue, + unstable_setDisableYieldValue, } from './Scheduler'; +import {setSuppressWarning} from 'shared/consoleWithStackDev'; +import {disableLogs, reenableLogs} from 'shared/ConsolePatchingDev'; declare var __REACT_DEVTOOLS_GLOBAL_HOOK__: Object | void; @@ -171,3 +178,37 @@ export function onCommitUnmount(fiber: Fiber) { } } } + +export function setIsStrictModeForDevtools(newIsStrictMode: boolean) { + if (consoleManagedByDevToolsDuringStrictMode) { + if (typeof unstable_yieldValue === 'function') { + // We're in a test because Scheduler.unstable_yieldValue only exists + // in SchedulerMock. To reduce the noise in strict mode tests, + // suppress warnings and disable scheduler yielding during the double render + unstable_setDisableYieldValue(newIsStrictMode); + setSuppressWarning(newIsStrictMode); + } + + if (injectedHook && typeof injectedHook.setStrictMode === 'function') { + try { + injectedHook.setStrictMode(rendererID, newIsStrictMode); + } catch (err) { + if (__DEV__) { + if (!hasLoggedError) { + hasLoggedError = true; + console.error( + 'React instrumentation encountered an error: %s', + err, + ); + } + } + } + } + } else { + if (newIsStrictMode) { + disableLogs(); + } else { + reenableLogs(); + } + } +} diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index ae029cd01580b..1f92626fa8b2c 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -48,8 +48,6 @@ import { observeVisibleRects as observeVisibleRects_old, runWithPriority as runWithPriority_old, getCurrentUpdatePriority as getCurrentUpdatePriority_old, - getIsStrictModeForDevtools as getIsStrictModeForDevtools_old, - setIsStrictModeForDevtools as setIsStrictModeForDevtools_old, } from './ReactFiberReconciler.old'; import { @@ -86,8 +84,6 @@ import { observeVisibleRects as observeVisibleRects_new, runWithPriority as runWithPriority_new, getCurrentUpdatePriority as getCurrentUpdatePriority_new, - getIsStrictModeForDevtools as getIsStrictModeForDevtools_new, - setIsStrictModeForDevtools as setIsStrictModeForDevtools_new, } from './ReactFiberReconciler.new'; export const createContainer = enableNewReconciler @@ -189,10 +185,3 @@ export const observeVisibleRects = enableNewReconciler export const runWithPriority = enableNewReconciler ? runWithPriority_new : runWithPriority_old; - -export const getIsStrictModeForDevtools = enableNewReconciler - ? getIsStrictModeForDevtools_new - : getIsStrictModeForDevtools_old; -export const setIsStrictModeForDevtools = enableNewReconciler - ? setIsStrictModeForDevtools_new - : setIsStrictModeForDevtools_old; diff --git a/packages/react-reconciler/src/ReactFiberReconciler.new.js b/packages/react-reconciler/src/ReactFiberReconciler.new.js index 4219fdc72cf05..c9b78964c1725 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.new.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.new.js @@ -35,10 +35,7 @@ import { import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; import invariant from 'shared/invariant'; import isArray from 'shared/isArray'; -import { - enableSchedulingProfiler, - consoleManagedByDevToolsDuringStrictMode, -} from 'shared/ReactFeatureFlags'; +import {enableSchedulingProfiler} from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import {getPublicInstance} from './ReactFiberHostConfig'; import { @@ -107,10 +104,6 @@ export { observeVisibleRects, } from './ReactTestSelectors'; -import * as Scheduler from './Scheduler'; -import {setSuppressWarning} from 'shared/consoleWithStackDev'; -import {disableLogs, reenableLogs} from 'shared/ConsolePatchingDev'; - type OpaqueRoot = FiberRoot; // 0 is PROD, 1 is DEV. @@ -464,8 +457,6 @@ export function shouldSuspend(fiber: Fiber): boolean { return shouldSuspendImpl(fiber); } -let isStrictMode = false; - let overrideHookState = null; let overrideHookStateDeletePath = null; let overrideHookStateRenamePath = null; @@ -715,30 +706,6 @@ function getCurrentFiberForDevTools() { return ReactCurrentFiberCurrent; } -export function getIsStrictModeForDevtools() { - return isStrictMode; -} - -export function setIsStrictModeForDevtools(newIsStrictMode: boolean) { - isStrictMode = newIsStrictMode; - - if (consoleManagedByDevToolsDuringStrictMode) { - // We're in a test because Scheduler.unstable_yieldValue only exists - // in SchedulerMock. To reduce the noise in strict mode tests, - // suppress warnings and disable scheduler yielding during the double render - if (typeof Scheduler.unstable_yieldValue === 'function') { - Scheduler.unstable_setDisableYieldValue(newIsStrictMode); - setSuppressWarning(newIsStrictMode); - } - } else { - if (newIsStrictMode) { - disableLogs(); - } else { - reenableLogs(); - } - } -} - export function injectIntoDevTools(devToolsConfig: DevToolsConfig): boolean { const {findFiberByHostInstance} = devToolsConfig; const {ReactCurrentDispatcher} = ReactSharedInternals; @@ -768,7 +735,6 @@ export function injectIntoDevTools(devToolsConfig: DevToolsConfig): boolean { setRefreshHandler: __DEV__ ? setRefreshHandler : null, // Enables DevTools to append owner stacks to error messages in DEV mode. getCurrentFiber: __DEV__ ? getCurrentFiberForDevTools : null, - getIsStrictMode: __DEV__ ? getIsStrictModeForDevtools : null, // Enables DevTools to detect reconciler version rather than renderer version // which may not match for third party renderers. reconcilerVersion: ReactVersion, diff --git a/packages/react-reconciler/src/ReactFiberReconciler.old.js b/packages/react-reconciler/src/ReactFiberReconciler.old.js index 84bcb7e6e0d1a..6ad7587eed230 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.old.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.old.js @@ -35,10 +35,7 @@ import { import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; import invariant from 'shared/invariant'; import isArray from 'shared/isArray'; -import { - enableSchedulingProfiler, - consoleManagedByDevToolsDuringStrictMode, -} from 'shared/ReactFeatureFlags'; +import {enableSchedulingProfiler} from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import {getPublicInstance} from './ReactFiberHostConfig'; import { @@ -107,10 +104,6 @@ export { observeVisibleRects, } from './ReactTestSelectors'; -import * as Scheduler from './Scheduler'; -import {setSuppressWarning} from 'shared/consoleWithStackDev'; -import {disableLogs, reenableLogs} from 'shared/ConsolePatchingDev'; - type OpaqueRoot = FiberRoot; // 0 is PROD, 1 is DEV. @@ -464,8 +457,6 @@ export function shouldSuspend(fiber: Fiber): boolean { return shouldSuspendImpl(fiber); } -let isStrictMode = false; - let overrideHookState = null; let overrideHookStateDeletePath = null; let overrideHookStateRenamePath = null; @@ -715,30 +706,6 @@ function getCurrentFiberForDevTools() { return ReactCurrentFiberCurrent; } -export function getIsStrictModeForDevtools() { - return isStrictMode; -} - -export function setIsStrictModeForDevtools(newIsStrictMode: boolean) { - isStrictMode = newIsStrictMode; - - if (consoleManagedByDevToolsDuringStrictMode) { - // We're in a test because Scheduler.unstable_yieldValue only exists - // in SchedulerMock. To reduce the noise in strict mode tests, - // suppress warnings and disable scheduler yielding during the double render - if (typeof Scheduler.unstable_yieldValue === 'function') { - Scheduler.unstable_setDisableYieldValue(newIsStrictMode); - setSuppressWarning(newIsStrictMode); - } - } else { - if (newIsStrictMode) { - disableLogs(); - } else { - reenableLogs(); - } - } -} - export function injectIntoDevTools(devToolsConfig: DevToolsConfig): boolean { const {findFiberByHostInstance} = devToolsConfig; const {ReactCurrentDispatcher} = ReactSharedInternals; @@ -768,7 +735,6 @@ export function injectIntoDevTools(devToolsConfig: DevToolsConfig): boolean { setRefreshHandler: __DEV__ ? setRefreshHandler : null, // Enables DevTools to append owner stacks to error messages in DEV mode. getCurrentFiber: __DEV__ ? getCurrentFiberForDevTools : null, - getIsStrictMode: __DEV__ ? getIsStrictModeForDevtools : null, // Enables DevTools to detect reconciler version rather than renderer version // which may not match for third party renderers. reconcilerVersion: ReactVersion, diff --git a/packages/react-reconciler/src/ReactUpdateQueue.new.js b/packages/react-reconciler/src/ReactUpdateQueue.new.js index a42384322fcdb..cbfe307b903f2 100644 --- a/packages/react-reconciler/src/ReactUpdateQueue.new.js +++ b/packages/react-reconciler/src/ReactUpdateQueue.new.js @@ -110,7 +110,7 @@ import { isInterleavedUpdate, } from './ReactFiberWorkLoop.new'; import {pushInterleavedQueue} from './ReactFiberInterleavedUpdates.new'; -import {setIsStrictModeForDevtools} from './ReactFiberReconciler'; +import {setIsStrictModeForDevtools} from './ReactFiberDevToolsHook.new'; import invariant from 'shared/invariant'; diff --git a/packages/react-reconciler/src/ReactUpdateQueue.old.js b/packages/react-reconciler/src/ReactUpdateQueue.old.js index 724dfa0d7171b..ece4321d6bfdb 100644 --- a/packages/react-reconciler/src/ReactUpdateQueue.old.js +++ b/packages/react-reconciler/src/ReactUpdateQueue.old.js @@ -110,7 +110,7 @@ import { isInterleavedUpdate, } from './ReactFiberWorkLoop.old'; import {pushInterleavedQueue} from './ReactFiberInterleavedUpdates.old'; -import {setIsStrictModeForDevtools} from './ReactFiberReconciler'; +import {setIsStrictModeForDevtools} from './ReactFiberDevToolsHook.old'; import invariant from 'shared/invariant';