From a9dc73cfd4db480b4cadf8624351004fe96f5a12 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Tue, 6 Sep 2022 19:03:27 +0200 Subject: [PATCH] Handle info, group, and groupCollapsed in Strict Mode logging (#25172) * Handle info, group, and groupCollapsed in Strict Mode logging While working on the new Next.js router which heavily relies on useReducer I noticed that `group` and `groupCollapsed` which both take labels were showing as-is in the console for the second render/dispatch in Strict Mode logs. While looking at the code I found that `info` was also not instrumented. I've added additional handling for: - `info` - `group` - `groupCollapsed` * Remove console.log * Fix tests --- .../src/__tests__/console-test.js | 65 +++++++++++++++++++ .../src/backend/console.js | 10 ++- packages/react-devtools-shared/src/hook.js | 10 ++- 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/console-test.js b/packages/react-devtools-shared/src/__tests__/console-test.js index 40eb388a955ca..407150b5e39b8 100644 --- a/packages/react-devtools-shared/src/__tests__/console-test.js +++ b/packages/react-devtools-shared/src/__tests__/console-test.js @@ -16,6 +16,8 @@ let fakeConsole; let legacyRender; let mockError; let mockInfo; +let mockGroup; +let mockGroupCollapsed; let mockLog; let mockWarn; let patchConsole; @@ -25,6 +27,7 @@ let rendererID; describe('console', () => { beforeEach(() => { const Console = require('react-devtools-shared/src/backend/console'); + patchConsole = Console.patch; unpatchConsole = Console.unpatch; @@ -33,6 +36,8 @@ describe('console', () => { // because Jest itself has hooks into it as does our test env setup. mockError = jest.fn(); mockInfo = jest.fn(); + mockGroup = jest.fn(); + mockGroupCollapsed = jest.fn(); mockLog = jest.fn(); mockWarn = jest.fn(); fakeConsole = { @@ -40,6 +45,8 @@ describe('console', () => { info: mockInfo, log: mockLog, warn: mockWarn, + group: mockGroup, + groupCollapsed: mockGroupCollapsed, }; Console.dangerous_setTargetConsoleForTesting(fakeConsole); @@ -69,6 +76,8 @@ describe('console', () => { expect(fakeConsole.info).toBe(mockInfo); expect(fakeConsole.log).toBe(mockLog); expect(fakeConsole.warn).not.toBe(mockWarn); + expect(fakeConsole.group).toBe(mockGroup); + expect(fakeConsole.groupCollapsed).toBe(mockGroupCollapsed); }); // @reactVersion >=18.0 @@ -491,6 +500,9 @@ describe('console', () => { fakeConsole.log('log'); fakeConsole.warn('warn'); fakeConsole.error('error'); + fakeConsole.info('info'); + fakeConsole.group('group'); + fakeConsole.groupCollapsed('groupCollapsed'); return
; } @@ -528,6 +540,36 @@ describe('console', () => { `color: ${process.env.DARK_MODE_DIMMED_ERROR_COLOR}`, 'error', ]); + + expect(mockInfo).toHaveBeenCalledTimes(2); + expect(mockInfo.mock.calls[0]).toHaveLength(1); + expect(mockInfo.mock.calls[0][0]).toBe('info'); + expect(mockInfo.mock.calls[1]).toHaveLength(3); + expect(mockInfo.mock.calls[1]).toEqual([ + '%c%s', + `color: ${process.env.DARK_MODE_DIMMED_LOG_COLOR}`, + 'info', + ]); + + expect(mockGroup).toHaveBeenCalledTimes(2); + expect(mockGroup.mock.calls[0]).toHaveLength(1); + expect(mockGroup.mock.calls[0][0]).toBe('group'); + expect(mockGroup.mock.calls[1]).toHaveLength(3); + expect(mockGroup.mock.calls[1]).toEqual([ + '%c%s', + `color: ${process.env.DARK_MODE_DIMMED_LOG_COLOR}`, + 'group', + ]); + + expect(mockGroupCollapsed).toHaveBeenCalledTimes(2); + expect(mockGroupCollapsed.mock.calls[0]).toHaveLength(1); + expect(mockGroupCollapsed.mock.calls[0][0]).toBe('groupCollapsed'); + expect(mockGroupCollapsed.mock.calls[1]).toHaveLength(3); + expect(mockGroupCollapsed.mock.calls[1]).toEqual([ + '%c%s', + `color: ${process.env.DARK_MODE_DIMMED_LOG_COLOR}`, + 'groupCollapsed', + ]); }); it('should not double log if hideConsoleLogsInStrictMode is enabled in Strict mode', () => { @@ -538,9 +580,16 @@ describe('console', () => { const root = ReactDOMClient.createRoot(container); function App() { + console.log( + 'CALL', + global.__REACT_DEVTOOLS_HIDE_CONSOLE_LOGS_IN_STRICT_MODE__, + ); fakeConsole.log('log'); fakeConsole.warn('warn'); fakeConsole.error('error'); + fakeConsole.info('info'); + fakeConsole.group('group'); + fakeConsole.groupCollapsed('groupCollapsed'); return
; } @@ -563,6 +612,18 @@ describe('console', () => { expect(mockError).toHaveBeenCalledTimes(1); expect(mockError.mock.calls[0]).toHaveLength(1); expect(mockError.mock.calls[0][0]).toBe('error'); + + expect(mockInfo).toHaveBeenCalledTimes(1); + expect(mockInfo.mock.calls[0]).toHaveLength(1); + expect(mockInfo.mock.calls[0][0]).toBe('info'); + + expect(mockGroup).toHaveBeenCalledTimes(1); + expect(mockGroup.mock.calls[0]).toHaveLength(1); + expect(mockGroup.mock.calls[0][0]).toBe('group'); + + expect(mockGroupCollapsed).toHaveBeenCalledTimes(1); + expect(mockGroupCollapsed.mock.calls[0]).toHaveLength(1); + expect(mockGroupCollapsed.mock.calls[0][0]).toBe('groupCollapsed'); }); it('should double log in Strict mode initial render for extension', () => { @@ -734,6 +795,8 @@ describe('console error', () => { // because Jest itself has hooks into it as does our test env setup. mockError = jest.fn(); mockInfo = jest.fn(); + mockGroup = jest.fn(); + mockGroupCollapsed = jest.fn(); mockLog = jest.fn(); mockWarn = jest.fn(); fakeConsole = { @@ -741,6 +804,8 @@ describe('console error', () => { info: mockInfo, log: mockLog, warn: mockWarn, + group: mockGroup, + groupCollapsed: mockGroupCollapsed, }; Console.dangerous_setTargetConsoleForTesting(fakeConsole); diff --git a/packages/react-devtools-shared/src/backend/console.js b/packages/react-devtools-shared/src/backend/console.js index 5a85b68deac32..ee802f8ea47a1 100644 --- a/packages/react-devtools-shared/src/backend/console.js +++ b/packages/react-devtools-shared/src/backend/console.js @@ -301,7 +301,15 @@ 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']; + const overrideConsoleMethods = [ + 'error', + 'group', + 'groupCollapsed', + 'info', + 'log', + 'trace', + 'warn', + ]; if (unpatchForStrictModeFn !== null) { // Don't patch twice. diff --git a/packages/react-devtools-shared/src/hook.js b/packages/react-devtools-shared/src/hook.js index 8adbb7534a254..29cdea1e2b0b7 100644 --- a/packages/react-devtools-shared/src/hook.js +++ b/packages/react-devtools-shared/src/hook.js @@ -228,7 +228,15 @@ export function installHook(target: any): DevToolsHook | null { hideConsoleLogsInStrictMode: boolean, browserTheme: BrowserTheme, }) { - const overrideConsoleMethods = ['error', 'trace', 'warn', 'log']; + const overrideConsoleMethods = [ + 'error', + 'group', + 'groupCollapsed', + 'info', + 'log', + 'trace', + 'warn', + ]; if (unpatchFn !== null) { // Don't patch twice.