From 107a2f8c3e43ee5f4f6872e68cd9aff14f3fa6d3 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Mon, 17 Jun 2024 16:38:03 +0100 Subject: [PATCH] chore[react-devtools]: improve console arguments formatting before passing it to original console (#29873) Stacked on https://github.com/facebook/react/pull/29869. ## Summary When using ANSI escape sequences, we construct a message in the following way: `console.('\x1b...%s\x1b[0m', userspaceArgument1?, userspaceArgument2?, userspaceArgument3?, ...)`. This won't dim all arguments, if user had something like `console.log(1, 2, 3)`, we would only apply it to `1`, since this is the first arguments, so we need to: - inline everything whats possible into a single string, while preserving console substitutions defined by the user - omit css and object substitutions, since we can't really inline them and will delegate in to the environment ## How did you test this change? Added some tests, manually inspected that it works well for web and native cases. --- .../src/__tests__/utils-test.js | 90 ++++++++++++++----- .../src/backend/console.js | 10 ++- .../src/backend/renderer.js | 11 ++- .../src/backend/utils.js | 66 +++++++++++++- packages/react-devtools-shared/src/hook.js | 60 ++++++++++++- 5 files changed, 209 insertions(+), 28 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/utils-test.js b/packages/react-devtools-shared/src/__tests__/utils-test.js index 71cd2aaf38dc0..9e781e072ba75 100644 --- a/packages/react-devtools-shared/src/__tests__/utils-test.js +++ b/packages/react-devtools-shared/src/__tests__/utils-test.js @@ -14,7 +14,8 @@ import { } from 'react-devtools-shared/src/utils'; import {stackToComponentSources} from 'react-devtools-shared/src/devtools/utils'; import { - format, + formatConsoleArguments, + formatConsoleArgumentsToSingleString, formatWithStyles, gt, gte, @@ -123,51 +124,51 @@ describe('utils', () => { }); }); - describe('format', () => { - // @reactVersion >= 16.0 + describe('formatConsoleArgumentsToSingleString', () => { it('should format simple strings', () => { - expect(format('a', 'b', 'c')).toEqual('a b c'); + expect(formatConsoleArgumentsToSingleString('a', 'b', 'c')).toEqual( + 'a b c', + ); }); - // @reactVersion >= 16.0 it('should format multiple argument types', () => { - expect(format('abc', 123, true)).toEqual('abc 123 true'); + expect(formatConsoleArgumentsToSingleString('abc', 123, true)).toEqual( + 'abc 123 true', + ); }); - // @reactVersion >= 16.0 it('should support string substitutions', () => { - expect(format('a %s b %s c', 123, true)).toEqual('a 123 b true c'); + expect( + formatConsoleArgumentsToSingleString('a %s b %s c', 123, true), + ).toEqual('a 123 b true c'); }); - // @reactVersion >= 16.0 it('should gracefully handle Symbol types', () => { - expect(format(Symbol('a'), 'b', Symbol('c'))).toEqual( - 'Symbol(a) b Symbol(c)', - ); + expect( + formatConsoleArgumentsToSingleString(Symbol('a'), 'b', Symbol('c')), + ).toEqual('Symbol(a) b Symbol(c)'); }); - // @reactVersion >= 16.0 it('should gracefully handle Symbol type for the first argument', () => { - expect(format(Symbol('abc'), 123)).toEqual('Symbol(abc) 123'); + expect(formatConsoleArgumentsToSingleString(Symbol('abc'), 123)).toEqual( + 'Symbol(abc) 123', + ); }); }); describe('formatWithStyles', () => { - // @reactVersion >= 16.0 it('should format empty arrays', () => { expect(formatWithStyles([])).toEqual([]); expect(formatWithStyles([], 'gray')).toEqual([]); expect(formatWithStyles(undefined)).toEqual(undefined); }); - // @reactVersion >= 16.0 it('should bail out of strings with styles', () => { expect( formatWithStyles(['%ca', 'color: green', 'b', 'c'], 'color: gray'), ).toEqual(['%ca', 'color: green', 'b', 'c']); }); - // @reactVersion >= 16.0 it('should format simple strings', () => { expect(formatWithStyles(['a'])).toEqual(['a']); @@ -186,7 +187,6 @@ describe('utils', () => { ]); }); - // @reactVersion >= 16.0 it('should format string substituions', () => { expect( formatWithStyles(['%s %s %s', 'a', 'b', 'c'], 'color: gray'), @@ -199,7 +199,6 @@ describe('utils', () => { ); }); - // @reactVersion >= 16.0 it('should support multiple argument types', () => { const symbol = Symbol('a'); expect( @@ -219,7 +218,6 @@ describe('utils', () => { ]); }); - // @reactVersion >= 16.0 it('should properly format escaped string substituions', () => { expect(formatWithStyles(['%%s'], 'color: gray')).toEqual([ '%c%s', @@ -234,7 +232,6 @@ describe('utils', () => { expect(formatWithStyles(['%%c%c'], 'color: gray')).toEqual(['%%c%c']); }); - // @reactVersion >= 16.0 it('should format non string inputs as the first argument', () => { expect(formatWithStyles([{foo: 'bar'}])).toEqual([{foo: 'bar'}]); expect(formatWithStyles([[1, 2, 3]])).toEqual([[1, 2, 3]]); @@ -387,4 +384,55 @@ describe('utils', () => { }); }); }); + + describe('formatConsoleArguments', () => { + it('works with empty arguments list', () => { + expect(formatConsoleArguments(...[])).toEqual([]); + }); + + it('works for string without escape sequences', () => { + expect( + formatConsoleArguments('This is the template', 'And another string'), + ).toEqual(['This is the template', 'And another string']); + }); + + it('works with strings templates', () => { + expect(formatConsoleArguments('This is %s template', 'the')).toEqual([ + 'This is the template', + ]); + }); + + it('skips %%s', () => { + expect(formatConsoleArguments('This %%s is %s template', 'the')).toEqual([ + 'This %%s is the template', + ]); + }); + + it('works with %%%s', () => { + expect( + formatConsoleArguments('This %%%s is %s template', 'test', 'the'), + ).toEqual(['This %%test is the template']); + }); + + it("doesn't inline objects", () => { + expect( + formatConsoleArguments('This is %s template with object %o', 'the', {}), + ).toEqual(['This is the template with object %o', {}]); + }); + + it("doesn't inline css", () => { + expect( + formatConsoleArguments( + 'This is template with %c %s object %o', + 'color: rgba(...)', + 'the', + {}, + ), + ).toEqual([ + 'This is template with %c the object %o', + 'color: rgba(...)', + {}, + ]); + }); + }); }); diff --git a/packages/react-devtools-shared/src/backend/console.js b/packages/react-devtools-shared/src/backend/console.js index 081df398893da..524bad82cca2b 100644 --- a/packages/react-devtools-shared/src/backend/console.js +++ b/packages/react-devtools-shared/src/backend/console.js @@ -15,8 +15,11 @@ import type { WorkTagMap, ConsolePatchSettings, } from './types'; -import {formatWithStyles} from './utils'; +import { + formatConsoleArguments, + formatWithStyles, +} from 'react-devtools-shared/src/backend/utils'; import { FIREFOX_CONSOLE_DIMMING_COLOR, ANSI_STYLE_DIMMING_TEMPLATE, @@ -335,7 +338,10 @@ export function patchForStrictMode() { ...formatWithStyles(args, FIREFOX_CONSOLE_DIMMING_COLOR), ); } else { - originalMethod(ANSI_STYLE_DIMMING_TEMPLATE, ...args); + originalMethod( + ANSI_STYLE_DIMMING_TEMPLATE, + ...formatConsoleArguments(...args), + ); } } }; diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 666493cbd9a3d..fa2b098a81dc2 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -40,6 +40,7 @@ import { } from 'react-devtools-shared/src/utils'; import {sessionStorageGetItem} from 'react-devtools-shared/src/storage'; import { + formatConsoleArgumentsToSingleString, gt, gte, parseSourceFromComponentStack, @@ -95,7 +96,6 @@ import { MEMO_SYMBOL_STRING, SERVER_CONTEXT_SYMBOL_STRING, } from './ReactSymbols'; -import {format} from './utils'; import {enableStyleXFeatures} from 'react-devtools-feature-flags'; import is from 'shared/objectIs'; import hasOwnProperty from 'shared/hasOwnProperty'; @@ -851,7 +851,14 @@ export function attach( return; } } - const message = format(...args); + + // We can't really use this message as a unique key, since we can't distinguish + // different objects in this implementation. We have to delegate displaying of the objects + // to the environment, the browser console, for example, so this is why this should be kept + // as an array of arguments, instead of the plain string. + // [Warning: %o, {...}] and [Warning: %o, {...}] will be considered as the same message, + // even if objects are different + const message = formatConsoleArgumentsToSingleString(...args); if (__DEBUG__) { debug('onErrorOrWarning', fiber, null, `${type}: "${message}"`); } diff --git a/packages/react-devtools-shared/src/backend/utils.js b/packages/react-devtools-shared/src/backend/utils.js index ace070d8b6dd8..bd762467b6481 100644 --- a/packages/react-devtools-shared/src/backend/utils.js +++ b/packages/react-devtools-shared/src/backend/utils.js @@ -164,6 +164,7 @@ export function serializeToString(data: any): string { ); } +// NOTE: KEEP IN SYNC with src/hook.js // Formats an array of args with a style for console methods, using // the following algorithm: // 1. The first param is a string that contains %c @@ -220,11 +221,72 @@ export function formatWithStyles( } } +// NOTE: KEEP IN SYNC with src/hook.js +// Skips CSS and object arguments, inlines other in the first argument as a template string +export function formatConsoleArguments( + maybeMessage: any, + ...inputArgs: $ReadOnlyArray +): $ReadOnlyArray { + if (inputArgs.length === 0 || typeof maybeMessage !== 'string') { + return [maybeMessage, ...inputArgs]; + } + + const args = inputArgs.slice(); + + let template = ''; + let argumentsPointer = 0; + for (let i = 0; i < maybeMessage.length; ++i) { + const currentChar = maybeMessage[i]; + if (currentChar !== '%') { + template += currentChar; + continue; + } + + const nextChar = maybeMessage[i + 1]; + ++i; + + // Only keep CSS and objects, inline other arguments + switch (nextChar) { + case 'c': + case 'O': + case 'o': { + ++argumentsPointer; + template += `%${nextChar}`; + + break; + } + case 'd': + case 'i': { + const [arg] = args.splice(argumentsPointer, 1); + template += parseInt(arg, 10).toString(); + + break; + } + case 'f': { + const [arg] = args.splice(argumentsPointer, 1); + template += parseFloat(arg).toString(); + + break; + } + case 's': { + const [arg] = args.splice(argumentsPointer, 1); + template += arg.toString(); + + break; + } + + default: + template += `%${nextChar}`; + } + } + + return [template, ...args]; +} + // based on https://github.com/tmpfs/format-util/blob/0e62d430efb0a1c51448709abd3e2406c14d8401/format.js#L1 // based on https://developer.mozilla.org/en-US/docs/Web/API/console#Using_string_substitutions // Implements s, d, i and f placeholders -// NOTE: KEEP IN SYNC with src/hook.js -export function format( +export function formatConsoleArgumentsToSingleString( maybeMessage: any, ...inputArgs: $ReadOnlyArray ): string { diff --git a/packages/react-devtools-shared/src/hook.js b/packages/react-devtools-shared/src/hook.js index d16c09e8ce77c..0dd96dc471cbc 100644 --- a/packages/react-devtools-shared/src/hook.js +++ b/packages/react-devtools-shared/src/hook.js @@ -220,6 +220,61 @@ export function installHook(target: any): DevToolsHook | null { return [firstArg, style, ...inputArgs]; } } + // NOTE: KEEP IN SYNC with src/backend/utils.js + function formatConsoleArguments( + maybeMessage: any, + ...inputArgs: $ReadOnlyArray + ): $ReadOnlyArray { + if (inputArgs.length === 0 || typeof maybeMessage !== 'string') { + return [maybeMessage, ...inputArgs]; + } + + const args = inputArgs.slice(); + + let template = ''; + let argumentsPointer = 0; + for (let i = 0; i < maybeMessage.length; ++i) { + const currentChar = maybeMessage[i]; + if (currentChar !== '%') { + template += currentChar; + continue; + } + + const nextChar = maybeMessage[i + 1]; + ++i; + + // Only keep CSS and objects, inline other arguments + switch (nextChar) { + case 'c': + case 'O': + case 'o': { + ++argumentsPointer; + template += `%${nextChar}`; + + break; + } + case 'd': + case 'i': { + const [arg] = args.splice(argumentsPointer, 1); + template += parseInt(arg, 10).toString(); + + break; + } + case 'f': { + const [arg] = args.splice(argumentsPointer, 1); + template += parseFloat(arg).toString(); + + break; + } + case 's': { + const [arg] = args.splice(argumentsPointer, 1); + template += arg.toString(); + } + } + } + + return [template, ...args]; + } let unpatchFn = null; @@ -274,7 +329,10 @@ export function installHook(target: any): DevToolsHook | null { ...formatWithStyles(args, FIREFOX_CONSOLE_DIMMING_COLOR), ); } else { - originalMethod(ANSI_STYLE_DIMMING_TEMPLATE, ...args); + originalMethod( + ANSI_STYLE_DIMMING_TEMPLATE, + ...formatConsoleArguments(...args), + ); } } };