Skip to content

Commit

Permalink
chore[react-devtools]: improve console arguments formatting before pa…
Browse files Browse the repository at this point in the history
…ssing it to original console (#29873)

Stacked on #29869.

## Summary

When using ANSI escape sequences, we construct a message in the
following way: `console.<method>('\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.
  • Loading branch information
hoxyq authored Jun 17, 2024
1 parent ff6e05a commit 107a2f8
Show file tree
Hide file tree
Showing 5 changed files with 209 additions and 28 deletions.
90 changes: 69 additions & 21 deletions packages/react-devtools-shared/src/__tests__/utils-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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']);

Expand All @@ -186,7 +187,6 @@ describe('utils', () => {
]);
});

// @reactVersion >= 16.0
it('should format string substituions', () => {
expect(
formatWithStyles(['%s %s %s', 'a', 'b', 'c'], 'color: gray'),
Expand All @@ -199,7 +199,6 @@ describe('utils', () => {
);
});

// @reactVersion >= 16.0
it('should support multiple argument types', () => {
const symbol = Symbol('a');
expect(
Expand All @@ -219,7 +218,6 @@ describe('utils', () => {
]);
});

// @reactVersion >= 16.0
it('should properly format escaped string substituions', () => {
expect(formatWithStyles(['%%s'], 'color: gray')).toEqual([
'%c%s',
Expand All @@ -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]]);
Expand Down Expand Up @@ -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(...)',
{},
]);
});
});
});
10 changes: 8 additions & 2 deletions packages/react-devtools-shared/src/backend/console.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
);
}
}
};
Expand Down
11 changes: 9 additions & 2 deletions packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
} from 'react-devtools-shared/src/utils';
import {sessionStorageGetItem} from 'react-devtools-shared/src/storage';
import {
formatConsoleArgumentsToSingleString,
gt,
gte,
parseSourceFromComponentStack,
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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}"`);
}
Expand Down
66 changes: 64 additions & 2 deletions packages/react-devtools-shared/src/backend/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<any>
): $ReadOnlyArray<any> {
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<any>
): string {
Expand Down
60 changes: 59 additions & 1 deletion packages/react-devtools-shared/src/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<any>
): $ReadOnlyArray<any> {
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;

Expand Down Expand Up @@ -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),
);
}
}
};
Expand Down

0 comments on commit 107a2f8

Please sign in to comment.