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.
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">
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.
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.
[`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.
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.
## 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
## 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)
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.
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).
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
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
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.
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.
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.
There are not so many changes, most of them are changing imports,
because I've moved types for UI in a single file.
In https://github.com/facebook/react/pull/27357 I've added support for
pausing polling events: when user inspects an element, we start polling
React DevTools backend for updates in props / state. If user switches
tabs, extension's service worker can be killed by browser and this
polling will start spamming errors.
What I've missed is that we also have a separate call for this API, but
which is executed only once when user selects an element. We don't
handle promise rejection here and this can lead to some errors when user
selects an element and switches tabs right after it.
The only change here is that this API now has
`shouldListenToPauseEvents` param, which is `true` for polling, so we
will pause polling once user switches tabs. It is `false` by default, so
we won't pause initial call by accident.
af8beeebf6/packages/react-devtools-shared/src/backendAPI.js (L96)
Some context:
- When user selects an element in tree inspector, we display current
state of the component. In order to display really current state, we
start polling the backend to get available updates for the element.
Previously:
- Straight-forward sending an event to get element updates each second.
Potential race condition is not handled in any form.
- If user navigates from the page, timeout wouldn't be cleared and we
would potentially throw "Timed out ..." error.
- Bridge disconnection is not handled in any form, if it was shut down,
we could spam with "Timed out ..." errors.
With these changes:
- Requests are now chained, so there can be a single request at a time.
- Handling both navigation and shut down events.
This should reduce the number of "Timed out ..." errors that we see in
our logs for the extension. Other surfaces will also benefit from it,
but not to the full extent, as long as they utilize
"resumeElementPolling" and "pauseElementPolling" events.
Tested this on Chrome, running React DevTools on multiple tabs,
explicitly checked the case when service worker is in idle state and we
return back to the tab.
Fixes https://github.com/facebook/react/issues/27119,
https://github.com/facebook/react/issues/27185.
Fixed:
- React DevTools now works as expected when user performs in-tab
navigation, previously it was just stuck.
https://github.com/facebook/react/assets/28902667/b11c5f84-7155-47a5-8b5a-7e90baca5347
- When user closes browser DevTools panel, we now do some cleanup to
disconnect ports and emit shutdown event for bridge. This should fix the
issue with registering duplicated fibers with the same id in Store.
Changed:
- We reconnect proxy port once in 25 seconds, in order to [keep service
worker
alive](https://developer.chrome.com/docs/extensions/whatsnew/#m110-sw-idle).
- Instead of unregistering dynamically injected content scripts, wen now
get list of already registered scripts and filter them out from scripts
that we want to inject again, see dynamicallyInjectContentScripts.js.
- Split `main.js` and `background.js` into multiple files.
Tested on Chromium and Firefox browsers.
When some element is inspected in DevTools, we have a polling which
updates the data for this element.
Sometimes when service worker dies or bridge is getting shutdown, we
continue to poll this data and will spam with the same "timed out
error".
<img width="1728" alt="Screenshot 2023-07-28 at 17 39 23"
src="https://github.com/facebook/react/assets/28902667/220c4504-1ccc-4e87-9d78-bfff8b708230">
These changes add an explicit check that polling is allowed only while
bridge is alive.
## Summary
>Warning: Received NaN for the `children` attribute. If this is
expected, cast the value to a string.
Fixes this warning, when we try to display NaN as NaN in key-value list.
Just a small upgrade to keep us current and remove unused suppressions
(probably fixed by some upgrade since).
- `*` is no longer allowed and has been an alias for `any` for a while
now.
## Summary
We have a case:
1. Open components tab
2. Close Chrome / Firefox devtools window completely
3. Reopen browser devtools panel
4. Open components tab
Currently, in version 4.27.6, we cannot load the components tree.
This PR contains two changes:
- non-functional refactoring in
`react-devtools-shared/src/devtools/store.js`: removed some redundant
type castings.
- fixed backend manager logic (introduced in
https://github.com/facebook/react/pull/26615) to activate already
registered backends. Looks like frontend of devtools also depends on
`renderer-attached` event, without it component tree won't load.
## How did you test this change?
This fixes the case mentioned prior. Currently in 4.27.6 version it is
not working, we need to refresh the page to make it work.
I've tested this in several environments: chrome, firefox, standalone
with RN application.
- substr is Annex B
- substring silently flips its arguments if they're in the "wrong order", which is confusing
- slice is better than sliced bread (no pun intended) and also it works the same way on Arrays so there's less to remember
---
> I'd be down to just lint and enforce a single form just for the potential compression savings by using a repeated string.
_Originally posted by @sebmarkbage in https://github.com/facebook/react/pull/26663#discussion_r1170455401_
## Summary
Removing `enableNamedHooksFeature`, `enableProfilerChangedHookIndices`,
`enableProfilerComponentTree` feature flags, they are the same for all
configurations.
Fixes https://github.com/facebook/react/issues/26500
## Summary
- No more using `clipboard-js` from the backend side, now emitting
custom `saveToClipboard` event, also adding corresponding listener in
`store.js`
- Not migrating to `navigator.clipboard` api yet, there were some issues
with using it on Chrome, will add more details to
https://github.com/facebook/react/pull/26539
## How did you test this change?
- Tested on Chrome, Firefox, Edge
- Tested on standalone electron app: seems like context menu is not
expected to work there (cannot right-click on value, the menu is not
appearing), other logic (pressing on copy icon) was not changed
## Summary
This pull request aims to improve the maintainability of the codebase by
consolidating types and constants that are shared between the backend
and frontend. This consolidation will allow us to maintain backwards
compatibility in the frontend in the future.
To achieve this, we have moved the shared types and constants to the
following blessed files:
- react-devtools-shared/src/constants
- react-devtools-shared/src/types
- react-devtools-shared/src/backend/types
- react-devtools-shared/src/backend/NativeStyleEditor/types
Please note that the inclusion of NativeStyleEditor in this list is
temporary, and we plan to remove it once we have a better plugin system
in place.
## How did you test this change?
I have tested it by running `yarn flow dom-node`, which reports no
errors.
