dillon1000/react
Commit
Browse files [react-devtools] substitute %i and %f in console format strings (#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!
```Changed paths2 files
First-parent comparisonpackages/react-devtools-shared/src/__tests__/utils-test.js ModifiedM packages/react-devtools-shared/src/backend/utils/index.js ModifiedPatch
Files changed
Rendering syntax-highlighted changes…