@samitouri / QOS-React-2 / commits / 63927e0843

Fixed another Symbol concatenation issue with DevTools format() util (#21521)

Brian Vaughn committed May 18, 2021 at 08:46 UTC 63927e08438e8a52b41840bd583a07b3ceb2956e
2 files changed +71 -30
packages/react-devtools-shared/src/__tests__/utils-test.js
+31
@@ -11,6 +11,7 @@ import {
11 getDisplayName,
12 getDisplayNameForReactElement,
13 } from 'react-devtools-shared/src/utils';
14 +import {format} from 'react-devtools-shared/src/backend/utils';
15 import {
16 REACT_SUSPENSE_LIST_TYPE as SuspenseList,
17 REACT_STRICT_MODE_TYPE as StrictMode,
@@ -45,6 +46,7 @@ describe('utils', () => {
46 expect(getDisplayName(FauxComponent, 'Fallback')).toEqual('Fallback');
47 });
48 });
49 +
50 describe('getDisplayNameForReactElement', () => {
51 it('should return correct display name for an element with function type', () => {
52 function FauxComponent() {}
@@ -54,29 +56,58 @@ describe('utils', () => {
56 'OverrideDisplayName',
57 );
58 });
59 +
60 it('should return correct display name for an element with a type of StrictMode', () => {
61 const element = createElement(StrictMode);
62 expect(getDisplayNameForReactElement(element)).toEqual('StrictMode');
63 });
64 +
65 it('should return correct display name for an element with a type of SuspenseList', () => {
66 const element = createElement(SuspenseList);
67 expect(getDisplayNameForReactElement(element)).toEqual('SuspenseList');
68 });
69 +
70 it('should return NotImplementedInDevtools for an element with invalid symbol type', () => {
71 const element = createElement(Symbol('foo'));
72 expect(getDisplayNameForReactElement(element)).toEqual(
73 'NotImplementedInDevtools',
74 );
75 });
76 +
77 it('should return NotImplementedInDevtools for an element with invalid type', () => {
78 const element = createElement(true);
79 expect(getDisplayNameForReactElement(element)).toEqual(
80 'NotImplementedInDevtools',
81 );
82 });
83 +
84 it('should return Element for null type', () => {
85 const element = createElement();
86 expect(getDisplayNameForReactElement(element)).toEqual('Element');
87 });
88 });
89 +
90 + describe('format', () => {
91 + it('should format simple strings', () => {
92 + expect(format('a', 'b', 'c')).toEqual('a b c');
93 + });
94 +
95 + it('should format multiple argument types', () => {
96 + expect(format('abc', 123, true)).toEqual('abc 123 true');
97 + });
98 +
99 + it('should support string substitutions', () => {
100 + expect(format('a %s b %s c', 123, true)).toEqual('a 123 b true c');
101 + });
102 +
103 + it('should gracefully handle Symbol types', () => {
104 + expect(format(Symbol('a'), 'b', Symbol('c'))).toEqual(
105 + 'Symbol(a) b Symbol(c)',
106 + );
107 + });
108 +
109 + it('should gracefully handle Symbol type for the first argument', () => {
110 + expect(format(Symbol('abc'), 123)).toEqual('Symbol(abc) 123');
111 + });
112 + });
113 });
packages/react-devtools-shared/src/backend/utils.js
+40 -30
@@ -163,43 +163,53 @@ export function format(
163 maybeMessage: any,
164 ...inputArgs: $ReadOnlyArray<any>
165 ): string {
166 - if (typeof maybeMessage !== 'string') {
167 - return [maybeMessage, ...inputArgs].join(' ');
168 - }
169 -
170 - const re = /(%?)(%([jds]))/g;
166 const args = inputArgs.slice();
172 - let formatted: string = maybeMessage;
167
174 - if (args.length) {
175 - formatted = formatted.replace(re, (match, escaped, ptn, flag) => {
176 - let arg = args.shift();
177 - switch (flag) {
178 - case 's':
179 - arg += '';
180 - break;
181 - case 'd':
182 - case 'i':
183 - arg = parseInt(arg, 10).toString();
184 - break;
185 - case 'f':
186 - arg = parseFloat(arg).toString();
187 - break;
188 - }
189 - if (!escaped) {
190 - return arg;
191 - }
192 - args.unshift(arg);
193 - return match;
194 - });
168 + // Symbols cannot be concatenated with Strings.
169 + let formatted: string =
170 + typeof maybeMessage === 'symbol'
171 + ? maybeMessage.toString()
172 + : '' + maybeMessage;
173 +
174 + // If the first argument is a string, check for substitutions.
175 + if (typeof maybeMessage === 'string') {
176 + if (args.length) {
177 + const REGEXP = /(%?)(%([jds]))/g;
178 +
179 + formatted = formatted.replace(REGEXP, (match, escaped, ptn, flag) => {
180 + let arg = args.shift();
181 + switch (flag) {
182 + case 's':
183 + arg += '';
184 + break;
185 + case 'd':
186 + case 'i':
187 + arg = parseInt(arg, 10).toString();
188 + break;
189 + case 'f':
190 + arg = parseFloat(arg).toString();
191 + break;
192 + }
193 + if (!escaped) {
194 + return arg;
195 + }
196 + args.unshift(arg);
197 + return match;
198 + });
199 + }
200 }
201
197 - // arguments remain after formatting
202 + // Arguments that remain after formatting.
203 if (args.length) {
199 - formatted += ' ' + args.join(' ');
204 + for (let i = 0; i < args.length; i++) {
205 + const arg = args[i];
206 +
207 + // Symbols cannot be concatenated with Strings.
208 + formatted += ' ' + (typeof arg === 'symbol' ? arg.toString() : arg);
209 + }
210 }
211
202 - // update escaped %% values
212 + // Update escaped %% values.
213 formatted = formatted.replace(/%{2,2}/g, '%');
214
215 return '' + formatted;