`formatConsoleArguments` in
`packages/react-devtools-shared/src/backend/utils/formatConsoleArguments.js`
is used by the DevTools backend (via `hook.js`) to inline `console.*`
printf-style substitutions after stripping React's appended component
stack.
For `%s`/`%d`/`%i`/`%f` it consumes the next argument with
`args.splice(argumentsPointer, 1)` and formats the result.
When a format string has more specifiers than arguments, `splice`
returns an
empty array, so `arg` is `undefined` and the specifier is rendered as
text:
`%s` becomes `"undefined"`, and `%d`/`%i`/`%f` become `"NaN"`.
```js
formatConsoleArguments('%s %s', 'the');
// before: ['the undefined']
// after: ['the %s']
```
Browsers and Node's `util.format` leave an unmatched specifier as a
literal
(`console.log('%s %s', 'a')` prints `a %s`; `console.log('%d')` prints
`%d`).
So a message like `console.warn('value: %s')` was shown in DevTools as
`value: undefined` instead of `value: %s`.
This guards each of the `%d`/`%i`, `%f`, and `%s` cases on argument
availability (`argumentsPointer >= args.length`): when nothing is left
to
consume it keeps the specifier text and does not splice, mirroring the
existing trailing-`%` handling added in #36852. An explicitly passed
`undefined`/`null` argument is unchanged and still renders as
`undefined`/`null`, since a value is present at that position (the
`formats nullish values` test still passes).
## How did you test this change?
Added a regression test to the existing `formatConsoleArguments`
describe
block in `packages/react-devtools-shared/src/__tests__/utils-test.js`:
```js
it('keeps specifiers literal when no argument is supplied', () => {
expect(formatConsoleArguments('%s %s', 'the')).toEqual(['the %s']);
expect(formatConsoleArguments('%s %d', 'value')).toEqual(['value %d']);
expect(formatConsoleArguments('%s %i', 'value')).toEqual(['value %i']);
expect(formatConsoleArguments('%s %f', 'value')).toEqual(['value %f']);
});
```
Each assertion fails on `main` (it produces `['the undefined']` and
`['value NaN']`) and passes with the fix.
Commands run locally:
```
yarn test --build --project devtools packages/react-devtools-shared/src/__tests__/utils-test.js
# 51 passed, 51 total
yarn lint packages/react-devtools-shared/src/backend/utils/formatConsoleArguments.js \
packages/react-devtools-shared/src/__tests__/utils-test.js
# Lint passed.
yarn flow dom-node
# No errors!
yarn prettier-check packages/react-devtools-shared/src/backend/utils/formatConsoleArguments.js \
packages/react-devtools-shared/src/__tests__/utils-test.js
# clean
```
Cross-checked the expected output against Node `util.format`:
`util.format('%s %s', 'the')` -> `the %s`; `util.format('%d')` -> `%d`.
`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!
```
## Summary
`formatConsoleArguments` (used by the DevTools backend to inline console
substitutions) walks the format string and inlines `%s`/`%d`/`%i`/`%f`
arguments while leaving `%c`/`%o`/`%O` in place. For each `%` it reads
the **next** character to decide what to do.
When the format string ends with a lone `%` — e.g.
`console.log('Progress 100%', value)` — the character after `%` is
`undefined`, and the `default` branch ran:
```js
template += `%${nextChar}`; // -> "%undefined"
```
So the function emitted the literal text `%undefined`:
```js
formatConsoleArguments('Progress 100%', 'extra');
// before: ['Progress 100%undefined', 'extra']
// after: ['Progress 100%', 'extra']
```
Browsers render a trailing `%` in a console format string as a literal
percent sign, so this PR keeps it as `%` when there is no following
character.
## How did you test this change?
Added a `keeps a trailing percent sign` test to the existing
`formatConsoleArguments` suite in `utils-test.js` (covering both a bare
trailing `%` and one that follows another substitution). Since the
function is pure and import-free, I also verified the patched logic
against every existing case in that suite plus the new ones to confirm
there are no regressions.
## Summary
Mostly changing the casting syntax from `(x: Y)` to `x as Y`, as the old
syntax was deprecated and always causing an error in Flow
## How did you test this change?
`yarn flow-ci`
## Summary
Notable changes:
- Only $FlowFixMe, $FlowExpectedError suppression comments are supported
- All suppression comments need error code
- A lot of new invalid-compare and constant-condition errors suppressed.
These errors reveal potential logical bugs
## How did you test this change?
`yarn flow-ci`
`Error.prepareStackTrace` is non-standard feature and not all JavaScript
runtimes implement the methods that we are using in React DevTools
backend.
This PR adds additional checks for the presence of the methods that we
are using.
Follow up to #34093.
There's an issue where the skipFrames argument isn't part of the cache
key so the other parsers that expect skipping one frame might skip zero
and show the internal `fakeJSXDEV` callsite. Ideally we should include
the skipFrames as part of the cache key but we can also always just skip
one.
We'll need complete parsing of stack traces for both owner stacks and
async debug info so we need to expand the stack parsing capabilities a
bit. This refactors the source location extraction to use some helpers
we can use for other things too.
This is a fork of `ReactFlightStackConfigV8` which also supports
DevTools requirements like checking both `react_stack_bottom_frame` and
`react-stack-bottom-frame` as well as supporting Firefox stacks.
It also supports extracting the first frame of a component stack or the
last frame of an owner stack for the source location.
In RSC and other stacks now we use a lot of `ReactFunctionLocation` type
to represent the location of a function. I.e. the location of the
beginning of the function (the enclosing line/col) that is represented
by the "Source" of the function. This is also what the parent Component
Stacks represents.
As opposed to `ReactCallSite` which is what normal stack traces and
owner stacks represent. I.e. the line/column number of the callsite into
the next function.
We can start sharing more code by using the `ReactFunctionLocation` type
to represent the component source location and it also helps clarify
which ones are function locations and which ones are callsites as we
start adding more stack traces (e.g. for async debug info and owner
stack traces).
`react-stack-bottom-frame` -> `react_stack_bottom_frame`.
This survives `@babel/plugin-transform-function-name`, but now frames
will be displayed as `at Object.react_stack_bottom_frame (...)` in V8.
Checks that were relying on exact function name match were updated to
use either `.indexOf()` or `.includes()`
For backwards compatibility, both React DevTools and Flight Client will
look for both options. I am not so sure about the latter and if React
version is locked.
When we get the source location for "View source for this element" we
should be using the enclosing function of the callsite of the child. So
that we don't just point to some random line within the component.
This is similar to the technique in #33136.
This technique is now really better than the fake throw technique, when
available. So I now favor the owner technique. The only problem it's
only available in DEV and only if it has a child that's owned (and not
filtered).
We could implement this same technique for the error that's thrown in
the fake throwing solution. However, we really shouldn't need that at
all because for client components we should be able to call
`inspect(fn)` at least in Chrome which is even better.
<!--
Thanks for submitting a pull request!
We appreciate you spending the time to work on these changes. Please
provide enough information so that others can review your pull request.
The three fields below are mandatory.
Before submitting a pull request, please make sure the following is
done:
1. Fork [the repository](https://github.com/facebook/react) and create
your branch from `main`.
2. Run `yarn` in the repository root.
3. If you've fixed a bug or added code that should be tested, add tests!
4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch
TestName` is helpful in development.
5. Run `yarn test --prod` to test in the production environment. It
supports the same options as `yarn test`.
6. If you need a debugger, run `yarn test --debug --watch TestName`,
open `chrome://inspect`, and press "Inspect".
7. Format your code with
[prettier](https://github.com/prettier/prettier) (`yarn prettier`).
8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only
check changed files.
9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`).
10. If you haven't already, complete the CLA.
Learn more about contributing:
https://reactjs.org/docs/how-to-contribute.html
-->
## Summary
<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->
When using React Devtools, calling `console.log('%s', null)` in userland
can cause it to throw an error:
```
TypeError: Cannot read properties of null (reading 'toString')
```
## How did you test this change?
Added a unit test.
<!--
Demonstrate the code is solid. Example: The exact commands you ran and
their output, screenshots / videos if the pull request changes the user
interface.
How exactly did you verify that your PR solves the issue you wanted to
solve?
If you leave this empty, your PR will very likely be closed.
-->
See https://github.com/47ng/nuqs/issues/808.
## Summary
We have been getting unhandled `TypeError: Cannot convert object to
primitive value` errors in development that only occur when using
devtools. I tracked it down to `console.error()` calls coming from
Apollo Client where one of the arguments is an object without a
prototype (created with `Object.create(null)`). This causes
`formatConsoleArgumentsToSingleString()` in React's devtools to error as
the function does not defend against `String()` throwing an error.
My attempted fix is to introduce a `safeToString` function (naming
suggestions appreciated) which expects `String()` to throw on certain
object and in that case falls back to returning `[object Object]`, which
is what `String({})` would return.
## How did you test this change?
Added a new unit test.