This is using the same trick as #30798 but for runtime code too. It's
essential zero cost.
This lets us include a source location for parent stacks of Server
Components when it has an owned child's location. Either from JSX or
I/O.
Ironically, a Component that throws an error will likely itself not get
the stack because it won't have any JSX rendered yet.
## Summary
The devtools Components tab's component tree view currently has a
behavior where the indentation of each level of the tree scales based on
the available width of the view. If the view is narrow or component
names are long, all indentation showing the hierarchy of the tree scales
down with the view width until there is no indentation at all. This
makes it impossible to see the nesting of the tree, making the tree view
much less useful. With long component names and deep hierarchies this
issue is particularly egregious. For comparison, the Chrome Dev Tools
Elements panel uses a fixed indentation size, so it doesn't suffer from
this issue.
This PR adds a minimum pixel value for the indentation width, so that
even when the window is narrow some indentation will still be visible,
maintaining the visual representation of the component tree hierarchy.
Alternatively, we could match the behavior of the Chrome Dev Tools and
just use a constant indentation width.
## How did you test this change?
- tests (yarn test-build-devtools)
- tested in browser:
- added an alternate left/right split pane layout to
react-devtools-shell to test with
(https://github.com/facebook/react/pull/33516)
- tested resizing the tree view in different layout modes
### before this change:
https://github.com/user-attachments/assets/470991f1-dc05-473f-a2cb-4f7333f6bae4
with a long component name:
https://github.com/user-attachments/assets/1568fc64-c7d7-4659-bfb1-9bfc9592fb9d
### after this change:
https://github.com/user-attachments/assets/f60bd7fc-97f6-4680-9656-f0db3d155411
with a long component name:
https://github.com/user-attachments/assets/6ac3f58c-42ea-4c5a-9a52-c3b397f37b45
## Summary
This tool leverages DevTools to get the component tree from the
currently open React App. This gives realtime information to agents
about the state of the app.
## How did you test this change?
Tested integration with Claude Desktop
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.
## Summary
When using React DevTools to highlight component updates, the highlights
would sometimes appear behind elements that use the browser's
[top-layer](https://developer.mozilla.org/en-US/docs/Glossary/Top_layer)
(such as `<dialog>` elements or components using the Popover API). This
made it difficult to see which components were updating when they were
inside or behind top-layer elements.
This PR fixes the issue by using the Popover API to ensure that
highlighting appears on top of all content, including elements in the
top-layer. The implementation maintains backward compatibility with
browsers that don't support the Popover API.
## How did you test this change?
I tested this change in the following ways:
1. Manually tested in Chrome (which supports the Popover API) with:
- Created a test application with React components inside `<dialog>`
elements and custom elements using the Popover API
- Verified that component highlighting appears above these elements when
they update
- Confirmed that highlighting displays correctly for nested components
within top-layer elements
2. Verified backward compatibility:
- Tested in browsers without Popover API support to ensure fallback
behavior works correctly
- Confirmed that no errors occur and highlighting still functions as
before
3. Ran the React DevTools test suite:
- All tests pass successfully
- No regressions were introduced
[demo-page](https://devtools-toplayer-demo.vercel.app/)
[demo-repo](https://github.com/yongsk0066/devtools-toplayer-demo)
### AS-IS
https://github.com/user-attachments/assets/dc2e1281-969f-4f61-82c3-480153916969
### TO-BE
https://github.com/user-attachments/assets/dd52ce35-816c-42f0-819b-0d5d0a8a21e5
This change adds a background color to Toggles to make them easier to
see. This is especially important when DevTools are not in focus, and
it's harder to see.
Test plan:
1. `yarn build:chrome:local`
2. Inspect components
3. Hover over "Select an Element in page to inspect it"
4. Observe background change
<!--
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
This PR adds support for displaying the names of changed hooks directly
in the Profiler tab, making it easier to identify specific updates.
A `HookChangeSummary` component has been introduced to show these hook
names, with a `displayMode` prop that toggles between `“compact”` for
tooltips and `“detailed”` for more in-depth views. This keeps tooltip
summaries concise while allowing for a full breakdown where needed.
This functionality also respects the `“Always parse hook names from
source”` setting from the Component inspector, as it uses the same
caching mechanism already in place for the Components tab. Additionally,
even without hook names parsed, the Profiler will now display hook types
(like `State`, `Callback`, etc.) based on data from `inspectedElement`.
To enable this across the DevTools, `InspectedElementContext` has been
moved higher in the component tree, allowing it to be shared between the
Profiler and Components tabs. This update allows hook name data to be
reused across tabs without duplication.
Additionally, a `getAlreadyLoadedHookNames` helper function was added to
efficiently access cached hook names, reducing the need for repeated
fetching when displaying changes.
These changes improve the ability to track specific hook updates within
the Profiler tab, making it clearer to see what’s changed.
### Before
Previously, the Profiler tab displayed only the IDs of changed hooks, as
shown below:
<img width="350" alt="Screenshot 2024-11-01 at 12 02 21_cropped"
src="https://github.com/user-attachments/assets/7a5f5f67-f1c8-4261-9ba3-1c76c9a88af3">
### After (without hook names parsed)
When hook names aren’t parsed, custom hooks and hook types are displayed
based on the inspectedElement data:
<img width="350" alt="Screenshot 2024-11-01 at 12 03 09_cropped"
src="https://github.com/user-attachments/assets/ed857a6d-e6ef-4e5b-982c-bf30c2d8a7e2">
### After (with hook names parsed)
Once hook names are fully parsed, the Profiler tab provides a complete
breakdown of specific hooks that have changed:
<img width="350" alt="Screenshot 2024-11-01 at 12 03 14_cropped"
src="https://github.com/user-attachments/assets/1ddfcc35-7474-4f4d-a084-f4e9f993a5bf">
This should resolve#21856🎉
Native only. Displays the native tag for Native Host components inside a
badge, when user inspects the component.
Only displaying will be supported for now, because in order to get
native tags indexable, they should be part of the bridge operations,
which is technically a breaking change that requires significantly more
time investment.
The text will only be shown when user hovers over the badge.
<!--
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
1. Having a development build for FB will be convenient for fb internal
feature development
2. Add a new checkbox to toggle new internal features added to React
Devtools.
## How did you test this change?
1. yarn test
2. set extra env variables in bash profile and build an internal version
with the new script.
3. toggle on/off the new checkbox, the value is stored in local storage
correctly.
---------
Co-authored-by: Aohua Mu <muaohua@fb.com>
We don't have an experimental-only build of devtools, but we can at
least add these filters to the internal build.
A better way would be to use feature detection, but I'm not sure how and
this isn't a very heavily used feautre.
This PR separates Activity to it's own element type separate from
Offscreen. The goal is to allow us to add Activity element boundary
semantics during hydration similar to Suspense semantics, without
impacting the Offscreen behavior in suspended children.
For Hookstate Proxies of class instances, `data.constructor.name`
returns `Proxy({})`, so use
`Object.getPrototypeOf(data).constructor.name` instead, which works
correctly from my testing.
<!--
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
React DevTools immediately bricks itself if you inspect any component
that has a prop that is a Hookstate that wraps a class instance ...
because these are proxies where `data.constructor.name` returns some
un-cloneable object, but `Object.getPrototypeOf(data)` doesn't return
`Object` (it returns the prototype of the class inside).
## How did you test this change?
This part of the code has no associated tests at all.
Technically,
`packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js`
exists, but I tried `yarn test` and these tests aren't even executed
anymore. I can't figure it out, so whatever.
If you run this code:
```js
class Class {}
const instance = new Class();
const instanceProxy = new Proxy(instance, {
get(target, key, receiver) {
if (key === 'constructor') {
return { name: new Proxy({}, {}) };
}
return Reflect.get(target, key, receiver);
},
});
```
then `instanceProxy.constructor.name` returns some proxy that cannot be
cloned, but `Object.getPrototypeOf(instanceProxy).constructor.name`
returns the correct value.
This PR fixes the devtools to use
`Object.getPrototypeOf(instanceProxy).constructor.name`.
I modified my local copy of devtools to use this method and it fixed the
bricking that I experienced.
Related #29954
<!--
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.
We have this really old (5+ years) feature for inspecting native styles
of React Native Host components.
We also have a custom Cache implementation in React DevTools, which was
forked from React at some point. We know that this should be removed,
but it spans through critical parts of the application, like fetching
and caching inspected element.
Before this PR, this was also used for caching native style and layouts
of RN Host components. This approach is out of date, and was based on
the presence of Suspense boundary around inspected element View, which
we have removed to speed up element inspection -
https://github.com/facebook/react/pull/30555.
Looks like I've introduced a regression in
https://github.com/facebook/react/pull/31956:
- Custom Cache implementation will throw thenables and suspend.
- Because of this, some descendant Suspense boundaries will not resolve
for a long time, and React will throw an error
https://react.dev/errors/482.
I've switched from a usage of this custom Cache implementation to a
naive fetching in effect and keeping the layout and style in a local
state of a Context, which will be propagated downwards. The race should
be impossible, this is guaranteed by the mechanism for queueing messages
through microtasks queue.
The only downside is the UI. If you quickly switch between 2 elements,
and one of them has native style, while the other doesn't, UI will feel
jumpy. We can address this later with a Suspense boundary, if needed.
Addresses https://github.com/facebook/react/issues/32244.
### Chromium
We will use
[chrome.permissions](https://developer.chrome.com/docs/extensions/reference/api/permissions)
for checking / requesting `clipboardWrite` permission before copying
something to the clipboard.
### Firefox
We will keep `clipboardWrite` as a required permission, because there is
no reliable and working API for requesting optional permissions for
extensions that are extending browser DevTools:
- `chrome.permissions` is unavailable for devtools pages -
https://bugzilla.mozilla.org/show_bug.cgi?id=1796933
- You can't call `chrome.permissions.request` from background, because
this instruction has to be executed inside user-event callback,
basically only initiated by user.
I don't really want to come up with solutions like opening a new tab
with a button that user has to click.
<!--
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
This pull request resolves an issue where consecutive profiling sessions
would cause Dev Tools to freeze due to an infinite loop of state
updates. The problem occurs when the startProfiling function triggers a
call to [`selectCommitIndex(0)` in
SnapshotSelector](b3a95caf61/packages/react-devtools-shared/src/devtools/views/Profiler/SnapshotSelector.js (L77-L85))
as previous profiling data is available, which causes a re-render. Then,
[ProfilerContextProvider calls
`selectCommitIndex(null)`](b3a95caf61/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js (L231-L241))
to clear the view while profiling is in progress, leading to another
re-render and creating an infinite loop. This behavior was prevented by
clearing the existing profiling data before starting a new session.
Closes#31977Closes#31679
## 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.
-->
I ran the Dev Tools locally following [the contributing
guideline](b3a95caf61/packages/react-devtools/CONTRIBUTING.md).
I observed the freeze at the start of the second profiling session.
Then, I modified the code to clear the store when starting a new session
and ran the Dev Tools again. This time, no freeze was observed.
Before:
https://github.com/user-attachments/assets/9d790f84-f6d0-4951-8202-e599cf8d225b
After:
https://github.com/user-attachments/assets/af097019-0b8f-49dd-8afc-0f6cd72af787
Stacked on https://github.com/facebook/react/pull/31956. See [commit on
top](ecb8df4175).
Use `initialScrollOffset` prop for `FixedSizeList` from `react-window`.
This happens when user selects an element in built-in Elements panel in
DevTools, and then opens Components panel from React DevTools - elements
will be synced and corresponding React Element will be pre-selected, we
just have to scroll to its position now.
Stacked on https://github.com/facebook/react/pull/31892, see commit on
top.
For some reason, there were 2 fields different fields for essentially
same thing: `selectedElementID` and `inspectedElementID`. Basically, the
change is:
```
selectedElementID -> inspectedElementID
selectedElementIndex -> inspectedElementIndex
```
I have a theory that it was due to previously used async approach around
element inspection, and the whole `InspectedElementView` was wrapped in
`Suspense`.
Related: https://github.com/facebook/react/pull/31342
This fixes RDT behaviour when some DOM element was pre-selected in
built-in browser's Elements panel, and then Components panel of React
DevTools was opened for the first time. With this change, React DevTools
will correctly display the initial state of the Components Tree with the
corresponding React Element (if possible) pre-selected.
Previously, we would only subscribe listener when `TreeContext` is
mounted, but this only happens when user opens one of React DevTools
panels for the first time. With this change, we keep state inside
`Store`, which is created when Browser DevTools are opened. Later,
`TreeContext` will use it for initial state value.
Planned next changes:
1. Merge `inspectedElementID` and `selectedElementID`, I have no idea
why we need both.
2. Fix issue with `AutoSizer` rendering a blank container.
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.
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.
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>
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.
## 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-9a867554f74chttps://github.com/user-attachments/assets/32af4d98-92a5-47dd-a732-f05c2293e41b
---------
Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>
## 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.
<!--
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
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`.
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>
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)
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.
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.
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.
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.
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">
<!--
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
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)
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
<!--
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
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.
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`.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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)
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.
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">
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.
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.
This represents a virtual renderer that connects to the Flight Client.
It's virtual in the sense that the actual rendering has already happened
on the server. The Flight Client parses the result. Most of the result
then end up in objects that render into another renderer and that's how
we see most Server Components in DevTools. As part of the client's tree.
However, some things are side-effects that don't really connect to any
particular client renderer. For example preloads() and logs. For those
we need to treat the Flight Client as if it was its own renderer just
like a Fiber renderer or even legacy renderer. We really could support
Fizz and Flight Server as DevTools targets too for example to connect it
to the backend but there's just not much demand for that.
This will initially only be used to track the owners of replayed console
logs but could be expanded to more. For example to send controls to
start profiling on the server. It could also be expanded to build an RSC
payload inspector that is automatically connected.
We can simplify this tracking by not having a separate pending set of
logs and the logs tracked per instance and instead we just track the
logs per Fiber. This avoids the need to move it back into the pending
set after unmounts in case a Fiber is reparented.
The main motivation for this is to unify with an upcoming tracking of
logs for Server Components. For those it doesn't make sense to move them
into a per instance set and because the same Server Component - and its
logs - may appear more than once. So no particular instance should steal
it.
The second part of this change is that instead of looking up the
instance from fiber, which requires the fiberToFiberInstanceMap, we
instead look up if a component has any new logs when we traverse it in
the commit phase. After all for a component to have had a log it must
have updated. This is a similar technique to #30897. This technique also
works for Server Components without having to maintain a one to many
relationship from ComponentInfo to VirtualInstance. So it unifies them.
Normally this look up would be fast since the `fiberToComponentsLogs`
set would be empty and so doesn't add any significant weight to the
commit phase. If there's a ton of logs on many different components then
it's not great since it would slow down the commit phase but that's not
what we expect to see so in typical usage, this is better.
There is an unfortunate consequence though which is that
`console.warn/error` in passive effects (i.e. `useEffect`) wouldn't be
picked up because currently we traverse the logs in
`handleCommitFiberRoot` which is too early. If we moved that to
`handlePostCommitFiberRoot` this wouldn't be a problem. In the meantime,
I just detect this and do a brute force flush by walking all mounted
instances if there's a `console.warn/error` inside a passive effect.
If we ever add "owners" to event handlers that are triggered outside the
render/commit phases (like `<div onClick={...}>`) and we want to
associate error/warnings in those, we'd need a different technique to
ensure those get flushed in time.
## Summary
This PR bumps Flow all the way to the latest 0.245.2.
Most of the suppressions comes from Flow v0.239.0's change to include
undefined in the return of `Array.pop`.
I also enabled `react.custom_jsx_typing=true` and added custom jsx
typing to match the old behavior that `React.createElement` is
effectively any typed. This is necessary since various builtin
components like `React.Fragment` is actually symbol in the React repo
instead of `React.AbstractComponent<...>`. It can be made more accurate
by customizing the `React$CustomJSXFactory` type, but I will leave it to
the React team to decide.
## How did you test this change?
`yarn flow` for all the renderers
Stacked on #30896.
The problem with the `getUpdatersList` function is that it iterates over
Fibers and then looks up each of those Fibers in the
fiberToFiberInstanceMap which we ideally could get rid of.
However, every time an updater comes into play for a commit it must mean
that something below the updater itself updated and so the updater will
also be cloned which means we'll pass it on the way down when traversing
the tree in the commit.
When we do this traversal, we can just look if the Fiber is in the
updater set and if so add it to the updater list as we go.
When Context change tracking was added to support modern Context it
relied on the "memoizedValue" to read the current value. This only works
in React 18+ when it was added to support Lazy Context Propagation.
However, the backend stored the old value the same way it used to work
for legacy Context in a global map. This was unnecessary since we *also*
have the old value on the previous Fiber.
This removes all the costly tracking of previous values for every Fiber
that uses Contexts slowing down profiling. Instead, we just compare the
Contexts from
The downside is that this no longer supports detecting changes due to
legacy Context because it doesn't have a similar "previous" value.
However, legacy Context has long been deprecated and is completely
removed in 19. So I don't think it's worth supporting since you have to
be on an old version *and* actually use legacy Context *and* trying to
profile something that updates it. Which btw, updating legacy contexts
only worked at all from 16 something when we made updates work. So it
was unusual even in the slight gap where you could and before you had
migrated to modern Context introduced in 16.3.
Ideally we shouldn't use the `.alternate` to access previous state
because ideally Fibers shouldn't have alternates.
The only case it's ok to use it is when it is used to identity the
stateful part of a component's identity. In a non-alternate Fiber model
there would instead be another object that represents instance but in
the current model it's modeled by the pair.
It's not ok is to get the previous state of the tree since that would
not live on the stateful part.
We don't generally need this though because we have the previous state
on instance.data before updating it, or passed from above.
While looking at the Context tracking implementation for other reasons I
noticed this bug.
Originally it wasn't allowed to have conditional `useContext(context)`
(although we did because it's technically possible). With `use(context)`
it is officially allowed to be conditional as long as it is within a
Hook/Component and not within a try/catch.
This means that this loop comparing previous and next contexts need to
consider that the Context objects might not line up and so it's possibly
comparing apples to oranges. We already bailed if one was longer than
the other.
If the order of contexts changes later in the component that means
something else must have already changed earlier so the reason for the
rerender isn't the context so we can just return false in that case.
First, this basically reverts
1f3892ef8c
to use a Map/Set to track what is forced to suspend/error again instead
of flags on the Instance. The difference is that now the key in the
Fiber itself instead of the ID. Critically this avoids the
fiberToFiberInstance map to look up whether or not a Fiber should be
forced to suspend when asked by the renderer.
This also allows us to force suspend/error on filtered instances. It's a
bit unclear what should happen when you try to Suspend or Error a child
but its parent boundary is filtered. It was also inconsistent between
Suspense and Error due to how they were implemented.
I think conceptually you're trying to simulate what would happen if that
Component errored or suspended so it would be misleading if we triggered
a different boundary than would happen in real life. So I think we
should trigger the nearest unfiltered Fiber, not the nearest Instance.
The consequence of this however is that if this instance was filtered,
there's no way to undo it without refreshing or removing the filter.
This is an edge case though since it's unusual you'd filter these in the
first place.
It used to be that Suspense walked the store in the frontend and Error
walked the Fibers in the backend. They also did this somewhat eagerly.
This simplifies and unifies the model by passing the id of what you
clicked in the frontend and then we walk the Fiber tree from there in
the backend to lazily find the boundary. However I also eagerly walk the
tree at first to find whether we have any Suspense or Error boundary
parents at all so we can hide the buttons if not.
This also implements it to work with VirtualInstances using #30865. I
find the nearest Fiber Instance downwards filtered or otherwise. Then
from its parent we find the nearest Error or Suspense boundary. That's
because VirtualInstance will always have their inner Fiber as an
Instance but they might not have their parent since it might be
filtered. Which would potentially cause us to skip over a filtered
parent Suspense boundary.
When we filter Fiber Instances where have no way to recover our position
in the Fiber tree. The extreme form of this is if you filter out all the
Fibers and keep only Server Components.
This affects operations that are performed against fibers such as
collecting Host Instances for highlighting or emulating
suspending/erroring.
Conceptually we don't need to add this into the DevToolsInstance tree
because we only need to get to some Fibers from a VirtualInstance. A
Virtual Instance can contain more than one conceptual child Fiber. It
would be easier if we didn't include them in the tree on one hand
because we could just traverse the tree and assume it looks like the one
on the frontend. But it's also tricky to manage the lifetime. So I went
with a special FilteredFiberInstance node in the tree.
Currently I only add it if its parent would've been a VirtualInstance
since we don't need it in any other cases. If the parent was another
FiberInstance it already has a Fiber.
There might be need for always tracking all Instances whether they're
filtered or not or just moving filtering to the frontend but for now I'm
keeping the general architecture as is.
This lets us get from a HostInstance to the nearest DevToolsInstance
without relying on `findFiberByHostInstance` and
`fiberToDevToolsInstanceMap`. We already did the equivalent of this for
Resources in HostHoistables.
One issue before was that we'd ideally get away from the
`fiberToDevToolsInstanceMap` map in general since we should ideally not
treat Fibers as stateful but they could be replaced by something else
stateful in principle.
This PR also addresses Virtual Instances. Now you can select a DOM node
and have it select a Virtual Instance if that's the nearest parent since
the parent doesn't have to be a Fiber anymore.
However, the other reason for this change is that I'd like to get rid of
the need for the `findFiberByHostInstance` from being injected. A
renderer should not need to store a reference back from its instance to
a Fiber. Without the Synthetic Event system this wouldn't be needed by
the renderer so we should be able to remove it. We also don't really
need it since we have all the information by just walking the commit to
collect the nodes if we just maintain our own Map.
There's one subtle nuance that the different renderers do. Typically a
HostInstance is the same thing as a PublicInstance in React but
technically in Fabric they're not the same. So we need to translate
between PublicInstance and HostInstance. I just hardcoded the Fabric
implementation of this since it's the only known one that does this but
could feature detect other ones too if necessary. On one hand it's more
resilient to refactors to not rely on injected helpers and on hand it
doesn't follow changes to things like this.
For the conflict resolution I added in #30494 I had to make that
specific to DOM so we can move the DOM traversal to the backend instead
of the injected helper.
This lets us highlight Server Components.
However, there is a problem with this because if the actual nearest
Fiber is filtered, there's no FiberInstance and so we might skip past it
and maybe never find a child while walking the whole tree. This is very
common in the case where you have just Server Components and Host
Components which are filtered by default.
Note how the DOM nodes that are just plain host instances without client
component wrappers are not highlighted here:
<img width="1102" alt="Screenshot 2024-08-30 at 4 33 55 PM"
src="https://github.com/user-attachments/assets/c9a7b91e-5faf-4c60-99a8-1195539ff8b5">
Fixing that needs a separate refactor though and related to several
other features that already have a similar issue without
VirtualInstances like Suspense/Error Boundaries (triggering
suspense/error on a filtered Suspense/ErrorBoundary doesn't work
correctly). So this first PR just adds the feature for the common case
where there's at least some Fibers.
Stacked on #30842.
This adds a filter to be able to exclude Components from a certain
environment. Default to Client or Server.
The available options are computed into a dropdown based on the names
that are currently used on the page (or an option that were previously
used). In addition to the hardcoded "Client". Meaning that if you have
Server Components on the page you see "Server" or "Client" as possible
options but it can be anything if there are multiple RSC environments on
the page.
"Client" in this case means Function and Class Components in Fiber -
excluding built-ins.
If a Server Component has two environments (primary and secondary) then
both have to be filtered to exclude it.
We don't show the option at all if there are no Server Components used
in the page to avoid confusing existing users that are just using Client
Components and wouldn't know the difference between Server vs Client.
<img width="815" alt="Screenshot 2024-08-30 at 12 56 42 AM"
src="https://github.com/user-attachments/assets/e06b225a-e85d-4cdc-8707-d4630fede19e">
Support filtering Virtual Instances with existing filters.
Server Components are considered "Functions".
In a follow up I'll a new filter for "Environment" which will let you
filter by Client vs Server (and more).
This appends a (filtered) virtual instance path at the end of the fiber
path. If a virtual instance is selected inside the fiber.
The main part of the path is still just the fiber path since that's the
semantically stateful part. Then we just tack on a few virtual path
frames at the end if we're currently selecting a specific Server
Component within the nearest Fiber.
I also took the opportunity to fix a bug which caused selections inside
Suspense boundaries to not be tracked.
Firefox [finally supports
`ExecutionWorld.MAIN`](https://bugzilla.mozilla.org/show_bug.cgi?id=1736575)
in content scripts, which means we can migrate the browser extension to
Manifest V3.
This PR also removes a bunch of no longer required explicit branching
for Firefox case, when we are using Manifest V3-only APIs.
We are also removing XMLHttpRequest injection, which is no longer needed
and restricted in Manifest V3. The new standardized approach (same as in
Chromium) doesn't violate CSP rules, which means that extension can
finally be used for apps running in production mode.
This loops over the remainingReconcilingChildren to find existing
FiberInstances that match the updated Fiber. This is the same thing we
already do for virtual instances. This avoids the need for a
`fiberToFiberInstanceMap`.
This loop is fast but there is a downside when the children set is very
large and gets reordered with keys since we might have to loop over the
set multiple times to get to the instances in the bottom. If that
becomes a problem we can optimize it the same way ReactChildFiber does
which is to create a temporary Map only when the children don't line up
properly. That way everything except the first pass can use the Map but
there's no need to create it eagerly.
Now that we have the loop we don't need the previousSibling field so we
can save some memory there.
These don't have their own time since they don't take up any time to
render but they show up in the tree for context. However they never
render themselves. Their base tree time is the base time of their
children. This way they take up the same space as their combined
children in the Profiler tree. (Instead of leaving a blank line which
they did before this PR.)
The frontend doesn't track the difference between a virtual instance and
a Fiber that didn't render this update. This might be a bit confusing as
to why it didn't render. I add the word "client" to make it a bit
clearer and works for both. We should probably have different verbiage
here based on it is a Server Component or something else.
<img width="1103" alt="Screenshot 2024-08-26 at 5 00 47 PM"
src="https://github.com/user-attachments/assets/87b811d4-7024-466a-845d-542493ed3ca2">
I also took the opportunity to remove idToTreeBaseDurationMap and
idToRootMap maps. Cloning the Map isn't really all that super fast
anyway and it means we have to maintain the map continuously as we
render. Instead, we can track it on the instances and then walk the
instances to create a snapshot when starting to profile. This isn't as
fast but really fast too and requires less bookkeeping while rendering
instead which is more sensitive than that one snapshot in the beginning.
We don't have the source location of Server Components on the client
because we don't want to eagerly do the throw trick for all Server
Components just in case. Unfortunately Node.js doesn't expose V8's API
to get a source location of a function.
We do have the owner stacks of the JSX that created it though and at
some point we'll also show that location in DevTools.
However, the realization is that if a Server Component is the owner of
any child. The owner stack of that child will have the owner component's
source location as its bottom stack frame.
The technique I'm implementing here is to track whenever a child mounts
we already have its owner. We track the first discovered owned child's
stack on the owner. Then when we ask for a Source location of the owner
do we parse that stack and extract the location of the bottom frame.
This doesn't give us a location necessarily in the top of the function
but somewhere in the function.
In this case the first owned child is the Container:
<img width="1107" alt="Screenshot 2024-08-22 at 10 24 42 PM"
src="https://github.com/user-attachments/assets/95f32850-24a5-4151-8ce6-b7b89db68aee">
<img width="648" alt="Screenshot 2024-08-22 at 10 24 20 PM"
src="https://github.com/user-attachments/assets/4bcba033-866f-4684-9beb-de09d189deff">
We can even use this technique for Fibers too. Currently I use this as a
fallback in case the error technique didn't work. This covers a case
where nothing errors but you still render a child. This case is actually
quite common:
```
function Foo() {
return <Bar />;
}
```
However, for Fibers we could really just use the `inspect(function)`
technique which works for all cases. At least in Chrome.
Unfortunately, this technique doesn't work if a Component doesn't create
any new JSX but just renders its children. It also currently doesn't
work if the child is filtered since I only look up the owner if an
instance is not filtered. This means that the container in the fixture
can't view source by default since the host component is filtered:
```
export default function Container({children}) {
return <div>{children}</div>;
}
```
<img width="1107" alt="Screenshot 2024-08-22 at 10 24 35 PM"
src="https://github.com/user-attachments/assets/c3f8f9c5-5add-4d35-9290-3a5079e82adc">
I noticed that there is a delay due to the inspection being split into
one part that gets the attribute and another eval that does the
inspection. This is a bit hacky and uses temporary global names that are
leaky. The timeout was presumably to ensure that the first step had
fully propagated but it's slow. As we've learned, it can be throttled,
and it isn't a guarantee either way.
Instead, we can just consolidate these into a single operation that
by-passes the bridge and goes straight to the renderer interface from
the eval.
I did the same for the viewElementSource helper even though that's not
currently in use since #28471 but I think we probably should return to
that technique when it's available since it's more reliable than the
throw - at least in Chrome. I'm not sure about the status of React
Native here. In Firefox, inspecting a function with source maps doesn't
seem to work. It doesn't jump to original code.
Currently you can jump to definition of a function by right clicking
through the context menu. However, it's pretty difficult to discover.
This makes the functions clickable to jump to definition - like links.
This uses the same styling as we do for links (which are btw only
clickable if they're not editable). Including cursor: pointer.
I added a background on hover which follows the same pattern as the
owners list.
I also dropped the ƒ prefix when displaying functions. This is a cute
short cut and there's precedence in how Chrome prints functions in the
console *if* the function's toString would've had a function prefix like
if it was a function declaration or expression. It does not do this for
arrow functions or object methods.
Elsewhere in the JS ecosystem this isn't really used anywhere. It
invites more questions than it answers.
The parenthesis and curlies are enough. There's no ambiguity here since
strings have quotations. It looks better with just its object method
form. Keeping it simple seems best. To my eyes this flows better because
I'm used to looking at function syntax but not weird "f"s.
Before:
<img width="433" alt="Screenshot 2024-08-20 at 11 55 09 PM"
src="https://github.com/user-attachments/assets/9dd50da6-598f-4291-9e24-1cdc7200dc9e">
After:
<img width="388" alt="Screenshot 2024-08-20 at 11 46 01 PM"
src="https://github.com/user-attachments/assets/dd988e14-412e-4deb-8c8c-26a54be8331f">
After (Hover):
<img width="389" alt="Screenshot 2024-08-20 at 11 46 31 PM"
src="https://github.com/user-attachments/assets/6fb4ebed-5dc1-448a-8e4d-b6d4f3903329">
DevTools shouldn't use react-is since that's versioned to one version of
React. We don't need to since we use all the symbols from
shared/ReactSymbols anyway and have a fork of typeOf that can cover
both.
Now JSX of old React versions show up with proper JSX formatting when
inspecting.
This enables finding Server Components on the owner path. Server
Components aren't stateful so there's not actually one specific owner
that it necessarily matches. So it can't be a global look up. E.g. the
same Server Component can be rendered in two places or even nested
inside each other.
Therefore we need to find an appropriate instance using a heuristic. We
can do that by traversing the parent path since the owner is likely also
a parent. Not always but almost always.
To simplify things we can also do the same for Fibers. That brings us
one step closer to being able to get rid of the global
fiberToFiberInstance map since we can just use the shadow tree to find
this information.
This does mean that we can't find owners that aren't parents which is
usually ok. However, there is a test case that's interesting where you
have a React ART tree inside a DOM tree. In that case the owners
actually span multiple renderers and roots so the owner is not on the
parent stack. Usually this is fine since you'd just care about the
owners within React ART but ideally we'd support this. However, I think
that really the fix to this is that the React ART tree itself should
actually show up inside the DOM tree in DevTools and in the virtual
shadow tree because that's conceptually where it belongs. That would
then solve this particular issue. We'd just need some way to associate
the root with a DOM parent when it gets mounted.
Supports showing the key in DevTools on the Server Component that the
key was applied to. We can also use this to reconcile to preserve
instance equality when they're reordered.
One thing that's a bit weird about this is that if you provide an
explicit key on a Server Component that alone doesn't have any
semantics. It's because we pass the key down and let the nearest child
inherit the key or get prefixed by the key.
So you might see the same key as a prefix on the child of the Server
Component too which might be a bit confusing. We could remove the prefix
from children but that might also be a bit confusing if they collide.
The div in this case doesn't have a key explicitly specified. It gets it
from the Server Component parent.
<img width="1107" alt="Screenshot 2024-08-14 at 10 06 36 PM"
src="https://github.com/user-attachments/assets/cfc517cc-e737-44c3-a1be-050049267ee2">
Overall keys get a bit confusing when you apply filter. Especially since
it's so common to actually apply the key on a Host Instance. So you
often don't see the key.
This adds VirtualInstances to the tree. Each Fiber has a list of its
parent Server Components in `_debugInfo`. The algorithm is that when we
enter a set of fibers, we actually traverse level 0 of all the
`_debugInfo` in each fiber. Then level 1 of each `_debugInfo` and so on.
It would be simpler if `_debugInfo` only contained Server Component
since then we could just look at the index in the array but it actually
contains other data as well which leads to multiple passes but we don't
expect it to have a lot of levels before hitting a reified fiber.
Finally when we hit the end a traverse the fiber itself.
This lets us match consecutive `ReactComponentInfo` that are all the
same at the same level. This creates a single VirtualInstance for each
sequence. This lets the same Server Component instance that's a parent
to multiple children appear as a single Instance instead of one per
Fiber.
Since a Server Component's result can be rendered in more than one place
there's not a 1:1 mapping though. If it is in different parents or if
the sequence is interrupted, then it gets split into two different
instances with the same `ReactComponentInfo` data.
The real interesting case is what happens during updates because this
algorithm means that a Fiber can become reparented during an update to
end up in a different VirtualInstance. The ideal would maybe be that the
frontend could deal with this reparenting but instead I basically just
unmount the previous instance (and its children) and mount a new
instance which leads to some interesting scenarios. This is inline with
the strategy I was intending to pursue anyway where instances are
reconciled against the previous children of the same parent instead of
the `fiberToFiberInstance` map - which would let us get rid of that map.
In that case the model is resilient to Fiber being in more than one
place at a time.
However this unmount/remount does mean that we can lose selection when
this happens. We could maybe do something like using the tracked path
like I did for component filters. Ideally it's a weird edge case though
because you'd typically not have it. The main case that it happens now
is for reorders of list of server components. In that case basically all
the children move between server components while the server components
themselves stay in place. We should really include the key in server
components so that we can reconcile them using the key to handle
reorders which would solve the common case anyway.
I convert the name to the `Env(Name)` pattern which allows the
Environment Name to be used as a badge.
<img width="1105" alt="Screenshot 2024-08-13 at 9 55 29 PM"
src="https://github.com/user-attachments/assets/323c20ba-b655-4ee8-84fa-8233f55d2999">
(Screenshot is with #30667. I haven't tried it with the alternative
fix.)
---------
Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>
Alternative to https://github.com/facebook/react/pull/30667.
Basically wrap every section in a `div` with the same class, and only
apply `border-bottom` for every instance, except for the last child. We
are paying some cost by having more divs, but thats more explicit.
There was a comment that it's not safe to walk the unmounted fiber tree
which I'm not sure is correct or not but we need to walk the instance
tree to be able to clean up virtual instances anyway. We already walk
the instance tree to clean up "remaining instances".
This is also simpler because we don't need to special case Suspense
boundaries. We simply clean up whatever branch we had before.
The ultimate goal is to also walk the instance tree for updates so we
don't need a fiber to instance map.
Stacked on #30625 and #30657.
This ensures that we only create instances during the commit
reconciliation and that we don't create unnecessary instances for things
that are filtered or not mounted. This ensures that we also can rely on
the reconciliation to do all the clean up. Now everything is created and
deleted as a pair in the same pass.
Previously we were including unfiltered components in the owner stack
which probably doesn't make sense since you're intending to filter them
everywhere presumably. However, it also means that those links were
broken since you can't link into owners that don't exist in the parent
tree.
The main complication is the component filters. It relied on not
unmounting the old instances. I had to update some tests that asserted
on ids that are now shifted.
For warnings/errors tracking I now restore them back into the pending
set when they unmount. Basically it puts them back into their
"pre-commit" state. That way when they remount they’re still there.
For restoring the current selection I use the tracked path mechanism
instead of relying on the id being unchanged. This is better anyway
because if you filter out the currently selected item it's better to
select the nearest match instead of just losing the selection.
Same principle as #30555. We shouldn't be throttling the UI to make it
feel less snappy. Instead, we should use back-pressure to handle it.
Normally the browser handles it automatically with frame aligned events.
E.g. if the thread can't keep up with sync updates it doesn't send each
event but the next one. E.g. pointermove or resize.
However, it is possible that we end up queuing too many events if the
frontend can't keep up but the solution to this is the same as mentioned
in #30555. I.e. to track the last message and only send after we get a
response.
I still keep the throttle to persist the selection since that affects
the disk usage and doesn't have direct UX effects.
The main motivation for this change though is that lodash throttle
doesn't rely on timers but Date.now which makes it incompatible with
most jest helpers which means I can't write tests against these
functions properly.
This no longer uses the handleCommitFiberUnmount hook to track unmounts.
Instead, we can just unmount the DevToolsInstances that we didn't reuse.
This doesn't account for cleaning up instances that were unnecessarily
created when they weren't in the tree. I have a separate follow up for
that.
This also removes the queuing of untracking. This was added in #21523
but I don't believe it has been needed for a while because the mentioned
flushPendingErrorsAndWarningsAfterDelay hasn't called untrackFiberID for
a while so the race condition doesn't exist. It's hard to tell though
because from the issues there weren't really any repros submitted.
This is the beginning of a refactor of the DevTools Fiber backend. The
new approach is basically that we listen to each commit from Fiber and
traverse the tree - building up a filtered shadow tree. Then we send
diffs based on that tree and perform our own operations against that
instead of using Fibers as the source of truth.
Fiber diffs Elements -> Fibers. The backend diffs Fibers ->
DevToolsInstances as a side-effect it sends deltas to the front end.
This makes the algorithm resilient to a different Fiber implementation
that doesn't use pairs of Fibers (alternates) but instead stateless new
clones each time. In that world we can't treat Fibers as instances. They
can hold onto instances but they're not necessarily 1:1 themselves.
The same thing also applies to Server Components that don't have their
own instances.
The algorithm is more or less the same as React's reconciliation in
ReactChildFiber itself. However, we do a mutable update of the tree as
we go. We also cheat a bit here in the first version in that we still
have fiberToFiberInstance map and alternate which makes reorders easier.
Further down we could do the reorders by adding the previous set to a
temporary map like ChildFiber does but only if they're not already in
order.
This first bit is just about making sure that we produce correct trees.
We have fairly good test coverage already of that already.
In the next few follow ups I'll start simplifying the rest of the logic
by taking advantage of the new tree.
Follow up from #30584.
You can already select a singleton or hoistable (that's not a resource)
in the browser elements panel and it'll select the corresponding node in
the RDT Components panel. That works because it uses the same mechanism
as event dispatching and those need to be able to receive events.
However, you can't select a resource. Because that's conceptually one to
many.
This keeps track of which fiber is acquiring which resource so we can
find all the corresponding instances.
E.g. now you can select the `<link rel="stylesheet">` in the Flight
fixture in the Element panel and then the component that rendered it in
the Components panel will be selected.
If we had a concept multi-selection we could potentially select all of
them. This similar to how a Server Component can be rendered in more
than one place and if we want to select all matching ones. It's kind of
weird though and both cases are edge cases.
Notably imperative preloads do have elements that don't have any
corresponding component but that's ok. So they'll just select `<head>`.
Maybe in dev we could track the owners of those.
This is just for clarity at first.
Before:
- mountFiberRecursively accepts a set of children and flag that says
whether to just do one
- updateFiberRecursively accepts a fiber and loops over its children
- unmountFiberChildrenRecursively accepts a fiber and loops over its
children
After:
- mountFiberRecursively accepts a Fiber and calls
mountChildrenRecursively
- updateFiberRecursively accepts a Fiber and calls
updateChildrenRecursively
- unmountFiberRecursively accepts a Fiber and calls
unmountChildrenRecursively
- mountChildrenRecursively accepts a set of children and loops over each
one
- updateChildrenRecursively accepts a set of children and loops over
each one
- unmountChildrenRecursively accepts a set of children and loops over
each one
So now there's one place where things happens for the single item and
one place where we do the loop.
Basically the new Float types needs to be supported. Resources are a bit
special because they're a DOM specific type but we can expect any other
implementation using resources to provide and instance on this field if
needed.
There's a slightly related case for the reverse lookup. You can already
select a singleton or hoistable (that's not a resource) in the browser
elements panel and it'll select the corresponding node in the RDT
Components panel. That works because it uses the same mechanism as event
dispatching and those need to be able to receive events.
However, you can't select a resource. Because that's conceptually one to
many. We could in principle just search the tree for the first one or
keep a map of currently mounted resources and just pick the first fiber
that created it. So that you can select a resource and see what created
it. Particularly useful when there's only one Fiber which is most of the
time.
---------
Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>
Adding `__IS_NATIVE__` global, which will be used for forking backend
implementation. Will only be set to `true` for `react-devtools-core`
package, which is used by `react-native`.
Ideally, we should name it `react-devtools-native`, and keep
`react-devtools-core` as host-agnostic.
With this change, the next release of `react-devtools-core` should
append component stack as Error object, not as string, and should add
`(<anonymous>)` suffix to component stack frames.
[`react-window` disables `pointerEvents` while scrolling meaning you
can't click anything while
scrolling.](https://github.com/bvaughn/react-window/issues/128).
This means that the first click when you stop the scroll with inertial
scrolling doesn't get registered. This is suuuper annoying. This might
make sense when you click to stop on a more intentional UI but it
doesn't makes sense in a list like this because we eagerly click things
even on mousedown.
This PR just override that to re-enable pointer events.
Supposedly this is done for performance but that might be outdated
knowledge. I haven't observed any difference so far.
If we discover that it's a perf problem, there's another technique we
can use where we call `ownerDocument.elementFromPoint(e.pageX, e.pageY)`
and then dispatch the event against that element. But let's try the
simplest approach first?
There's two problems. The biggest one is that it turns out that Chrome
is throttling looping timers that we're using both while polling and for
batching bridge traffic. This means that bridge traffic a lot of the
time just slows down to 1 second at a time. No wonder it feels sluggish.
The only solution is to not use timers for this.
Even when it doesn't like in Firefox the batching into 100ms still feels
too sluggish.
The fix I use is to batch using a microtask instead so we can still
batch multiple commands sent in a single event but we never artificially
slow down an interaction.
I don't think we've reevaluated this for a long time since this was in
the initial commit of DevTools to this repo. If it causes other issues
we can follow up on those.
We really shouldn't use timers for debouncing and such. In fact, React
itself recommends against it because we have a better technique with
scheduling in Concurrent Mode. The correct way to implement this in the
bridge is using a form of back-pressure where we don't keep sending
messages until we get a message back and only send the last one that
matters. E.g. when moving the cursor over a the elements tab we
shouldn't let the backend one-by-one move the DOM node to each one we
have ever passed. We should just move to the last one we're currently
hovering over. But this can't be done at the bridge layer since it
doesn't know if it's a last-one-wins or imperative operation where each
one needs to be sent. It needs to be done higher. I'm not currently
seeing any perf problems with this new approach but I'm curious on React
Native or some thing. RN might need the back-pressure approach. That can
be a follow up if we ever find a test case.
Finally, the other problem is that we use a Suspense boundary around the
Element Inspection. Suspense boundaries are for things that are expected
to take a long time to load. This shows a loading state immediately. To
avoid flashing when it ends up being fast, React throttles the reveal to
200ms. This means that we take a minimum of 200ms to show the props. The
way to show fast async data in React is using a Transition (either using
startTransition or useDeferredValue). This lets the old value remaining
in place while we're loading the next one.
We already implement this using `inspectedElementID` which is the async
one. It would be more idiomatic to implement this with useDeferredValue
rather than the reducer we have now but same principle. We were just
using the wrong ID in a few places so when it synchronously updated they
suspended. So I just made them use the inspectedElementID instead.
Then I can simply remove the Suspense boundary. Now the selection
updates in the tree view synchronously and the sidebar lags a frame or
two but it feels instant. It doesn't flash to white between which is
key.
This just tracks the `.parent` field properly and uses DevToolsInstances
in more places that used to use IDs or Fibers.
I also use this new parent path when looking up a DevToolsInstance from
a DOM node. This should ideally be simple because the `.parent` field
represents only the unfiltered parents and include any virtual parents.
So we should be able to just get one from nearest Fiber that has one.
However, because we don't currently always clean up the map of
DevToolsInstances (e.g. updateComponentFilters doesn't recursively clean
out everything) it can leave matches hanging that shouldn't be there. So
we need to run the shouldFilterFiber filter to ignore those.
Another interesting implication is that without a FiberInstance we don't
have a way to get to a VirtualInstance from a HostComponent. Which means
that even filtered Fibers need to have a FiberInstance if they have a
VirtualInstance parent. Even if we don't actually mount them into the
front-end.
Stacked on #30494 and #30491.
This is setting us up to be able to track Server Components. This is now
split into a FiberInstance (Client Component) and a VirtualInstance
(Server Component). We're not actually creating any VirtualInstances yet
though this is just the data structures.
Server Components and potentially other compiled away or runtime
optimized away (e.g. calling through a function without creating an
intermediate Fiber) don't have a stateful instance. They just represent
the latest data. It's kind of like a React Element.
However, in the DevTools UI we need them to be stateful partly just so
that you can select and refer to them separately. E.g. the same Server
Component output rendered into different slots on the client should
still have two different representations in the DevTools. Also if the
same child Fibers update in place because the Server Component refreshed
we shouldn't lose the selection if you've selected a Server Component.
I'll implement this by creating Virtual Instances that only exist for
the purpose of the DevTools UI and so it'll be implemented in the
DevTools.
We could just make a Map from `id` to `Fiber | ReactComponentInfo` but
that requires a branching without a consistent hidden class. We also
need some more states on there. We also have some other Maps that tracks
extra states like that of component stacks, errors and warnings.
Constantly resizing and adding/removing from a Map isn't exactly fast.
It's faster to have a single Map with an object in it than one Map per
object. However, having extra fields that are usually just `null` can
instead mean more memory gets used. Since only a very small fraction of
instances will have errors/warnings or having initialized its component
stack, it made sense to store that in a separate Map that is usually
just empty.
However, with the addition of particularly the `parent` field and the
ability to do a fast hidden-class safe branching on the `kind` field I
think it's probably worth actually allocating an extra first class
object per Instance to store DevTools state into. That's why I converted
from just storing `Fiber` -> `id` to storing `Fiber` ->
`DevToolsInstance` which then keeps the warnings/errors/componentStack
as extra fields that are usually `null`. That is a lot of objects though
since it's one per Fiber pair basically.
Stacked on #30491.
When going from DOM Node to select a component or highlight a component
we find the nearest mounted ancestor. However, when multiple renderers
are nested there can be multiple ancestors. The original fix#24665 did
this by taking the inner renderer if it was an exact match but if it
wasn't it just took the first renderer.
Instead, we can track the inner most node we've found so far. Then get
the ID from that node (which will be fast since it's now a perfect
match). This is a better match.
However, the main reason I'm doing this is because the old mechanism
leaked the `Fiber` type outside the `RendererInterface` which is
supposed to abstract all of that. With the new algorithm this doesn't
leak.
I've tested this with a new build against the repro in the old issue
#24539 and it seems to work.
Stacked on #30490.
This is in the same spirit but to clarify the difference between what is
React Native vs part of any generic Host. We used to use "Native" to
mean three different concepts. Now "Native" just means React Native.
E.g. from the frontend's perspective the Host can be
Highlighted/Inspected. However, that in turn can then be implemented as
either direct DOM manipulation or commands to React Native. So frontend
-> backend is "Host" but backend -> React Native is "Native" while
backend -> DOM is "Web".
Rename NativeElementsPanel to BuiltinElementsPanel. This isn't a React
Native panel but one part of the surrounding DevTools. We refer to Host
more as the thing running React itself. I.e. where the backend lives.
The runtime you're inspecting. The DevTools itself needs a third term.
So I went with "Builtin".
I need to start clarifying where things are really actually Fibers and
where they're not since I'm adding Server Components as a separate type
of component instance which is not backed by a Fiber.
Nothing in the front end should really know anything about what kind of
renderer implementation we're inspecting and indeed it's already not
always a "Fiber" in the legacy renderer.
We typically refer to this as a "Component Instance" but the front end
currently refers to it as an Element as it historically grew from the
browser DevTools Elements tab.
I also moved the renderer.js implementation into the `backend/fiber`
folder. These are at the same level as `backend/legacy`. This clarifies
that anything outside of this folder ideally shouldn't refer to a
"Fiber".
console.js and profilingHooks.js unfortunately use Fibers a lot which
needs further refactoring. The profiler frontend also uses the term
alot.
We still filter them before passing from server to client in Flight
Server but when presenting a native stack, we don't need to filter them.
That's left to ignore listing in the presentation.
The stacks are pretty clean regardless thanks to the bottom stack
frames.
We can also unify the owner stack formatters into one shared module
since Fizz/Flight/Fiber all do the same thing. DevTools currently does
the same thing but is forked so it can support multiple versions.
Stacked on #30427.
Most hooks and such are called inside renders which already have these
on the stack but life-cycles that call out on them are useful to cut off
too.
Typically we don't create JSX in here so they wouldn't be part of owner
stacks anyway but they can be apart of plain stacks such as the ones
prefixes to console logs or printed by error dialogs.
This lets us cut off any React internals below. This should really be
possible using just ignore listing too ideally.
At this point we should maybe just build a Babel plugin that lets us
annotate a function to need to have this name.
The current stack is available in the native UI but that's hidden by
default so you don't see the actual current component on the stack.
This is unlike the native async stacks UI where they're all together.
So we prefix the stack with the current stack first.
<img width="279" alt="Screenshot 2024-07-22 at 10 05 13 PM"
src="https://github.com/user-attachments/assets/8f568fda-6493-416d-a0be-661caf44d808">
---------
Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>
This will allow us to parse new flow syntax since the `flow` parser is
no longer updated.
I had to exclude some files and have them fall back to `flow` parser
since they contain invalid graphql syntax that makes the plugin crash.
Stacked on #30410.
Use "owner stacks" as the appended component stack if it is available on
the Fiber. This will only be available if the enableOwnerStacks flag is
on. Otherwise it fallback to parent stacks. In prod, there's no owner so
it's never added there.
I was going back and forth on whether to inject essentially
`captureOwnerStack` as part of the DevTools hooks or replicate the
implementation but decided to replicate the implementation.
The DevTools needs all the same information from internals to implement
owner views elsewhere in the UI anyway so we're not saving anything in
terms of the scope of internals. Additionally, we really need this
information for non-current components as well like "rendered by" views
of the currently selected component.
It can also be useful if we need to change the format after the fact
like we did for parent stacks in:
https://github.com/facebook/react/pull/30289
Injecting the implementation would lock us into specifics both in terms
of what the core needs to provide and what the DevTools can use.
The implementation depends on the technique used in #30369 which tags
frames to strip out with `react-stack-bottom-frame`. That's how the
implementation knows how to materialize the error if it hasn't already.
Firefox:
<img width="487" alt="Screenshot 2024-07-21 at 11 33 37 PM"
src="https://github.com/user-attachments/assets/d3539b53-4578-4fdd-af25-25698b2bcc7d">
Follow up: One thing about this view is that it doesn't include the
current actual synchronous stack. When I used to append these I would
include both the real current stack and the owner stack. That's because
the owner stack doesn't include the name of the currently executing
component. I'll probably inject the current stack too in addition to the
owner stack. This is similar to how native Async Stacks are basically
just appended onto the current stack rather than its own.
Component stacks have a similar problem to the problem with keyPath
where we had to move it down and set it late right before recursing.
Currently we work around that by popping exactly one off when something
suspends. That doesn't work with the new server stacks being added which
are more than one. It also meant that we kept having add a single frame
that could be popped when there shouldn't need to be one.
Unlike keyPath component stacks has this weird property that once
something throws we might need the stack that was attempted for errors
or the previous stack if we're going to retry and just recreate it.
I've tried a few different approaches and I didn't like either but this
is the one that seems least problematic.
I first split out renderNodeDestructive into a retryNode helper. During
retries only retryNode is called. When we first discover a node, we pass
through renderNodeDestructive.
Instead of add a component stack frame deep inside renderNodeDestructive
after we've already refined a node, we now add it before in
renderNodeDestructive. That way it's only added once before being
attempted. This is similar to how Fiber works where in ChildFiber we
match the node once to create the instance and then later do we attempt
to actually render it and it's only the second part that's ever retried.
This unfortunately means that we now have to refine the node down to
element/lazy/thenables twice. To avoid refining the type too I move that
to be done lazily.
Before:
<img width="844" alt="Screenshot 2024-07-04 at 3 20 34 PM"
src="https://github.com/facebook/react/assets/63648/0fd8a53f-538a-4429-a4cf-c22f85a09aa8">
After:
<img width="845" alt="Screenshot 2024-07-05 at 6 08 28 PM"
src="https://github.com/facebook/react/assets/63648/7b9da13a-fa97-4581-9899-06de6fface65">
Firefox:
<img width="1338" alt="Screenshot 2024-07-05 at 6 09 50 PM"
src="https://github.com/facebook/react/assets/63648/f2eb9f2a-2251-408f-86d0-b081279ba378">
The first log doesn't get a stack because it's logged before DevTools
boots up and connects which is unfortunate.
The second log already has a stack printed by React (this is on stable)
it gets replaced by our object now.
The third and following logs don't have a stack and get one appended.
I only turn the stack into an error object if it matches what we would
emit from DevTools anyway. Otherwise we assume it's not React. Since I
had to change the format slightly to make this work, I first normalize
the stack slightly before doing a comparison since it won't be 1:1.
<!--
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
Fix how devtools handles URLs. It
- cannot handle relative source map URLs `//# sourceMappingURL=x.map`
- cannot recognize Windows style URLs
## How did you test this change?
works on my side
## Summary
When DevTools frontend and backend are connected, we patch console in 2
places:
- `patch()`, when renderer is attached to:
- listen to any errors / warnings emitted
- append component stack if requested by the user
- `patchForStrictMode()`, when React notifies about that the next
invocation is about to happed during StrictMode
`patchForStrictMode()` will always be at the top of the patch stack,
because it is called at runtime when React notifies React DevTools,
because of this, `patch()` may receive already modified arguments (with
stylings for dimming), we should attempt to restore the original
arguments
## How did you test this change?
Look at yellow warnings on the element view:
| Before | After |
| --- | --- |
| 
| 
|
## Summary
Sometimes `constructor` happens to be the name of an unrelated property,
or we may be dealing with a `Proxy` that intercepts every read. Verify
the constructor is a function before using its name, and reset the name
anyway if it turns out not to be serializable.
Fixes some cases of the devtools crashing and becoming inoperable upon
attempting to inspect components whose props are Hookstate `State`s.
## How did you test this change?
Installed a patched version of the extension and confirmed that it
solves the problem.
---------
Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>
## Summary
This is the pre-requisite for
https://github.com/facebook/react/pull/29231.
Current implementation of profiling hooks is only using
`performance.mark` and then makes `performance.clearMarks` call right
after it to free the memory. We've been relying on this assumption in
the tests that every mark is cleared by the time we check something.
https://github.com/facebook/react/pull/29231 adds `performance.measure`
calls and the `start` mark is not cleared until the corresponding `stop`
one is registered, and then they are cleared together.
## How did you test this change?
To test against React from source:
```
yarn test --build --project=devtools -r=experimental --ci
```
To test against React 18:
```
./scripts/circleci/download_devtools_regression_build.js 18.0 --replaceBuild
node ./scripts/jest/jest-cli.js --build --project devtools --release-channel=experimental --reactVersion 18.0 --ci
```
Stacked on https://github.com/facebook/react/pull/29869.
## Summary
When using ANSI escape sequences, we construct a message in the
following way: `console.<method>('\x1b...%s\x1b[0m',
userspaceArgument1?, userspaceArgument2?, userspaceArgument3?, ...)`.
This won't dim all arguments, if user had something like `console.log(1,
2, 3)`, we would only apply it to `1`, since this is the first
arguments, so we need to:
- inline everything whats possible into a single string, while
preserving console substitutions defined by the user
- omit css and object substitutions, since we can't really inline them
and will delegate in to the environment
## How did you test this change?
Added some tests, manually inspected that it works well for web and
native cases.
## Summary
Removes the usage of `consoleManagedByDevToolsDuringStrictMode` flag
from React DevTools backend, this is the only place in RDT where this
flag was used. The only remaining part is
[`ReactFiberDevToolsHook`](6708115937/packages/react-reconciler/src/ReactFiberDevToolsHook.js (L203)),
so React renderers can start notifying DevTools when `render` runs in a
Strict Mode.
> TL;DR: it is broken, and we already incorrectly apply dimming, when
RDT frontend is not opened. Fixing in the next few changes, see next
steps.
Before explaining why I am removing this, some context is required. The
way RDT works is slightly different, based on the fact if RDT frontend
and RDT backend are actually connected:
1. For browser extension case, the Backend is a script, which is
injected by the extension when page is loaded and before React is
loaded. RDT Frontend is loaded together with the RDT panel in browser
DevTools, so ONLY when user actually opens the RDT panel.
2. For native case, RDT backend is shipped together with `react-native`
for DEV bundles. It is always injected before React is loaded. RDT
frontend is loaded only when user starts a standalone RDT app via `npx
react-devtools` or by opening React Native DevTools and then selecting
React DevTools panel.
When Frontend is not connected to the Backend, the only thing we have is
the `__REACT_DEVTOOLS_GLOBAL_HOOK__` — this thing inlines some APIs in
itself, so that it can work similarly when RDT Frontend is not even
opened. This is especially important for console logs, since they are
cached and stored, then later displayed to the user once the Console
panel is opened, but from RDT side, you want to modify these console
logs when they are emitted.
In order to do so, we [inline the console patching logic into the
hook](3ac551e855/packages/react-devtools-shared/src/hook.js (L222-L319)).
This implementation doesn't use the
`consoleManagedByDevToolsDuringStrictMode`. This means that if we enable
`consoleManagedByDevToolsDuringStrictMode` for Native right now, users
would see broken dimming in LogBox / Metro logs when RDT Frontend is not
opened.
Next steps:
1. Align this console patching implementation with the one in `hook.js`.
2. Make LogBox compatible with console stylings: both css and ASCII
escape symbols.
3. Ship new version of RDT with these changes.
4. Remove `consoleManagedByDevToolsDuringStrictMode` from
`ReactFiberDevToolsHook`, so this is rolled out for all renderers.
This lets us rethrow it in the conceptual place of the child.
There's currently a problem when we suspend or throw in the child fiber
reconciliation phase. This work is done by the parent component, so if
it suspends or errors it is as if that component errored or suspended.
However, conceptually it's like a child suspended or errored.
In theory any thing can throw but it is really mainly due to either
`React.lazy` (both in the element.type position and node position),
`Thenable`s or the `Thenable`s that make up `AsyncIterable`s.
Mainly this happens because a Server Component that errors turns into a
`React.lazy`. In practice this means that if you have a Server Component
as the direct child of an Error Boundary. Errors inside of it won't be
caught.
We used to have the same problem with Thenables and Suspense but because
it's now always nested inside an inner Offscreen boundary that shields
it by being one level nested. However, when we have raw Offscreen
(Activity) boundaries they should also be able to catch the suspense if
it's in a hidden state so the problem returns. This fixes it for thrown
promises but it doesn't fix it for SuspenseException. I'm not sure this
is even the right strategy for Suspense though. It kind of relies on the
node never actually mounting/committing.
It's conceptually a little tricky because the current component can
inspect the children and make decisions based on them. Such as
SuspenseList.
The other thing that this PR tries to address is that it sets the
foundation for dealing with error reporting for Server Components that
errored. If something client side errors it'll be a stack like Server
(DebugInfo) -> Fiber -> Fiber -> Server -> (DebugInfo) -> Fiber.
However, all error reporting relies on it eventually terminating into a
Fiber that is responsible for the error. To avoid having to fork too
much it would be nice if I could create a Fiber to associate with the
error so that even a Server component error in this case ultimately
terminates in a Fiber.
Basically make `console.error` and `console.warn` behave like normal -
when a component stack isn't appended. I need this because I need to be
able to print rich logs with the component stack option and to be able
to disable instrumentation completely in `console.createTask`
environments that don't need it.
Currently we can't print logs with richer objects because they're
toString:ed first. In practice, pretty much all arguments we log are
already toString:ed so it's not necessary anyway. Some might be like a
number. So it would only be a problem if some environment can't handle
proper consoles but then it's up to that environment to toString it
before logging.
The `Warning: ` prefix is historic and is both noisy and confusing. It's
mostly unnecessary since the UI surrounding `console.error` and
`console.warn` tend to have visual treatment around it anyway. However,
it's actively misleading when `console.error` gets prefixed with a
Warning that we consider an error level. There's an argument to be made
that some of our `console.error` don't make the bar for an error but
then the argument is to downgrade each of those to `console.warn` - not
to brand all our actual error logging with `Warning: `.
Apparently something needs to change in React Native before landing this
because it depends on the prefix somehow which probably doesn't make
sense already.
## Overview
Updates `eslint-plugin-jest` and enables the recommended rules with some
turned off that are unhelpful.
The main motivations is:
a) we have a few duplicated tests, which this found an I deleted
b) making sure we don't accidentally commit skipped tests
This refactors key warning to happen inline after we've matched a Fiber.
I didn't want to do that originally because it was riskier. But it turns
out to be straightforward enough.
This lets us use that Fiber as the source of the warning which matters
to DevTools because then DevTools can associate it with the right
component after it mounts.
We can also associate the duplicate key warning with this Fiber. That
way we'll get the callsite with the duplicate key on the stack and can
associate this warning with the child that had the duplicate.
I kept the forked DevTools tests because the warning now is counted on
the Child instead of the Parent (18 behavior).
However, this won't be released in 19.0.0 so I only test this in
whatever the next version is.
Doesn't seem worth it to have a test for just the 19.0.0 behavior.
## Summary
The test started to fail after
https://github.com/facebook/react/pull/29088.
Fork the test and the expected store state for:
- React 18.x, to represent the previous behavior
- React >= 19, to represent the current RDT behavior, where error can't
be connected to the fiber, because it was not yet mounted and shared
with DevTools.
Ideally, DevTools should start keeping track of such fibers, but also
distinguish them from some that haven't mounted due to Suspense or error
boundaries.
Stacked on #29206 and #29221.
This disables appending owner stacks to console when
`console.createTask` is available in the environment. Instead we rely on
native "async" stacks that end up looking like this with source maps and
ignore list enabled.
<img width="673" alt="Screenshot 2024-05-22 at 4 00 27 PM"
src="https://github.com/facebook/react/assets/63648/5313ed53-b298-4386-8f76-8eb85bdfbbc7">
Unfortunately Chrome requires a string name for each async stack and,
worse, a suffix of `(async)` is automatically added which is very
confusing since it seems like it might be an async component or
something which it is not.
In this case it's not so bad because it's nice to refer to the host
component which otherwise doesn't have a stack frame since it's
internal. However, if there were more owners here there would also be a
`<Counter> (async)` which ends up being kind of duplicative.
If the Chrome DevTools is not open from the start of the app, then
`console.createTask` is disabled and so you lose the stack for those
errors (or those parents if the devtools is opened later). Unlike our
appended ones that are always added. That's unfortunate and likely to be
a bit of a DX issue but it's also nice that it saves on perf in DEV mode
for those cases. Framework dialogs can still surface the stack since we
also track it in user space in parallel.
This currently doesn't track Server Components yet. We need a more
clever hack for that part in a follow up.
I think I probably need to also add something to React DevTools to
disable its stacks for this case too. Since it looks for stacks in the
console.error and adds a stack otherwise. Since we don't add them
anymore from the runtime, the DevTools adds them instead.
This is necessary to simplify the component stack handling to make way
for owner stacks. It also solves some hacks that we used to have but
don't quite make sense. It also solves the problem where things like key
warnings get silenced in RSC because they get deduped. It also surfaces
areas where we were missing key warnings to begin with.
Almost every type of warning is issued from the renderer. React Elements
are really not anything special themselves. They're just lazily invoked
functions and its really the renderer that determines there semantics.
We have three types of warnings that previously fired in
JSX/createElement:
- Fragment props validation.
- Type validation.
- Key warning.
It's nice to be able to do some validation in the JSX/createElement
because it has a more specific stack frame at the callsite. However,
that's the case for every type of component and validation. That's the
whole point of enableOwnerStacks. It's also not sufficient to do it in
JSX/createElement so we also have validation in the renderers too. So
this validation is really just an eager validation but also happens
again later.
The problem with these is that we don't really know what types are valid
until we get to the renderer. Additionally, by placing it in the
isomorphic code it becomes harder to do deduping of warnings in a way
that makes sense for that renderer. It also means we can't reuse logic
for managing stacks etc.
Fragment props validation really should just be part of the renderer
like any other component type. This also matters once we add Fragment
refs and other fragment features. So I moved this into Fiber. However,
since some Fragments don't have Fibers, I do the validation in
ChildFiber instead of beginWork where it would normally happen.
For `type` validation we already do validation when rendering. By
leaving it to the renderer we don't have to hard code an extra list.
This list also varies by context. E.g. class components aren't allowed
in RSC but client references are but we don't have an isomorphic way to
identify client references because they're defined by the host config so
the current logic is flawed anyway. I kept the early validation for now
without the `enableOwnerStacks` since it does provide a nicer stack
frame but with that flag on it'll be handled with nice stacks anyway. I
normalized some of the errors to ensure tests pass.
For `key` validation it's the same principle. The mechanism for the
heuristic is still the same - if it passes statically through a parent
JSX/createElement call then it's considered validated. We already did
print the error later from the renderer so this also disables the early
log in the `enableOwnerStacks` flag.
I also added logging to Fizz so that key warnings can print in SSR logs.
Flight is a bit more complex. For elements that end up on the client we
just pass the `validated` flag along to the client and let the client
renderer print the error once rendered. For server components we log the
error from Flight with the server component as the owner on the stack
which will allow us to print the right stack for context. The factoring
of this is a little tricky because we only want to warn if it's in an
array parent but we want to log the error later to get the right debug
info.
Fiber/Fizz has a similar factoring problem that causes us to create a
fake Fiber for the owner which means the logs won't be associated with
the right place in DevTools.
This lets us expose the component stack to the error reporting that
happens here as `console.error` patching. Now if you just call
`console.error` in the error handlers it'll get the component stack
added to the end by React DevTools.
However, unfortunately this happens a little too late so the Fiber will
be disconnected with its `.return` pointer set to null already. So it'll
be too late to extract a parent component stack from but you can at
least get the stack from source to error boundary. To work around this I
manually add the parent component stack in our default handlers when
owner stacks are off. We could potentially fix this but you can also
just include it yourself if you're calling `console.error` and it's not
a problem for owner stacks.
This is not a problem for owner stacks because we'll still have those
and so for those just calling `console.error` just works. However, the
main feature is that by letting React add them, we can switch to using
native error stacks when available.
We previously had two slightly different concepts for "current fiber".
There's the "owner" which is set inside of class components in prod if
string refs are enabled, and sometimes inside function components in DEV
but not other contexts.
Then we have the "current fiber" which is only set in DEV for various
warnings but is enabled in a bunch of contexts.
This unifies them into a single "current fiber".
The concept of string refs shouldn't really exist so this should really
be a DEV only concept. In the meantime, this sets the current fiber
inside class render only in prod, however, in DEV it's now enabled in
more contexts which can affect the string refs. That was already the
case that a string ref in a Function component was only connecting to
the owner in prod. Any string ref associated with any non-class won't
work regardless so that's not an issue. The practical change here is
that an element with a string ref created inside a life-cycle associated
with a class will work in DEV but not in prod. Since we need the current
fiber to be available in more contexts in DEV for the debugging
purposes. That wouldn't affect any old code since it would have a broken
ref anyway. New code shouldn't use string refs anyway.
The other implication is that "owner" doesn't necessarily mean
"rendering" since we need the "owner" to track other debug information
like stacks - in other contexts like useEffect, life cycles, etc.
Internally we have a separate `isRendering` flag that actually means
we're rendering but even that is a very overloaded concept. So anything
that uses "owner" to imply rendering might be wrong with this change.
This is a first step to a larger refactor for tracking current rendering
information.
---------
Co-authored-by: Sebastian Silbermann <silbermann.sebastian@gmail.com>
## Summary
- While rolling out RDT 5.2.0 on Fusebox, we've discovered that context
menus don't work well with this environment. The reason for it is the
context menu state implementation - in a global context we define a map
of registered context menus, basically what is shown at the moment (see
deleted Contexts.js file). These maps are not invalidated on each
re-initialization of DevTools frontend, since the bundle
(react-devtools-fusebox module) is not reloaded, and this results into
RDT throwing an error that some context menu was already registered.
- We should not keep such data in a global state, since there is no
guarantee that this will be invalidated with each re-initialization of
DevTools (like with browser extension, for example).
- The new implementation is based on a `ContextMenuContainer` component,
which will add all required `contextmenu` event listeners to the
anchor-element. This component will also receive a list of `items` that
will be displayed in the shown context menu.
- The `ContextMenuContainer` component is also using
`useImperativeHandle` hook to extend the instance of the component, so
context menus can be managed imperatively via `ref`:
`contextMenu.current?.hide()`, for example.
- **Changed**: The option for copying value to clipboard is now hidden
for functions. The reasons for it are:
- It is broken in the current implementation, because we call
`JSON.stringify` on the value, see
`packages/react-devtools-shared/src/backend/utils.js`.
- I don't see any reasonable value in doing this for the user, since `Go
to definition` option is available and you can inspect the real code and
then copy it.
- We already filter out fields from objects, if their value is a
function, because the whole object is passed to `JSON.stringify`.
## How did you test this change?
### Works with element props and hooks:
- All context menu items work reliably for props items
- All context menu items work reliably or hooks items
https://github.com/facebook/react/assets/28902667/5e2d58b0-92fa-4624-ad1e-2bbd7f12678f
### Works with timeline profiler:
- All context menu items work reliably: copying, zooming, ...
- Context menu automatically closes on the scroll event
https://github.com/facebook/react/assets/28902667/de744cd0-372a-402a-9fa0-743857048d24
### Works with Fusebox:
- Produces no errors
- Copy to clipboard context menu item works reliably
https://github.com/facebook/react/assets/28902667/0288f5bf-0d44-435c-8842-6b57bc8a7a24
Follow-up to https://github.com/facebook/react/pull/28813.
RDT is using `typeOf` from `react-is` to determine the element display
name, I've forked an implementation of this method, but will be using
legacy element symbol.
This PR reorganizes the `react-dom` entrypoint to only pull in code that
is environment agnostic. Previously if you required anything from this
entrypoint in any environment the entire client reconciler was loaded.
In a prior release we added a server rendering stub which you could
alias in server environments to omit this unecessary code. After landing
this change this entrypoint should not load any environment specific
code.
While a few APIs are truly client (browser) only such as createRoot and
hydrateRoot many of the APIs you import from this package are only
useful in the browser but could concievably be imported in shared code
(components running in Fizz or shared components as part of an RSC app).
To avoid making these require opting into the client bundle we are
keeping them in the `react-dom` entrypoint and changing their
implementation so that in environments where they are not particularly
useful they do something benign and expected.
#### Removed APIs
The following APIs are being removed in the next major. Largely they
have all been deprecated already and are part of legacy rendering modes
where concurrent features of React are not available
* `render`
* `hydrate`
* `findDOMNode`
* `unmountComponentAtNode`
* `unstable_createEventHandle`
* `unstable_renderSubtreeIntoContainer`
* `unstable_runWithPrioirty`
#### moved Client APIs
These APIs were available on both `react-dom` (with a warning) and
`react-dom/client`. After this change they are only available on
`react-dom/client`
* `createRoot`
* `hydrateRoot`
#### retained APIs
These APIs still exist on the `react-dom` entrypoint but have normalized
behavior depending on which renderers are currently in scope
* `flushSync`: will execute the function (if provided) inside the
flushSync implemention of FlightServer, Fizz, and Fiber DOM renderers.
* `unstable_batchedUpdates`: This is a noop in concurrent mode because
it is now the only supported behavior because there is no legacy
rendering mode
* `createPortal`: This just produces an object. It can be called from
anywhere but since you will probably not have a handle on a DOM node to
pass to it it will likely warn in environments other than the browser
* preloading APIS such as `preload`: These methods will execute the
preload across all renderers currently in scope. Since we resolve the
Request object on the server using AsyncLocalStorage or the current
function stack in practice only one renderer should act upon the
preload.
In addition to these changes the server rendering stub now just rexports
everything from `react-dom`. In a future minor we will add a warning
when using the stub and in the next major we will remove the stub
altogether
We have changed the shape (and the runtime) of React Elements. To help
avoid precompiled or inlined JSX having subtle breakages or deopting
hidden classes, I renamed the symbol so that we can early error if
private implementation details are used or mismatching versions are
used.
Why "transitional"? Well, because this is not the last time we'll change
the shape. This is just a stepping stone to removing the `ref` field on
the elements in the next version so we'll likely have to do it again.
## Summary
The Forget codename needs to be hidden from the UI to avoid confusion.
Going forward, we'll be referring to this set of features as part of the
larger React compiler. We'll be describing the primary feature that
we've built so far as auto-memoization, and this badge helps devs see
which components have been automatically memoized by the compiler.
## How did you test this change?
- force Forget badge on with and without the presence of other badges
- confirm colors/UI in light and dark modes
- force badges on for `ElementBadges`, `InspectableElementBadges`,
`IndexableElementBadges`
- Running yarn start in packages/react-devtools-shell
[demo
video](https://github.com/facebook/react/assets/973058/fa829018-7644-4425-8395-c5cd84691f3c)
The useMemoCache polyfill doesn't have access to the fiber, and it
simply uses state, which does not work with the existing devtools
badge for the compiler.
With this PR, devtools will look on the very first hook's state for the
memo cache sentinel and display the Forget badge if present.
The polyfill will add this sentinel to it's state (the cache array).
## Summary
Stacked on https://github.com/facebook/react/pull/28552. Review only the
[last commit at the
top](c69952f1bf).
These changes add new package `react-devtools-fusebox`, which is the
entrypoint for the RDT Frontend, which will be used in Chrome DevTools
panel. The main differences from other frontend shells (extension,
standalone) are:
1. This package builds scripts in ESM format, this is required by Chrome
DevTools, see webpack config:
c69952f1bf/packages/react-devtools-fusebox/webpack.config.frontend.js (L50-L52)
2. The build includes styles in a separate `.css` file, which is
required for Chrome DevTools: styles are loaded lazily once panel is
mounted.
Follow up to #28783 and #28786.
Since we've changed the implementations of these we can rename them to
something a bit more descriptive while we're at it, since anyone
depending on them will need to upgrade their code anyway.
"react" with no condition:
`__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE`
"react" with "react-server" condition:
`__SERVER_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE`
"react-dom":
`__DOM_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE`
This is similar to #28771 but for isomorphic. We need a make over for
these dispatchers anyway so this is the first step. Also helps flush out
some internals usage that will break anyway.
It flattens the inner mutable objects onto the ReactSharedInternals.
## Summary
1. RDT browser extension's content scripts will now ship source maps
(without source in prod, to save some bundle size).
2. `installHook` content script will be ignore listed via `ignoreList`
field in the corresponding source map.
3. Previously, source map for backend file used `x_google_ignoreList`
naming, now `ignoreList`.
## How did you test this change?
1. `ignoreList-test.js`
2. Tested manually that I don't see `installHook` in stack traces when
`console.error` is called.
This implements the concept of a DEV-only "owner" for Server Components.
The owner concept isn't really super useful. We barely use it anymore,
but we do have it as a concept in DevTools in a couple of cases so this
adds it for parity. However, this is mainly interesting because it could
be used to wire up future owner-based stacks.
I do this by outlining the DebugInfo for a Server Component
(ReactComponentInfo). Then I just rely on Flight deduping to refer to
that. I refer to the same thing by referential equality so that we can
associate a Server Component parent in DebugInfo with an owner.
If you suspend and replay a Server Component, we have to restore the
same owner. To do that, I did a little ugly hack and stashed it on the
thenable state object. Felt unnecessarily complicated to add a stateful
wrapper for this one dev-only case.
The owner could really be anything since it could be coming from a
different implementation. Because this is the first time we have an
owner other than Fiber, I have to fix up a bunch of places that assumes
Fiber. I mainly did the `typeof owner.tag === 'number'` to assume it's a
Fiber for now.
This also doesn't actually add it to DevTools / RN Inspector yet. I just
ignore them there for now.
Because Server Components can be async the owner isn't tracked after an
await. We need per-component AsyncLocalStorage for that. This can be
done in a follow up.
This PR relands #28672 on top of the flag removal and the test
demonstrating a breakage in Suspense for legacy mode.
React has deprecated module pattern Function Components for many years
at this point. Supporting this pattern required React to have a concept
of an indeterminate component so that when a component first renders it
can turn into either a ClassComponent or a FunctionComponent depending
on what it returns. While this feature was deprecated and put behind a
flag it is still in stable. This change remvoes the flag, removes the
warnings, and removes the concept of IndeterminateComponent from the
React codebase.
While removing IndeterminateComponent type Seb and I discovered that we
needed a concept of IncompleteFunctionComponent to support Suspense in
legacy mode. This new work tag is only needed as long as legacy mode is
around and ideally any code that considers this tag will be excludable
from OSS builds once we land extra gates using `disableLegacyMode` flag.
This breaks internal tests, so must be something in the refactor. Since
it's the top commit let's revert and split into two PRs, one that
removes the flag and one that does the refactor, so we can find the bug.
The module pattern
```
function MyComponent() {
return {
render() {
return this.state.foo
}
}
}
```
has been deprecated for approximately 5 years now. This PR removes
support for this pattern. It also simplifies a number of code paths in
particular related to the concept of `IndeterminateComponent` types.
## Summary
Based on
- https://github.com/facebook/react/pull/27903
This PR
- Silence warning in React tests
- Turn on flag
We want to finish cleaning up internal RTR usage, but let's prioritize
the deprecation process. We do this by silencing the internal warning
for now.
## How did you test this change?
`yarn build`
`yarn test ReactHooksInspectionIntegration -b`
## Overview
The error messages that say:
> ReactDOM.hydrate is no longer supported in React 18
Don't make sense in the React 19 release. Instead, they should say:
> ReactDOM.hydrate was removed in React 19.
For legacy mode, they should say:
> ReactDOM.hydrate has not been supported since React 18.
Reverting some of https://github.com/facebook/react/pull/27804 which
renamed this option to stable. This PR just replaces internal usage to
make upcoming PRs cleaner.
Keeping isConcurrent unstable for the next major release in order to
enable a broader deprecation of RTR and be consistent with concurrent
rendering everywhere for next major.
(https://github.com/facebook/react/pull/28498)
- Next major will use concurrent root
- The old behavior (legacy root by default, concurrent root with
unstable option) will be preserved for React Native until new
architecture is fully shipped.
- Flag and legacy root usage can be removed after RN dependency is
unblocked without an additional breaking change
We broke the ability to "break on uncaught exceptions" by adding a
try/catch higher up in the scheduling. We're giving up on fixing that so
we can remove the replay trick inside an event handler.
The issue with that approach is that we end up double logging a lot of
errors in DEV since they get reported to the page.
It's also a lot of complexity around this feature.
Stacked on https://github.com/facebook/react/pull/28351, please review
only the last commit.
Top-level description of the approach:
1. Once user selects an element from the tree, frontend asks backend to
return the inspected element, this is where we simulate an error
happening in `render` function of the component and then we parse the
error stack. As an improvement, we should probably migrate from custom
implementation of error stack parser to `error-stack-parser` from npm.
2. When frontend receives the inspected element and this object is being
propagated, we create a Promise for symbolicated source, which is then
passed down to all components, which are using `source`.
3. These components use `use` hook for this promise and are wrapped in
Suspense.
Caching:
1. For browser extension, we cache Promises based on requested resource
+ key + column, also added use of
`chrome.devtools.inspectedWindow.getResource` API.
2. For standalone case (RN), we cache based on requested resource url,
we cache the content of it.
`_debugSource` was removed in
https://github.com/facebook/react/pull/28265.
This PR migrates DevTools to define `source` for Fiber based on
component stacks. This will be done lazily for inspected elements, once
user clicks on the element in the tree.
`DevToolsComponentStackFrame.js` was just copy-pasted from the
implementation in `ReactComponentStackFrame`.
Symbolication part is done in
https://github.com/facebook/react/pull/28471 and stacked on this commit.
There is a weird behaviour in all shells of RDT: when user opens
`Components` tab and scrolls down a tree (without any prior click or
focus event), and then clicks on some element, the `click` event will
not be fired. Because `click` event hasn't been fired, the `focus` event
is fired for the whole list and we pre-select the first (root) element
in the tree:
034130c02f/packages/react-devtools-shared/src/devtools/views/Components/Tree.js (L217-L226)
Check the demo (before) what is happening. I don't know exactly why
`click` event is not fired there, but it only happens:
1. For elements, which were not previously rendered (for virtualization
purposes).
2. When HTML-element (div), which represents the container for the tree
was not focused previously.
Unlike the `click` event, the `mousedown` event is fired consistently.
### Before
https://github.com/facebook/react/assets/28902667/9f3ad75d-55d0-4c99-b2d0-ead63a120ea0
### After
https://github.com/facebook/react/assets/28902667/e34816be-644c-444c-8e32-562a79494e44
Tested that it works in all shells, including the select / deselect
features (with `metaKey` param in event).
Adds a flag to disable legacy mode. Currently this flag is used to cause
legacy mode apis like render and hydrate to throw. This change also
removes render, hydrate, unmountComponentAtNode, and
unstable_renderSubtreeIntoContainer from the experiemntal entrypoint.
Right now for Meta builds this flag is off (legacy mode is still
supported). In OSS builds this flag matches __NEXT_MAJOR__ which means
it currently is on in experiemental. This means that after merging
legacy mode is effectively removed from experimental builds. While this
is a breaking change, experimental builds are not stable and users can
pin to older versions or update their use of react-dom to no longer use
legacy mode APIs.
I'm a bit ambivalent about this one because it's not the main strategy
that I plan on pursuing. I plan on replacing most DEV-only specific
stacks like `console.error` stacks with a new take on owner stacks and
native stacks. The future owner stacks may or may not be exposed to
error boundaries in DEV but if they are they'd be a new errorInfo
property since they're owner based and not available in prod.
The use case in `console.error` mostly goes away in the future so this
PR is mainly for error boundaries. It doesn't hurt to have it in there
while I'm working on the better stacks though.
The `componentStack` property exposed to error boundaries is more like
production behavior similar to `new Error().stack` (which even in DEV
won't ever expose owner stacks because `console.createTask` doesn't
affect these). I'm not sure it's worth adding server components in DEV
(this PR) because then you have forked behavior between dev and prod.
However, since even in the future there won't be any other place to get
the *parent* stack, maybe this can be useful information even if it's
only dev. We could expose a third property on errorInfo that's DEV only
and parent stack but including server components. That doesn't seem
worth it over just having the stack differ in dev and prod.
I don't plan on adding line/column number to these particular stacks.
A follow up could be to add this to Fizz prerender too but only in DEV.
Following https://github.com/facebook/react/pull/28265, this should
disable location-based component filters.
```
// Following __debugSource removal from Fiber, the new approach for finding the source location
// of a component, represented by the Fiber, is based on lazily generating and parsing component stack frames
// To find the original location, React DevTools will perform symbolication, source maps are required for that.
// In order to start filtering Fibers, we need to find location for all of them, which can't be done lazily.
// Eager symbolication can become quite expensive for large applications.
```
I am planning to publish a patch version of RDT soon, so I think its
better to remove this feature, instead of shipping it in a broken state.
The reason for filtering out these filters is a potential cases, where
we load filters from the backend (like in RN, where we storing some
settings on device), or these filters can be stored in user land
(`window.__REACT_DEVTOOLS_COMPONENT_FILTERS__`).
Explicitly tested the case when:
1. Load current RDT extension, add location-based component filter
2. Reload the page and observe that previously created component filter
is preserved
3. Re-load RDT extension with these changes, observe there is no
previously created component filter and user can't create a new
location-based filter
4. Reload RDT extension without these changes, no location-based filters
saved, user can create location-based filters
1. Bumps `react-virtualized-auto-sizer` to 1.0.23, which has a fix for
cases with multiple realms -
https://github.com/bvaughn/react-virtualized-auto-sizer/pull/82
2. Removes `react-window` from react-devtools-shared/src/node_modules,
now listed as dependency in `package.json` and bumped to 1.8.10
Tested:
- Chrome extension
- Standalone shell with RN
Depends on:
- #28317
- #28320
---
Changes the behavior of the JSX runtime to pass through `ref` as a
normal prop, rather than plucking it from the props object and storing
on the element.
This is a breaking change since it changes the type of the receiving
component. However, most code is unaffected since it's unlikely that a
component would have attempted to access a `ref` prop, since it was not
possible to get a reference to one.
`forwardRef` _will_ still pluck `ref` from the props object, though,
since it's extremely common for users to spread the props object onto
the inner component and pass `ref` as a differently named prop. This is
for maximum compatibility with existing code — the real impact of this
change is that `forwardRef` is no longer required.
Currently, refs are resolved during child reconciliation and stored on
the fiber. As a result of this change, we can move ref resolution to
happen only much later, and only for components that actually use them.
Then we can remove the `ref` field from the Fiber type. I have not yet
done that in this step, though.
Depends on:
- #28317
---
There's a ton of overlap between the createElement implementation and
the JSX implementation, so I combined them into a single module.
In the actual build output, the shared code between JSX and
createElement will get duplicated anyway, because react/jsx-runtime and
react (where createElement lives) are separate, flat build artifacts.
So this is more about code organization — with a few key exceptions, the
implementations of createElement and jsx are highly coupled.
This option was added defensively but it's not needed. There's no cost
to including it always.
I suspect this optional was added mainly to avoid needing to update
tests. That's not a reason to have an unnecessary public API though.
We have a praxis for dealing with source location in tests to avoid them
failing tests. I also ported them to inline snapshots so that additions
to the protocol isn't such a pain.
Previously, `<Context>` was equivalent to `<Context.Consumer>`. However,
since the introduction of Hooks, the `<Context.Consumer>` API is rarely
used. The goal here is to make the common case cleaner:
```js
const ThemeContext = createContext('light')
function App() {
return (
<ThemeContext value="dark">
...
</ThemeContext>
)
}
function Button() {
const theme = use(ThemeContext)
// ...
}
```
This is technically a breaking change, but we've been warning about
rendering `<Context>` directly for several years by now, so it's
unlikely much code in the wild depends on the old behavior. [Proof that
it warns today (check
console).](https://codesandbox.io/p/sandbox/peaceful-nobel-pdxtfl)
---
**The relevant commit is 5696782b428a5ace96e66c1857e13249b6c07958.** It
switches `createContext` implementation so that `Context.Provider ===
Context`.
The main assumption that changed is that a Provider's fiber type is now
the context itself (rather than an intermediate object). Whereas a
Consumer's fiber type is now always an intermediate object (rather than
it being sometimes the context itself and sometimes an intermediate
object).
My methodology was to start with the relevant symbols, work tags, and
types, and work my way backwards to all usages.
This might break tooling that depends on inspecting React's internal
fields. I've added DevTools support in the second commit. This didn't
need explicit versioning—the structure tells us enough.
Alternative to #28295.
Instead of stashing all of the Usables eagerly, we can extract them by
replaying the render when we need them like we do with any other hook.
We already had an implementation of `use()` but it wasn't quite
complete.
These can also include further DebugInfo on them such as what Server
Component rendered the Promise or async debug info. This is nice just to
see which use() calls were made in the side-panel but it can also be
used to gather everything that might have suspended.
Together with https://github.com/facebook/react/pull/28286 we cover the
case when a Promise was used a child and if it was unwrapped with use().
Notably we don't cover a Promise that was thrown (although we do support
that in a Server Component which maybe we shouldn't). Throwing a Promise
isn't officially supported though and that use case should move to the
use() Hook.
The pattern of conditionally suspending based on cache also isn't really
supported with the use() pattern. You should always call use() if you
previously called use() with the same input. This also ensures that we
can track what might have suspended rather than what actually did.
One limitation of this strategy is that it's hard to find all the places
something might suspend in a tree without rerendering all the fibers
again. So we might need to still add something to the tree to indicate
which Fibers may have further debug info / thenables.
In our custom implementation for handling modals dismiss signal, we use
element's `ownerDocument` field, which expectedly doesn't work well with
shadow root. Now using
[`getRootNode`](https://developer.mozilla.org/en-US/docs/Web/API/Node/getRootNode)
instead of `ownerDocument` to support shadow root case.
Without this, if RDT Frontend is hosted inside the shadow root, the
modal gets closed after any click, including on the buttons hosted by
modal:
00d42ac354/packages/react-devtools-shared/src/devtools/views/hooks.js (L228-L238)
Test plan:
- Modals work as expected for Chrome DevTools integration
- Modals work as expected at every other surfaces: browser extension,
electron wrapper for RN, inline version for web
Along with all the places using it like the `_debugSource` on Fiber.
This still lets them be passed into `createElement` (and JSX dev
runtime) since those can still be used in existing already compiled code
and we don't want that to start spreading to DOM attributes.
We used to have a DEV mode that compiles the source location of JSX into
the compiled output. This was nice because we could get the actual call
site of the JSX (instead of just somewhere in the component). It had a
bunch of issues though:
- It only works with JSX.
- The way this source location is compiled is different in all the
pipelines along the way. It relies on this transform being first and the
source location we want to extract but it doesn't get preserved along
source maps and don't have a way to be connected to the source hosted by
the source maps. Ideally it should just use the mechanism other source
maps use.
- Since it's expensive it only works in DEV so if it's used for
component stacks it would vary between dev and prod.
- It only captures the callsite of the JSX and not the stack between the
component and that callsite. In the happy case it's in the component but
not always.
Instead, we have another zero-cost trick to extract the call site of
each component lazily only if it's needed. This ensures that component
stacks are the same in DEV and PROD. At the cost of worse line number
information.
The better way to get the JSX call site would be to get it from `new
Error()` or `console.createTask()` inside the JSX runtime which can
capture the whole stack in a consistent way with other source mappings.
We might explore that in the future.
This removes source location info from React DevTools and React Native
Inspector. The "jump to source code" feature or inspection can be made
lazy instead by invoking the lazy component stack frame generation. That
way it can be made to work in prod too. The filtering based on file path
is a bit trickier.
When redesigned this UI should ideally also account for more than one
stack frame.
With this change the DEV only Babel transforms are effectively
deprecated since they're not necessary for anything.
Adding getter-functions for renderer implementations, which can be used
for jest tests. If we are testing against React with version < 18, we
are going to use legacy rendering, otherwise the concurrent one.
- Moving `act` implementation to a single getter-function, which is
based on React version we are testing RDT against.
- Removing unused mocks for `act`, which were designed for legacy
versions of React, validated with running tests against React 16 build.
Starting in version 19, users can import the `act` testing API from the
`react` package instead of using a renderer specific API, like
`react-dom/test-utils`.
This Flow upgrade includes 2 fixes:
- Remove `React$StatelessFunctionalComponent` as that was replaced by
just `React$AbstractComponent` as Flow doesn't make any guarantees, see
the Flow change here:
521317c48f
- Flow no longer allows `number` type indexing into objects which
discovered an incorrect type that is actually an array of the data.
Used this command to upgrade
```
yarn add -W flow-bin flow-remove-types hermes-parser hermes-eslint
```
and ran `yarn flow-ci` to check for errors in different configurations.
https://github.com/facebook/react/pull/27805 broke integration tests for
React DevTools with React 17, these changes introduce a fallback for
such case when `act` is not available in `react`, but available in
`react-dom`, like before.
## Summary
Concurrent rendering has been the default since React 18 release.
ReactTestRenderer requires passing `{unstable_isConcurrent: true}` to
match this behavior, which means by default tests written with RTR use a
different rendering method than the code they test.
Eventually, RTR should only use ConcurrentRoot. As a first step, let's
add a version of the concurrent option that isn't marked unstable. Next
we will follow up with removing the unstable option when it is safe to
merge.
## How did you test this change?
`yarn test
packages/react-test-renderer/src/__tests__/ReactTestRendererAsync-test.js`
As part of the process of removing the deprecated `react-dom/test-utils`
package references to `act` from this module are replaced with
references to `unstable_act` in `react`. It is likely that the unstable
act implementation will be made stable. The test utils act is just a
reexport of the unstable_act implementation in react itself.
Adds `Forget` badge to all relevant components.
Changes:
- If component is compiled with Forget and using a built-in
`useMemoCache` hook, it will have a `Forget` badge next to its display
name in:
- components tree
- inspected element view
- owners list
- Such badges are indexable, so Forget components can be searched using
search bar.
Fixes:
- Displaying the badges for owners list inside the inspected component
view
Implementation:
- React DevTools backend is responsible for identifying if component is
compiled with Forget, based on `fiber.updateQueue.memoCache`. It will
wrap component's display name with `Forget(...)` prefix before passing
operations to the frontend. On the frontend side, we will parse the
display name and strip Forget prefix, marking the corresponding element
by setting `compiledWithForget` field. Almost the same logic is
currently used for HOC display names.
We are currently just pass the first element, which diverges from the
implementation for web. This is especially bad if you are inspecting
something like a list, where host fiber can represent multiple elements.
This part runs on the backend of React DevTools, so it should not affect
cases for React Native when frontend version can be more up-to-date than
backend's. I will double-check it before merging.
Once version of `react-devtools-core` is updated in React Native, this
should be supported, I will work on that later.
Had these stashed for some time, it includes:
- Some refactoring to remove unnecessary `FlowFixMe`s and type castings
via `any`.
- Optimized version of parsing component names. We encode string names
to utf8 and then pass it serialized from backend to frontend in a single
array of numbers. Previously we would call `slice` to get the
corresponding encoded string as a subarray and then parse each
character. New implementation skips `slice` step and just receives
`left` and `right` ranges for the string to parse.
- Early `break` instead of `continue` when Store receives unexpected
operation, like removing an element from the Store, which is not
registered yet.
Updates useFormState to allow a sync function to be passed as an action.
A form action is almost always async, because it needs to talk to the
server. But since we support client-side actions, too, there's no reason
we can't allow sync actions, too.
I originally chose not to allow them to keep the implementation simpler
but it's not really that much more complicated because we already
support this for actions passed to startTransition. So now it's
consistent: anywhere an action is accepted, a sync client function is a
valid input.
<!--
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
Updated the typo in test description
<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->
## How did you test this change?
NA, correct test description improves readability of the code and
confusion for anyone who is new to the codebase.
<!--
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.
-->
