From 75c642acae96ad54b45af63c4efa6f6400dbf6f8 Mon Sep 17 00:00:00 2001 From: Luna Date: Wed, 13 Apr 2022 18:25:31 -0500 Subject: [PATCH] don't stringify objects for console log second render --- .../src/__tests__/utils-test.js | 82 ++++++++++++++++++- .../src/backend/console.js | 4 +- .../src/backend/utils.js | 56 +++++++++++++ packages/react-devtools-shared/src/hook.js | 80 ++++++++---------- 4 files changed, 173 insertions(+), 49 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/utils-test.js b/packages/react-devtools-shared/src/__tests__/utils-test.js index 3ff01c5ef9600..02bc6be4657a7 100644 --- a/packages/react-devtools-shared/src/__tests__/utils-test.js +++ b/packages/react-devtools-shared/src/__tests__/utils-test.js @@ -11,7 +11,10 @@ import { getDisplayName, getDisplayNameForReactElement, } from 'react-devtools-shared/src/utils'; -import {format} from 'react-devtools-shared/src/backend/utils'; +import { + format, + formatWithStyles, +} from 'react-devtools-shared/src/backend/utils'; import { REACT_SUSPENSE_LIST_TYPE as SuspenseList, REACT_STRICT_MODE_TYPE as StrictMode, @@ -110,4 +113,81 @@ describe('utils', () => { expect(format(Symbol('abc'), 123)).toEqual('Symbol(abc) 123'); }); }); + + describe('formatWithStyles', () => { + it('should format empty arrays', () => { + expect(formatWithStyles([])).toEqual([]); + expect(formatWithStyles([], 'gray')).toEqual([]); + expect(formatWithStyles(undefined)).toEqual(undefined); + }); + + it('should bail out of strings with styles', () => { + expect( + formatWithStyles(['%ca', 'color: green', 'b', 'c'], 'color: gray'), + ).toEqual(['%ca', 'color: green', 'b', 'c']); + }); + + it('should format simple strings', () => { + expect(formatWithStyles(['a'])).toEqual(['a']); + + expect(formatWithStyles(['a', 'b', 'c'])).toEqual(['a', 'b', 'c']); + expect(formatWithStyles(['a'], 'color: gray')).toEqual([ + '%c%s', + 'color: gray', + 'a', + ]); + expect(formatWithStyles(['a', 'b', 'c'], 'color: gray')).toEqual([ + '%c%s %s %s', + 'color: gray', + 'a', + 'b', + 'c', + ]); + }); + + it('should format string substituions', () => { + expect( + formatWithStyles(['%s %s %s', 'a', 'b', 'c'], 'color: gray'), + ).toEqual(['%c%s %s %s', 'color: gray', 'a', 'b', 'c']); + + // The last letter isn't gray here but I think it's not a big + // deal, since there is a string substituion but it's incorrect + expect( + formatWithStyles(['%s %s', 'a', 'b', 'c'], 'color: gray'), + ).toEqual(['%c%s %s', 'color: gray', 'a', 'b', 'c']); + }); + + it('should support multiple argument types', () => { + const symbol = Symbol('a'); + expect( + formatWithStyles( + ['abc', 123, 12.3, true, {hello: 'world'}, symbol], + 'color: gray', + ), + ).toEqual([ + '%c%s %i %f %s %o %s', + 'color: gray', + 'abc', + 123, + 12.3, + true, + {hello: 'world'}, + symbol, + ]); + }); + + it('should properly format escaped string substituions', () => { + expect(formatWithStyles(['%%s'], 'color: gray')).toEqual([ + '%c%s', + 'color: gray', + '%%s', + ]); + expect(formatWithStyles(['%%c'], 'color: gray')).toEqual([ + '%c%s', + 'color: gray', + '%%c', + ]); + expect(formatWithStyles(['%%c%c'], 'color: gray')).toEqual(['%%c%c']); + }); + }); }); diff --git a/packages/react-devtools-shared/src/backend/console.js b/packages/react-devtools-shared/src/backend/console.js index 95d1f4d103329..d635a24b2b15f 100644 --- a/packages/react-devtools-shared/src/backend/console.js +++ b/packages/react-devtools-shared/src/backend/console.js @@ -10,7 +10,7 @@ import type {Fiber} from 'react-reconciler/src/ReactInternalTypes'; import type {CurrentDispatcherRef, ReactRenderer, WorkTagMap} from './types'; import type {BrowserTheme} from 'react-devtools-shared/src/devtools/views/DevTools'; -import {format} from './utils'; +import {format, formatWithStyles} from './utils'; import {getInternalReactConstants} from './renderer'; import {getStackByFiberInDevAndProd} from './DevToolsFiberComponentStack'; @@ -335,7 +335,7 @@ export function patchForStrictMode() { } else { const color = getConsoleColor(method); if (color) { - originalMethod(`%c${format(...args)}`, `color: ${color}`); + originalMethod(...formatWithStyles(args, `color: ${color}`)); } else { throw Error('Console color is not defined'); } diff --git a/packages/react-devtools-shared/src/backend/utils.js b/packages/react-devtools-shared/src/backend/utils.js index 09f821f4e1877..da911cf94458e 100644 --- a/packages/react-devtools-shared/src/backend/utils.js +++ b/packages/react-devtools-shared/src/backend/utils.js @@ -156,6 +156,62 @@ export function serializeToString(data: any): string { }); } +// 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 +// - Bail out and return the args without modifying the styles. +// We don't want to affect styles that the developer deliberately set. +// 2. The first param is a string that doesn't contain %c but contains +// string formatting +// - [`%c${args[0]}`, style, ...args.slice(1)] +// - Note: we assume that the string formatting that the developer uses +// is correct. +// 3. The first param is a string that doesn't contain string formatting +// OR is not a string +// - Create a formatting string where: +// boolean, string, symbol -> %s +// number -> %f OR %i depending on if it's an int or float +// default -> %o +export function formatWithStyles( + inputArgs: $ReadOnlyArray, + style?: string, +): $ReadOnlyArray { + if ( + inputArgs === undefined || + inputArgs === null || + inputArgs.length === 0 || + // Matches any of %c but not %%c + (typeof inputArgs[0] === 'string' && inputArgs[0].match(/([^%]|^)(%c)/g)) || + style === undefined + ) { + return inputArgs; + } + + // Matches any of %(o|O|i|s|f), but not %%(o|O|i|s|f) + const REGEXP = /([^%]|^)(%([oOdisf]))/g; + if (inputArgs[0].match(REGEXP)) { + return [`%c${inputArgs[0]}`, style, ...inputArgs.slice(1)]; + } else { + const firstArg = inputArgs.reduce((formatStr, elem, i) => { + if (i > 0) { + formatStr += ' '; + } + switch (typeof elem) { + case 'string': + case 'boolean': + case 'symbol': + return (formatStr += '%s'); + case 'number': + const formatting = Number.isInteger(elem) ? '%i' : '%f'; + return (formatStr += formatting); + default: + return (formatStr += '%o'); + } + }, '%c'); + return [firstArg, style, ...inputArgs]; + } +} + // 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 diff --git a/packages/react-devtools-shared/src/hook.js b/packages/react-devtools-shared/src/hook.js index 5844074863bf5..1ea54985fab68 100644 --- a/packages/react-devtools-shared/src/hook.js +++ b/packages/react-devtools-shared/src/hook.js @@ -172,54 +172,42 @@ export function installHook(target: any): DevToolsHook | null { } // NOTE: KEEP IN SYNC with src/backend/utils.js - function format( - maybeMessage: any, - ...inputArgs: $ReadOnlyArray - ): string { - const args = inputArgs.slice(); - - // Symbols cannot be concatenated with Strings. - let formatted = String(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; - }); - } + function formatWithStyles( + inputArgs: $ReadOnlyArray, + style?: string, + ): $ReadOnlyArray { + if ( + inputArgs === undefined || + inputArgs === null || + inputArgs.length === 0 || + (typeof inputArgs[0] === 'string' && inputArgs[0].includes('%c')) || + style === undefined + ) { + return inputArgs; } - // Arguments that remain after formatting. - if (args.length) { - for (let i = 0; i < args.length; i++) { - formatted += ' ' + String(args[i]); - } + const REGEXP = /(%?)(%([oOdisf]))/g; + if (inputArgs[0].match(REGEXP)) { + return [`%c${inputArgs[0]}`, style, ...inputArgs.slice(1)]; + } else { + const firstArg = inputArgs.reduce((formatStr, elem, i) => { + if (i > 0) { + formatStr += ' '; + } + switch (typeof elem) { + case 'string': + case 'boolean': + case 'symbol': + return (formatStr += '%s'); + case 'number': + const formatting = Number.isInteger(elem) ? '%i' : '%f'; + return (formatStr += formatting); + default: + return (formatStr += '%o'); + } + }, '%c'); + return [firstArg, style, ...inputArgs]; } - - // Update escaped %% values. - formatted = formatted.replace(/%{2,2}/g, '%'); - - return String(formatted); } let unpatchFn = null; @@ -291,7 +279,7 @@ export function installHook(target: any): DevToolsHook | null { } if (color) { - originalMethod(`%c${format(...args)}`, `color: ${color}`); + originalMethod(...formatWithStyles(args, `color: ${color}`)); } else { throw Error('Console color is not defined'); }