Commit Graph

736 Commits

Author SHA1 Message Date
Ruslan Lesiutin
d5f3c50f58 chore[DevTools/Tree]: don't pre-select root element and remove unused code (#32015)
In this PR:
1. Removed unused code in `Tree.js`
2. Removed logic for pre-selecting first element in the tree by default.
This is a bit clowny, because it steals focus and resets scroll, when
user attempts to expand / collapse some subtree.
3. Updated comments around
1c381c588a.

To expand on 3-rd point, for someone who might be reading this in the
future:
We can't guarantee focus of RDT browser extension panels, because they
are hosted in an `iframe`. Attempting to fire any events won't have any
result, user action with the corresponding `iframe` is required in order
for this `iframe` to obtain focus.

The only reason why built-in Elements panel in Chrome works correctly is
because it is supported natively somewhere in Chrome / Chrome DevTools.
Also, when you select an element on the application page, Chrome will
make sure that Elements panel opened, which technically guarantees focus
inside DevTools window and Elements panel subview.

As of today, we can't navigate user to third-party extensions panels,
there is no API for this, hence no ability to guarantee focused RDT
panels.
2025-01-09 18:00:30 +00:00
Ruslan Lesiutin
79dcc47191 chore[DevTools/TraceUpdates]: display names by default (#32019)
Feature was added in https://github.com/facebook/react/pull/31577, lets
enable it by default. Note: for gradual rollout with React Native, we
will continue to emit different event, requires some changes on React
Native side to support this.

I have plans to make this feature to be accessible via browser context
menu, which has really limited API. In order to minimize potential
divergence, lets make this the default state for the feature.
2025-01-09 18:00:09 +00:00
Sebastian Markbåge
c4595ca4c8 Add ViewTransitionComponent to Stacks and DevTools (#32034)
Just adding the name so it shows up.

(Note that no experimental ones are added to the list of filters atm.
Including SuspenseList etc.)
2025-01-09 11:33:34 -05:00
Sebastian Markbåge
a4d122f2d1 Add <ViewTransition> Component (#31975)
This will provide the opt-in for using [View
Transitions](https://developer.mozilla.org/en-US/docs/Web/API/View_Transition_API)
in React.

View Transitions only trigger for async updates like `startTransition`,
`useDeferredValue`, Actions or `<Suspense>` revealing from fallback to
content. Synchronous updates provide an opt-out but also guarantee that
they commit immediately which View Transitions can't.

There's no need to opt-in to View Transitions at the "cause" side like
event handlers or actions. They don't know what UI will change and
whether that has an animated transition described.

Conceptually the `<ViewTransition>` component is like a DOM fragment
that transitions its children in its own isolate/snapshot. The API works
by wrapping a DOM node or inner component:

```js
import {ViewTransition} from 'react';

<ViewTransition><Component /></ViewTransition>
```

The default is `name="auto"` which will automatically assign a
`view-transition-name` to the inner DOM node. That way you can add a
View Transition to a Component without controlling its DOM nodes styling
otherwise.

A difference between this and the browser's built-in
`view-transition-name: auto` is that switching the DOM nodes within the
`<ViewTransition>` component preserves the same name so this example
cross-fades between the DOM nodes instead of causing an exit and enter:

```js
<ViewTransition>{condition ? <ComponentA /> : <ComponentB />}</ViewTransition>
```

This becomes especially useful with `<Suspense>` as this example
cross-fades between Skeleton and Content:

```js
<ViewTransition>
  <Suspense fallback={<Skeleton />}>
    <Content />
  </Suspense>
</ViewTransition>
```

Where as this example triggers an exit of the Skeleton and an enter of
the Content:

```js
<Suspense fallback={<ViewTransition><Skeleton /></ViewTransition>}>
  <ViewTransition><Content /></ViewTransition>
</Suspense>
```

Managing instances and keys becomes extra important.

You can also specify an explicit `name` property for example for
animating the same conceptual item from one page onto another. However,
best practices is to property namespace these since they can easily
collide. It's also useful to add an `id` to it if available.

```js
<ViewTransition name="my-shared-view">
```

The model in general is the same as plain `view-transition-name` except
React manages a set of heuristics for when to apply it. A problem with
the naive View Transitions model is that it overly opts in every
boundary that *might* transition into transitioning. This is leads to
unfortunate effects like things floating around when unrelated updates
happen. This leads the whole document to animate which means that
nothing is clickable in the meantime. It makes it not useful for smaller
and more local transitions. Best practice is to add
`view-transition-name` only right before you're about to need to animate
the thing. This is tricky to manage globally on complex apps and is not
compositional. Instead we let React manage when a `<ViewTransition>`
"activates" and add/remove the `view-transition-name`. This is also when
React calls `startViewTransition` behind the scenes while it mutates the
DOM.

I've come up with a number of heuristics that I think will make a lot
easier to coordinate this. The principle is that only if something that
updates that particular boundary do we activate it. I hope that one day
maybe browsers will have something like these built-in and we can remove
our implementation.

A `<ViewTransition>` only activates if:

- If a mounted Component renders a `<ViewTransition>` within it outside
the first DOM node, and it is within the viewport, then that
ViewTransition activates as an "enter" animation. This avoids inner
"enter" animations trigger when the parent mounts.
- If an unmounted Component had a `<ViewTransition>` within it outside
the first DOM node, and it was within the viewport, then that
ViewTransition activates as an "exit" animation. This avoids inner
"exit" animations triggering when the parent unmounts.
- If an explicitly named `<ViewTransition name="...">` is deep within an
unmounted tree and one with the same name appears in a mounted tree at
the same time, then both are activated as a pair, but only if they're
both in the viewport. This avoids these triggering "enter" or "exit"
animations when going between parents that don't have a pair.
- If an already mounted `<ViewTransition>` is visible and a DOM
mutation, that might affect how it's painted, happens within its
children but outside any nested `<ViewTransition>`. This allows it to
"cross-fade" between its updates.
- If an already mounted `<ViewTransition>` resizes or moves as the
result of direct DOM nodes siblings changing or moving around. This
allows insertion, deletion and reorders into a list to animate all
children. It is only within one DOM node though, to avoid unrelated
changes in the parent to trigger this. If an item is outside the
viewport before and after, then it's skipped to avoid things flying
across the screen.
- If a `<ViewTransition>` boundary changes size, due to a DOM mutation
within it, then the parent activates (or the root document if there are
no more parents). This ensures that the container can cross-fade to
avoid abrupt relayout. This can be avoided by using absolutely
positioned children. When this can avoid bubbling to the root document,
whatever is not animating is still responsive to clicks during the
transition.

Conceptually each DOM node has its own default that activates the parent
`<ViewTransition>` or no transition if the parent is the root. That
means that if you add a DOM node like `<div><ViewTransition><Component
/></ViewTransition></div>` this won't trigger an "enter" animation since
it was the div that was added, not the ViewTransition. Instead, it might
cause a cross-fade of the parent ViewTransition or no transition if it
had no parent. This ensures that only explicit boundaries perform coarse
animations instead of every single node which is really the benefit of
the View Transitions model. This ends up working out well for simple
cases like switching between two pages immediately while transitioning
one floating item that appears on both pages. Because only the floating
item transitions by default.

Note that it's possible to add manual `view-transition-name` with CSS or
`style={{ viewTransitionName: 'auto' }}` that always transitions as long
as something else has a `<ViewTransition>` that activates. For example a
`<ViewTransition>` can wrap a whole page for a cross-fade but inside of
it an explicit name can be added to something to ensure it animates as a
move when something relates else changes its layout. Instead of just
cross-fading it along with the Page which would be the default.

There's more PRs coming with some optimizations, fixes and expanded
APIs. This first PR explores the above core heuristic.

---------

Co-authored-by: Sebastian "Sebbie" Silbermann <silbermann.sebastian@gmail.com>
2025-01-08 12:11:18 -05:00
Ruslan Lesiutin
62208bee5a DevTools: fork FastRefresh test for <18 versions of React (#31893)
We currently have a failing test for React DevTools against React 17.
This started failing in https://github.com/facebook/react/pull/30899,
where we changed logic for error tracking and started relying on
`onPostCommitFiberRoot` hook.

Looking at https://github.com/facebook/react/pull/21183,
`onPostCommitFiberRoot` was shipped in 18, which means that any console
errors / warnings emitted in passive effects won't be recorded by React
DevTools for React < 18.
2025-01-02 14:07:21 +00:00
Piotr Tomczewski
a7b829524b [DevTools] Show component names while highlighting renders (#31577)
## Summary
This PR improves the Trace Updates feature by letting developers see
component names directly on the update overlay. Before this change, the
overlay only highlighted updated regions, leaving it unclear which
components were involved. With this update, you can now match visual
updates to their corresponding components, making it much easier to
debug rendering performance.

### New Feature: Show component names while highlighting
When the new **"Show component names while highlighting"** setting is
enabled, the update overlay display the names of affected components
above the rectangles, along with the update count. This gives immediate
context about what’s rendering and why. The preference is stored in
local storage and synced with the backend, so it’s remembered across
sessions.

### Improvements to Drawing Logic
The drawing logic has been updated to make the overlay sharper and
easier to read. Overlay now respect device pixel ratios, so they look
great on high-DPI screens. Outlines have also been made crisper, which
makes it easier to spot exactly where updates are happening.

> [!NOTE]
> **Grouping Logic and Limitations**
> Updates are grouped by their screen position `(left, top coordinates)`
to combine overlapping or nearby regions into a single group. Groups are
sorted by the highest update count within each group, making the most
frequently updated components stand out.
> Overlapping labels may still occur when multiple updates involve
components that overlap but are not in the exact same position. This is
intentional, as the logic aims to maintain a straightforward mapping
between update regions and component names without introducing
unnecessary complexity.

### Testing
This PR also adds tests for the new `groupAndSortNodes` utility, which
handles the logic for grouping and sorting updates. The tests ensure the
behavior is reliable across different scenarios.

## Before & After


https://github.com/user-attachments/assets/6ea0fe3e-9354-44fa-95f3-9a867554f74c


https://github.com/user-attachments/assets/32af4d98-92a5-47dd-a732-f05c2293e41b

---------

Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>
2024-12-13 11:53:05 +00:00
Henry Q. Dineen
6e29479bff [devtools] allow non-coercible objects in formatConsoleArgumentsToSingleString (#31444)
## 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.
2024-11-10 19:24:15 +00:00
Jan Kassens
07aa494432 Remove enableRefAsProp feature flag (#30346)
The flag is fully rolled out.
2024-11-04 14:30:58 -05:00
Sam Zhou
45804af18d [flow] Eliminate usage of more than 1-arg React.AbstractComponent in React codebase (#31314)
<!--
  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

In order to adopt react 19's ref-as-prop model, Flow needs to eliminate
all the places where they are treated differently.
`React.AbstractComponent` is the worst example of this, and we need to
eliminate it.

This PR eliminates them from the react repo, and only keeps the one that
has 1 argument of props.

## How did you test this change?

yarn flow
2024-10-21 16:17:41 -07:00
Sebastian Markbåge
b8ae38f88b Audit try/finally around console patching (#31286)
Otherwise if something errors they can be left patched.

[Review without
whitespace](https://github.com/facebook/react/pull/31286/files?w=1)
2024-10-18 12:05:20 -04:00
Ruslan Lesiutin
bf7e210cb5 tests[react-devtools]: added tests for Compiler integration (#31241)
Adds tests for Compiler integration.

This includes:
- Tests against Compiler from source.
- Versioned (18.2 - <19) tests against Compiler from npm.

For tests against React 18.2, I had to download `react-compiler-runtime`
from npm and put it to `react/compiler-runtime.js`.
2024-10-17 09:02:41 +01:00
lauren
9806a4b0d4 [DevTools] Fix React Compiler badging (#31196)
In #31140 we switched over the uMC polyfill to use memo instead of state
since memo would FastRefresh properly. However this busted devtools'
badging of compiled components; this PR fixes it.

TODO: tests
Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>

---------

Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>
2024-10-15 12:53:45 +01:00
Ruslan Lesiutin
ec2bf02245 fix[react-devtools]: fixed timeline profiler tests (#31261)
Fixes tests against React 18 after
https://github.com/facebook/react/pull/31154:
- Set `supportsTimeline` to true for `Store`.
- Execute `store.profilerStore.startProfiling` after `legacyRender`
import, because this is where `react-dom` is imported and renderer is
registered. We don't yet propagate `isProfiling` flag to newly
registered renderers, when profiling already started see:

d5bba18b5d/packages/react-devtools-shared/src/hook.js (L203-L204)
2024-10-15 12:46:05 +01:00
Ruslan Lesiutin
d5bba18b5d fix[react-devtools]: record timeline data only when supported (#31154)
Stacked on https://github.com/facebook/react/pull/31132. See last
commit.

There are 2 issues:
1. We've been recording timeline events, even if Timeline Profiler was
not supported by the Host. We've been doing this for React Native, for
example, which would significantly regress perf of recording a profiling
session, but we were not even using this data.
2. Currently, we are generating component stack for every state update
event. This is extremely expensive, and we should not be doing this.

We can't currently fix the second one, because we would still need to
generate all these stacks, and this would still take quite a lot of
time. As of right now, we can't generate a component stack lazily
without relying on the fact that reference to the Fiber is not stale.
With `enableOwnerStacks` we could populate component stacks in some
collection, which would be cached at the Backend, and then returned only
once Frontend asks for it. This approach also eliminates the need for
keeping a reference to a Fiber.
2024-10-09 15:27:04 +01:00
Ruslan Lesiutin
bfe91fbecf refactor[react-devtools]: flatten reload and profile config (#31132)
Stacked on https://github.com/facebook/react/pull/31131. See last
commit.

This is a clean-up and a pre-requisite for next changes:
1. `ReloadAndProfileConfig` is now split into boolean value and settings
object. This is mainly because I will add one more setting soon, and
also because settings might be persisted for a longer time than the flag
which signals if the Backend was reloaded for profiling. Ideally, this
settings should probably be moved to the global Hook object, same as we
did for console patching.
2. Host is now responsible for reseting the cached values, Backend will
execute provided `onReloadAndProfileFlagsReset` callback.
2024-10-09 13:57:02 +01:00
Ruslan Lesiutin
1d8d12005f fix[react-devtools]: remove all listeners when Agent is shutdown (#31151)
Based on https://github.com/facebook/react/pull/31049, credits to
@EdmondChuiHW.

What is happening here:
1. Once Agent is destroyed, unsubscribe own listeners and bridge
listeners.
2. [Browser extension only] Once Agent is destroyed, unsubscribe
listeners from BackendManager.
3. [Browser extension only] I've discovered that `backendManager.js`
content script can get injected multiple times by the browser. When
Frontend is initializing, it will create Store first, and then execute a
content script for bootstraping backend manager. If Frontend was
destroyed somewhere between these 2 steps, Backend won't be notified,
because it is not initialized yet, so it will not unsubscribe listeners
correctly. We might end up duplicating listeners, and the next time
Frontend is launched, it will report an issues "Cannot add / remove node
...", because same operations are emitted twice.

To reproduce 3 you can do the following:
1. Click reload-to-profile
2. Right after when both app and Chrome DevTools panel are reloaded,
close Chrome DevTools.
3. Open Chrome DevTools again, open Profiler panel and observe "Cannot
add / remove node ..." error in the UI.
2024-10-09 13:34:01 +01:00
Ruslan Lesiutin
4a86ec5a66 fix[react-devtools]: removed redundant startProfiling call (#31131)
Stacked on https://github.com/facebook/react/pull/31118. See last
commit.

We don't need to call `startProfiling()` here, because we delegate this
to the Renderer itself:

830e823cd2/packages/react-devtools-shared/src/backend/fiber/renderer.js (L5227-L5232)

Since this is de-facto the constructor of Renderer, this will be called
earlier.

Validated via testing the reload-to-profile for Chrome browser
extension.
2024-10-09 13:32:04 +01:00
Ruslan Lesiutin
389a2deebc refactor[react-devtools/fiber/renderer]: optimize durations resolution (#31118)
Stacked on https://github.com/facebook/react/pull/31117. 

No need for sending long float numbers and to have resolution less than
a microsecond, we end up formatting it on a Frontend side:

6c7b41da3d/packages/react-devtools-shared/src/devtools/views/Profiler/utils.js (L359-L360)
2024-10-09 13:26:16 +01:00
Ruslan Lesiutin
dbf80c8d7a fix[react-devtools]: update profiling status before receiving response from backend (#31117)
We can't wait for a response from Backend, because it might take some
time to actually finish profiling.

We should keep a flag on the frontend side, so user can quickly see the
feedback in the UI.
2024-10-09 13:23:23 +01:00
Ruslan Lesiutin
bf0c054649 fix[react-devtools]: wrap key string in preformatted text html element (#31153)
Fixes https://github.com/facebook/react/issues/28984.
2024-10-09 09:54:34 +01:00
Ruslan Lesiutin
6e612587ec chore[react-devtools]: drop legacy context tests (#31059)
We've dropped the support for detecting changes in legacy Contexts in
https://github.com/facebook/react/pull/30896.
2024-10-01 14:26:12 +01:00
Ruslan Lesiutin
9ea5ffa9cb chore[react-devtools]: add legacy mode error message to the ignore list for tests (#31060)
Without this, the console gets spammy whenever we run React DevTools
tests against React 18.x, where this deprecation message was added.
2024-10-01 14:04:05 +01:00
Sebastian Markbåge
654e387d7e [Flight] Serialize Server Components Props in DEV (#31105)
This allows us to show props in React DevTools when inspecting a Server
Component.

I currently drastically limit the object depth that's serialized since
this is very implicit and you can have heavy objects on the server.

We previously was using the general outlineModel to outline
ReactComponentInfo but we weren't consistently using it everywhere which
could cause some bugs with the parsing when it got deduped on the
client. It also lead to the weird feature detect of `isReactComponent`.
It also meant that this serialization was using the plain serialization
instead of `renderConsoleValue` which means we couldn't safely serialize
arbitrary debug info that isn't serializable there.

So the main change here is to call `outlineComponentInfo` and have that
always write every "Server Component" instance as outlined and in a way
that lets its props be serialized using `renderConsoleValue`.

<img width="1150" alt="Screenshot 2024-10-01 at 1 25 05 AM"
src="https://github.com/user-attachments/assets/f6e7811d-51a3-46b9-bbe0-1b8276849ed4">
2024-10-01 01:39:20 -04:00
Edmond Chui
204a551eae Add: reload to profile for Fusebox (#31021)
## Summary

Add reload to profile for Fusebox 

Stacked on #31048. See
6be1977112

## How did you test this change?

Test E2E in [D63233256](https://www.internalfb.com/diff/D63233256)
2024-09-26 16:39:51 +01:00
Edmond Chui
f8024b0686 refactor: allow custom impl of backend realod-to-profile support check (#31048)
<!--
  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

In preparation to support reload-to-profile in Fusebox (#31021), we need
a way to check capability of different backends, e.g. web vs React
Native.

## How did you test this change?

<!--
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.
-->

* Default, e.g. existing web impl = no-op
* Custom impl: is called
2024-09-26 12:39:28 +01:00
Ruslan Lesiutin
d66fa02a30 fix: use public instance in Fiber renderer and expose it from getInspectorDataForViewAtPoint (#31068)
React DevTools no longer operates with just Fibers, it now builds its
own Shadow Tree, which represents the tree on the Host (Fabric on
Native, DOM on Web).

We have to keep track of public instances for a select-to-inspect
feature. We've recently changed this logic in
https://github.com/facebook/react/pull/30831, and looks like we've been
incorrectly getting a public instance for Fabric case.

Not only this, turns out that all `getInspectorData...` APIs are
returning Fibers, and not public instances. I have to expose it, so that
React DevTools can correctly identify the element, which was selected.

Changes for React Native are in
[D63421463](https://www.internalfb.com/diff/D63421463)
2024-09-26 10:17:16 +01:00
Ruslan Lesiutin
a15bbe1475 refactor: data source for errors and warnings tracking is now in Store (#31010)
Stacked on https://github.com/facebook/react/pull/31009.

1. Instead of keeping `showInlineWarningsAndErrors` in `Settings`
context (which was removed in
https://github.com/facebook/react/pull/30610), `Store` will now have a
boolean flag, which controls if the UI should be displaying information
about errors and warnings.
2. The errors and warnings counters in the Tree view are now counting
only unique errors. This makes more sense, because it is part of the
Elements Tree view, so ideally it should be showing number of components
with errors and number of components of warnings. Consider this example:
2.1. Warning for element `A` was emitted once and warning for element
`B` was emitted twice.
2.2. With previous implementation, we would show `3 ⚠️`, because in
total there were 3 warnings in total. If user tries to iterate through
these, it will only take 2 steps to do the full cycle, because there are
only 2 elements with warnings (with one having same warning, which was
emitted twice).
2.3 With current implementation, we would show `2 ⚠️`. Inspecting the
element with doubled warning will still show the warning counter (2)
before the warning message.

With these changes, the feature correctly works.
https://fburl.com/a7fw92m4
2024-09-24 19:51:21 +01:00
Ruslan Lesiutin
fc4a33eaa9 fix: consider alternate as a key for componentLogsEntry when inspecting raw fiber instance (#31009)
Related - https://github.com/facebook/react/pull/30899.

Looks like this was missed. We actually do this when we record errors
and warnings before sending them via Bridge:

e4953922a9/packages/react-devtools-shared/src/backend/fiber/renderer.js (L2169-L2173)

So, what is happening in the end, errors or warnings are displayed in
the Tree, but when user clicks on the component, nothing is shown,
because `fiberToComponentLogsMap` has only `alternate` as a key.
2024-09-24 17:49:19 +01:00
Edmond Chui
5d19e1c8d1 Fix: profiling crashes #30661 #28838 (#31024)
<!--
  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

Profiling fails sometimes because `onProfilingStatus` is called
repeatedly on some occasions, e.g. multiple calls to
`getProfilingStatus`.

Subsequent calls should be a no-op if the profiling status hasn't
changed.

Reported via #30661 #28838.

> [!TIP]
> Hide whitespace changes on this PR

<img width="328" alt="screenshot showing the UI controls for hiding
whitespace changes on GitHub"
src="https://github.com/user-attachments/assets/036385cf-2610-4e69-a717-17c05d7ef047">


## How did you test this change?

<!--
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.
-->

Tested as part of Fusebox implementation of reload-to-profile.

https://github.com/facebook/react/pull/31021?#discussion_r1770589753
2024-09-23 13:25:44 +01:00
Ruslan Lesiutin
e740d4b14b chore: remove using local storage for persisting console settings on the frontend (#31002)
After https://github.com/facebook/react/pull/30636 and
https://github.com/facebook/react/pull/30986 we no longer store settings
on the Frontend side via `localStorage`.

This PR removes all occurrences of it from
`react-devtools-core/standalone` and `react-devtools-inline`.
2024-09-19 15:47:25 +01:00
Ruslan Lesiutin
f2c57a31e9 chore: remove settings manager from react-devtools-core (#30986)
Stacked on https://github.com/facebook/react/pull/30636. See [this
commit](20cec76c44).

This has been only used for React Native and will be replaced by another
approach (initialization via `installHook` call) in the next PR.
2024-09-18 18:30:32 +01:00
Ruslan Lesiutin
f37c7bc653 feat[react-devtools/extension]: use chrome.storage to persist settings across sessions (#30636)
Stacked on https://github.com/facebook/react/pull/30610 and whats under
it. See [last
commit](248ddba186).

Now, we are using
[`chrome.storage`](https://developer.chrome.com/docs/extensions/reference/api/storage)
to persist settings for the browser extension across different sessions.
Once settings are updated from the UI, the `Store` will emit
`settingsUpdated` event, and we are going to persist them via
`chrome.storage.local.set` in `main/index.js`.

When hook is being injected, we are going to pass a `Promise`, which is
going to be resolved after the settings are read from the storage via
`chrome.storage.local.get` in `hookSettingsInjector.js`.
2024-09-18 18:26:39 +01:00
Ruslan Lesiutin
e33acfd67f refactor[react-devtools]: propagate settings from global hook object to frontend (#30610)
Stacked on https://github.com/facebook/react/pull/30597 and whats under
it. See [this
commit](59b4efa723).

With this change, the initial values for console patching settings are
propagated from hook (which is the source of truth now, because of
https://github.com/facebook/react/pull/30596) to the UI. Instead of
reading from `localStorage` the frontend is now requesting it from the
hook. This happens when settings modal is rendered, and wrapped in a
transition. Also, this is happening even if settings modal is not opened
yet, so we have enough time to fetch this data without displaying loader
or similar UI.
2024-09-18 18:19:01 +01:00
Ruslan Lesiutin
fce4606657 chore[react-devtools]: extract some utils into separate modules to unify implementations (#30597)
Stacked on https://github.com/facebook/react/pull/30596. See [this
commit](4ba5e784bb).

Moving `formatWithStyles` and `formatConsoleArguments` to its own
modules, so that we can finally have a single implementation for these
and stop inlining them in RDT global hook object.
2024-09-18 18:16:20 +01:00
Ruslan Lesiutin
3cac0875dc refactor[react-devtools]: move console patching to global hook (#30596)
Stacked on https://github.com/facebook/react/pull/30566 and whats under
it. See [this
commit](374fd737e4).

It is mostly copying code from one place to another and updating tests.
With these changes, for every console method that we patch, there is
going to be a single applied patch:
- For `error`, `warn`, and `trace` we are patching when hook is
installed. This guarantees that component stacks are going to be
appended even if browser DevTools are not opened. We pay some price for
it, though: if user has browser DevTools closed and if at this point
some warning or error is emitted (logged), the next time user opens
browser DevTools, they are going to see `hook.js` as the source frame.
Unfortunately, ignore listing from source maps is not applied
retroactively, and I don't know if its a bug or just a design
limitations. Once browser DevTools are opened, source maps will be
loaded and ignore listing will be applied for all emitted logs in the
future.
- For `log`, `info`, `group`, `groupCollapsed` we are only patching when
React notifies React DevTools about running in StrictMode. We unpatch
the methods right after it.
2024-09-18 18:12:18 +01:00
Ruslan Lesiutin
b521ef8a2a refactor[react-devtools]: remove browserTheme from ConsolePatchSettings (#30566)
Stacked on https://github.com/facebook/react/pull/30564.

We are no longer using browser theme in our console patching, this was
removed in unification of console patching for strict mode, we started
using ansi escape symbols and forking based on browser theme is no
longer required - https://github.com/facebook/react/pull/29869

The real browser theme initialization for frontend is happening at the
other place and is not affected:

40be968257/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js (L117-L120)
2024-09-18 18:02:13 +01:00
Ruslan Lesiutin
5e83d9ab3b feat[react-devtools]: add settings to global hook object (#30564)
Right now we are patching console 2 times: when hook is installed
(before page is loaded) and when backend is connected. Because of this,
even if user had `appendComponentStack` setting enabled, all emitted
error and warning logs are not going to have component stacks appended.
They also won't have component stacks appended retroactively when user
opens browser DevTools (this is when frontend is initialized and
connects to backend).

This behavior adds potential race conditions with LogBox in React
Native, and also unpredictable to the user, because in order to get
component stacks logged you have to open browser DevTools, but by the
time you do it, error or warning log was already emitted.

To solve this, we are going to only patch console in the hook object,
because it is guaranteed to load even before React. Settings are going
to be synchronized with the hook via Bridge, and React DevTools Backend
Host (React Native or browser extension shell) will be responsible for
persisting these settings across the session, this is going to be
implemented in a separate PR.
2024-09-18 17:37:00 +01:00
Ruslan Lesiutin
9f4e4611ea fix: add Error prefix to Error objects names (#30969)
This fixes printing Error objects in Chrome DevTools.

I've observed that Chrome DevTools is not source mapping and linkifying
URLs, when was running this on larger apps. Chrome DevTools talks to V8
via Chrome DevTools protocol, every object has a corresponding
[`RemoteObject`](https://chromedevtools.github.io/devtools-protocol/tot/Runtime/#type-RemoteObject).

When Chrome DevTools sees that Error object is printed in the console,
it will try to prettify it. `description` field of the corresponding
`RemoteObject` for the `Error` JavaScript object is a combination of
`Error` `name`, `message`, `stack` fields. This is not just a raw
`stack` field, so our prefix for this field just doesn't work. [V8 is
actually filtering out first line of the `stack` field, it only keeps
the stack frames as a string, and then this gets prefixed by `name` and
`message` fields, if they are
available](https://source.chromium.org/chromium/chromium/src/+/main:v8/src/inspector/value-mirror.cc;l=252-311;drc=bdc48d1b1312cc40c00282efb1c9c5f41dcdca9a?fbclid=IwZXh0bgNhZW0CMTEAAR1tMm5YC4jqowObad1qXFT98X4RO76CMkCGNSxZ8rVsg6k2RrdvkVFL0i4_aem_e2fRrqotKdkYIeWlJnk0RA).
As an illustration, this:
```
const fakeError = new Error('');
fakeError.name = 'Stack';
fakeError.stack = 'Error Stack:' + stack;
```

will be formatted by `V8` as this `RemoteObject`:
```
{
  ...
  description: 'Stack: ...',
  ...
}
```

Notice that there is no `Error` prefix, that was previously added.
Because of this, [Chrome DevTools won't even try to symbolicate the
stack](ee4729d2cc/front_end/panels/console/ErrorStackParser.ts (L33-L35)),
because it doesn't have such prefix.
2024-09-16 17:43:40 +01:00
Sebastian Markbåge
0eab377a96 Add enableComponentPerformanceTrack Flag (#30960)
This flag will be used to gate a new timeline profiler that's integrate
with the Performance Tab and the new performance.measure extensions in
Chrome.

It replaces the existing DevTools feature so this disables
enableSchedulingProfiler when it is enabled since they can interplay in
weird ways potentially.

This means that experimental React now disable scheduling profiler and
enables this new approach.
2024-09-16 11:09:40 -04:00
Ruslan Lesiutin
8cf64620c7 fix[rdt/fiber/renderer.js]: getCurrentFiber can be injected as null (#30968)
In production artifacts for `18.x.x` `getCurrentFiber` can actually be
injected as `null`. Updated `getComponentStack` and `onErrorOrWarning`
implementations to support this.

![Screenshot 2024-09-16 at 10 52
00](https://github.com/user-attachments/assets/a0c773aa-ebbf-4fd5-95c4-cac3cc0c203f)
2024-09-16 14:47:57 +01:00
Josh Story
94e652d505 disable enableSiblingPrerendering in experimental channel (#30952)
Disables `enableSiblingPrerendering` in the experimental builds until
the feature is tested at Meta first.
2024-09-12 09:33:20 -07:00
Ruslan Lesiutin
bb6b86ed59 refactor[react-devtools]: initialize renderer interface early (#30946)
The current state is that `rendererInterface`, which contains all the
backend logic, like generating component stack or attaching errors to
fibers, or traversing the Fiber tree, ..., is only mounted after the
Frontend is created.

For browser extension, this means that we don't patch console or track
errors and warnings before Chrome DevTools is opened.

With these changes, `rendererInterface` is created right after
`renderer` is injected from React via global hook object (e. g.
`__REACT_DEVTOOLS_GLOBAL_HOOK__.inject(...)`.

Because of the current implementation, in case of multiple Reacts on the
page, all of them will patch the console independently. This will be
fixed in one of the next PRs, where I am moving console patching to the
global Hook.

This change of course makes `hook.js` script bigger, but I think it is a
reasonable trade-off for better DevX. We later can add more heuristics
to optimize the performance (if necessary) of `rendererInterface` for
cases when Frontend was connected late and Backend is attempting to
flush out too many recorded operations.

This essentially reverts https://github.com/facebook/react/pull/26563.
2024-09-12 13:59:29 +01:00
Andrew Clark
d6cb4e7713 Start prerendering Suspense retries immediately (#30934)
When a component suspends and is replaced by a fallback, we should start
prerendering the fallback immediately, even before any new data is
received. During the retry, we can enter prerender mode directly if
we're sure that no new data was received since we last attempted to
render the boundary.

To do this, when completing the fallback, we leave behind a pending
retry lane on the Suspense boundary. Previously we only did this once a
promise resolved, but by assigning a lane during the complete phase, we
will know that there's speculative work to be done.

Then, upon committing the fallback, we mark the retry lane as suspended
— but only if nothing was pinged or updated in the meantime. That allows
us to immediately enter prerender mode (i.e. render without skipping any
siblings) when performing the retry.
2024-09-11 11:41:54 -04:00
Ruslan Lesiutin
344bc8128b refactor[Agent/Store]: Store to send messages only after Agent is initialized (#30945)
Both for browser extension, and for React Native (as part of
`react-devtools-core`) `Store` is initialized before the Backend (and
`Agent` as a part of it):

bac33d1f82/packages/react-devtools-extensions/src/main/index.js (L111-L113)

Any messages that we send from `Store`'s constructor are ignored,
because there is nothing on the other end yet. With these changes,
`Agent` will send `backendInitialized` message to `Store`, after which
`getBackendVersion` and other events will be sent.

Note that `isBackendStorageAPISupported` and `isSynchronousXHRSupported`
are still sent from `Agent`'s constructor, because we don't explicitly
ask for it from `Store`, but these are used.

This the pre-requisite for fetching settings and unsupported renderers
reliably from the Frontend.
2024-09-11 15:10:13 +01:00
Ruslan Lesiutin
3dfd5d9efb refactor[RendererInterface]: expose onErrorOrWarning and getComponentStack (#30931)
Make `onErrorOrWarning` and `getComponentStack` part of
`rendererInterface`. By doing this, they will be available from the
global hook `rendererInterfaces` Map. This makes them available to be
used by Hook, which soon will be the only one who is doing console
patching.

This is also a pre-requisite for removing `registerRenderer`:

d160aa0fbb/packages/react-devtools-shared/src/backend/console.js (L113-L121)
2024-09-10 15:59:40 +01:00
Sebastian Markbåge
63cefa2957 [DevTools] Implement getComponentStack and onErrorOrWarning for replayed Flight logs (#30930)
This adds owner stacks to replayed Server Component logs in environments
that don't support native console.createTask.

<img width="521" alt="Screenshot 2024-09-09 at 8 55 21 PM"
src="https://github.com/user-attachments/assets/261cfaee-ea65-4044-abf0-c41abf358fea">

It also tracks the logs in the global componentInfoToComponentLogsMap
which lets us associate those logs with Server Components when they
later commit into the fiber tree.

<img width="1280" alt="Screenshot 2024-09-09 at 9 31 16 PM"
src="https://github.com/user-attachments/assets/436312a6-f9f4-4add-8129-0fb9b9eb18ee">

I tried to create unit tests for this since it's now wired up
end-to-end. Unfortunately, the complicated testing set up for Flight
requires a complex set of resetting modules which are incompatible with
the complicated test setup in getVersionedRenderImplementation for
DevTools tests.
2024-09-10 10:37:29 -04:00
Sebastian Markbåge
d160aa0fbb [DevTools] Use Unicode Atom Symbol instead of Atom Emoji (#30832)
This reverts #19603.

Before:
<img width="724" alt="Screenshot 2024-08-28 at 12 07 29 AM"
src="https://github.com/user-attachments/assets/0613088f-c013-4f1c-92c3-fbdae8c1f109">

After:
<img width="771" alt="Screenshot 2024-08-28 at 12 08 13 AM"
src="https://github.com/user-attachments/assets/eef21bee-d11f-4f0a-9147-053a163f720f">

Consensus seems to be that while the purple on is a bit clearer and
easier to read. The purple is not on brand so it doesn't look like
React. It looks ugly. It's distracting (too eye catching). Taking away
attention from other tabs in an unfair way.

It also gets worse with more tabs added. We plan on both adding another
tab and panes inside other tabs (elements/sources) soon. Each needs to
be marked somehow as part of React but spelling it out is too long.
Putting inside a second tab means two clicks and takes away real-estate
from our extension and doesn't solve the problem with extension panes in
other tabs. We also plan on adding multiple different tracks to the
Performance tab which also needs a name other than just React and
spelling out React as a prefix is too long. The Emoji is too
distracting. So it seems best to uniformly apply the symbol - albeit it
might just look like a dot to many.

Dark mode looks close to on brand:

<img width="1089" alt="Screenshot 2024-08-28 at 12 32 50 AM"
src="https://github.com/user-attachments/assets/7175a540-4241-4c26-9e4d-4d367873af57">
2024-09-10 00:09:42 -04:00
Sebastian Markbåge
0dbacf2041 [DevTools] Improve Layering Between Console and Renderer (#30925)
The console instrumentation should not know about things like Fibers.
Only the renderer bindings should know about that stuff. We can improve
the layering by just moving all that stuff behind a `getComponentStack`
helper that gets injected by the renderer.

This sets us up for the Flight renderer #30906 to have its own
implementation of this function.
2024-09-09 15:33:30 -04:00
Sebastian Markbåge
fa3cf509a9 [DevTools] Add Map for Server Component Logs (#30905)
Stacked on #30899.

This adds another map to store Server Components logs. When they're
replayed with an owner we can associate them with a DevToolsInstance.
The replaying should happen before they can mount in Fiber so they'll
always have all logs when they mount. There can be more than one
Instance associated with any particular ReactComponentInfo. It can also
be unmounted and restored later.

One thing that's interesting about these is that when a Server Component
tree refreshes a new set of ReactComponentInfo will update through the
tree and the VirtualInstances will update with new instances. This means
that the old errors/warnings are no longer associated with the
VirtualInstance. I.e. it's not continually appended like updates do for
Fiber backed instances. On the client we dedupe errors/warnings for the
life time of the page. On the server that doesn't work well because it
would mean that when you refresh the page, you miss out on warnings so
we dedupe them per request instead. If we just appended on refresh it
would keep adding them.

If ever add a deduping mechanism that spans longer than a request, we
might need to do more of a merge when these updates.

Nothing actually adds logs to this map yet. That will need an
integration with Flight in a follow up.
2024-09-09 15:12:28 -04:00
Sebastian Markbåge
f4b3a1fea2 [DevTools] Delete fiberToFiberInstanceMap (#30900)
Stacked on #30899.

After the rest of the stack this is now unused so we can save time and
memory avoiding to maintain it. 🎉
2024-09-09 15:12:13 -04:00