Skip to content

Commit 00bacde

Browse files
authored
[react-devtools] substitute %i and %f in console format strings (react#36929)
`formatConsoleArgumentsToSingleString` in `packages/react-devtools-shared/src/backend/utils/index.js` inlines `console.*` printf-style substitutions into a single string. That string is used both as the dedup key and as the displayed text for per-component warnings/errors. The `switch` that consumes the captured flag handles `s`, `d`, `i`, and `f`, and the function's own header comment says it "Implements s, d, i and f placeholders". But the substitution regex only captured `[jds]`: ```js const REGEXP = /(%?)(%([jds]))/g; ``` So `%i` and `%f` were never matched. The `case 'i'` and `case 'f'` arms were dead code: the specifier was emitted literally and its argument was never consumed. Worse, because the unmatched specifier does not shift its argument, every following specifier in the same format string then binds to the wrong argument (a cascading off-by-one over the remaining args). `%i` and `%f` are standard console integer/float specifiers (Node `util.format` and browsers both support them), so this affected common log formats. The fix adds `i` and `f` to the regex class so the existing switch arms run: ```js const REGEXP = /(%?)(%([jdisf]))/g; ``` This is a one-character-class change that reconciles the regex with the switch and the header comment. The pre-existing behavior that `%j` is matched but has no `case` (so it falls through unchanged) is intentionally left as-is; it is out of scope for this fix. ## How did you test this change? Added three regression tests to the existing `formatConsoleArgumentsToSingleString` describe block in `packages/react-devtools-shared/src/__tests__/utils-test.js`: - `formatConsoleArgumentsToSingleString('%i', 3.14)` -> `'3'` - `formatConsoleArgumentsToSingleString('%f', 3.5)` -> `'3.5'` - `formatConsoleArgumentsToSingleString('a %i b %s', 7, 'x')` -> `'a 7 b x'` (locks in argument alignment) Commands run locally (experimental devtools bundles): ``` yarn build-for-devtools yarn test --build --project=devtools -r=experimental packages/react-devtools-shared/src/__tests__/utils-test.js ``` Result: `Tests: 53 passed, 53 total`. To confirm the tests actually cover the bug, I reverted the one-character fix back to `[jds]` and reran: the three new tests fail exactly as the bug predicts, e.g. `%i` yields `"%i 3.14"` and `a %i b %s` yields `"a %i b 7 x"` (the `%s` binds to `7` instead of `x`, showing the off-by-one). Restoring the fix makes them pass again. Also green: ``` yarn linc # ESLint on changed files: passed yarn prettier # no files reflagged yarn flow dom-node # No errors! ```
1 parent e71a639 commit 00bacde

2 files changed

Lines changed: 15 additions & 1 deletion

File tree

packages/react-devtools-shared/src/__tests__/utils-test.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,20 @@ describe('utils', () => {
147147
).toEqual('a 123 b true c');
148148
});
149149

150+
it('should support integer substitutions', () => {
151+
expect(formatConsoleArgumentsToSingleString('%i', 3.14)).toEqual('3');
152+
});
153+
154+
it('should support float substitutions', () => {
155+
expect(formatConsoleArgumentsToSingleString('%f', 3.5)).toEqual('3.5');
156+
});
157+
158+
it('should keep argument alignment across mixed substitutions', () => {
159+
expect(formatConsoleArgumentsToSingleString('a %i b %s', 7, 'x')).toEqual(
160+
'a 7 b x',
161+
);
162+
});
163+
150164
it('should gracefully handle Symbol types', () => {
151165
expect(
152166
formatConsoleArgumentsToSingleString(Symbol('a'), 'b', Symbol('c')),

packages/react-devtools-shared/src/backend/utils/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ export function formatConsoleArgumentsToSingleString(
193193
// If the first argument is a string, check for substitutions.
194194
if (typeof maybeMessage === 'string') {
195195
if (args.length) {
196-
const REGEXP = /(%?)(%([jds]))/g;
196+
const REGEXP = /(%?)(%([jdisf]))/g;
197197

198198
// $FlowFixMe[incompatible-type]
199199
formatted = formatted.replace(REGEXP, (match, escaped, ptn, flag) => {

0 commit comments

Comments
 (0)