Skip to content

Commit

Permalink
don't stringify objects for console log second render
Browse files Browse the repository at this point in the history
  • Loading branch information
@lunaruan
lunaruan committed Apr 14, 2022
1 parent ddb1ab1 commit 75c642a
Show file tree
Hide file tree
Showing 4 changed files with 173 additions and 49 deletions.
82 changes: 81 additions & 1 deletion packages/react-devtools-shared/src/__tests__/utils-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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']);
});
});
});
4 changes: 2 additions & 2 deletions packages/react-devtools-shared/src/backend/console.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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');
}
Expand Down
56 changes: 56 additions & 0 deletions packages/react-devtools-shared/src/backend/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<any>,
style?: string,
): $ReadOnlyArray<any> {
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
Expand Down
80 changes: 34 additions & 46 deletions packages/react-devtools-shared/src/hook.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<any>
): 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<any>,
style?: string,
): $ReadOnlyArray<any> {
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;
Expand Down Expand Up @@ -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');
}
Expand Down

0 comments on commit 75c642a

Please sign in to comment.