Skip to content

Commit

Permalink
Fixed another Symbol concatenation issue with DevTools format() util (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Vaughn authored May 18, 2021
1 parent 1e3383a commit 63927e0
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 30 deletions.
31 changes: 31 additions & 0 deletions packages/react-devtools-shared/src/__tests__/utils-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
getDisplayName,
getDisplayNameForReactElement,
} from 'react-devtools-shared/src/utils';
import {format} from 'react-devtools-shared/src/backend/utils';
import {
REACT_SUSPENSE_LIST_TYPE as SuspenseList,
REACT_STRICT_MODE_TYPE as StrictMode,
Expand Down Expand Up @@ -45,6 +46,7 @@ describe('utils', () => {
expect(getDisplayName(FauxComponent, 'Fallback')).toEqual('Fallback');
});
});

describe('getDisplayNameForReactElement', () => {
it('should return correct display name for an element with function type', () => {
function FauxComponent() {}
Expand All @@ -54,29 +56,58 @@ describe('utils', () => {
'OverrideDisplayName',
);
});

it('should return correct display name for an element with a type of StrictMode', () => {
const element = createElement(StrictMode);
expect(getDisplayNameForReactElement(element)).toEqual('StrictMode');
});

it('should return correct display name for an element with a type of SuspenseList', () => {
const element = createElement(SuspenseList);
expect(getDisplayNameForReactElement(element)).toEqual('SuspenseList');
});

it('should return NotImplementedInDevtools for an element with invalid symbol type', () => {
const element = createElement(Symbol('foo'));
expect(getDisplayNameForReactElement(element)).toEqual(
'NotImplementedInDevtools',
);
});

it('should return NotImplementedInDevtools for an element with invalid type', () => {
const element = createElement(true);
expect(getDisplayNameForReactElement(element)).toEqual(
'NotImplementedInDevtools',
);
});

it('should return Element for null type', () => {
const element = createElement();
expect(getDisplayNameForReactElement(element)).toEqual('Element');
});
});

describe('format', () => {
it('should format simple strings', () => {
expect(format('a', 'b', 'c')).toEqual('a b c');
});

it('should format multiple argument types', () => {
expect(format('abc', 123, true)).toEqual('abc 123 true');
});

it('should support string substitutions', () => {
expect(format('a %s b %s c', 123, true)).toEqual('a 123 b true c');
});

it('should gracefully handle Symbol types', () => {
expect(format(Symbol('a'), 'b', Symbol('c'))).toEqual(
'Symbol(a) b Symbol(c)',
);
});

it('should gracefully handle Symbol type for the first argument', () => {
expect(format(Symbol('abc'), 123)).toEqual('Symbol(abc) 123');
});
});
});
70 changes: 40 additions & 30 deletions packages/react-devtools-shared/src/backend/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,43 +163,53 @@ export function format(
maybeMessage: any,
...inputArgs: $ReadOnlyArray<any>
): string {
if (typeof maybeMessage !== 'string') {
return [maybeMessage, ...inputArgs].join(' ');
}

const re = /(%?)(%([jds]))/g;
const args = inputArgs.slice();
let formatted: string = maybeMessage;

if (args.length) {
formatted = formatted.replace(re, (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;
});
// Symbols cannot be concatenated with Strings.
let formatted: string =
typeof maybeMessage === 'symbol'
? maybeMessage.toString()
: '' + 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;
});
}
}

// arguments remain after formatting
// Arguments that remain after formatting.
if (args.length) {
formatted += ' ' + args.join(' ');
for (let i = 0; i < args.length; i++) {
const arg = args[i];

// Symbols cannot be concatenated with Strings.
formatted += ' ' + (typeof arg === 'symbol' ? arg.toString() : arg);
}
}

// update escaped %% values
// Update escaped %% values.
formatted = formatted.replace(/%{2,2}/g, '%');

return '' + formatted;
Expand Down

0 comments on commit 63927e0

Please sign in to comment.