## Summary
This PR adds a "perf regression tests" page to react-devtools-shell.
This page is meant to be used as a performance sanity check we will run
whenever we release a new version or finish a major refactor.
Similar to other pages in the shell, this page can load the inline
version of devtools and a test react app on the same page. But this page
does not load devtools automatically like other pages. Instead, it
provides a button that allows us to load devtools on-demand, so that we
can easily compare perf numbers without devtools against the numbers
with devtools.
<img width="561" alt="image"
src="https://user-images.githubusercontent.com/1001890/184059633-e4f0852c-8464-4d94-8064-1684eee626f4.png">
As a first step, this page currently only contain one test:
mount/unmount a large subtree. This is to catch perf issues that
devtools can cause on the react applications it's running on, which was
once a bug fixed in #24863.
In the future, we plan to add:
- more test apps covering different scenarios
- perf numbers within devtools (e.g. initial load)
## How did you test this change?
In order to show this test app can actually catch the perf regression
it's aiming at, I reverted #24863 locally. Here is the result:
https://user-images.githubusercontent.com/1001890/184059214-9c9b308c-173b-4dd7-b815-46fbd7067073.mov
As shown in the video, the time it takes to unmount the large subtree
significantly increased after DevTools is loaded.
For comparison, here is how it looks like before the fix was reverted:
<img width="452" alt="image"
src="https://user-images.githubusercontent.com/1001890/184059743-0968bc7d-4ce4-42cd-b04a-f6cbc078d4f4.png">
## about the `requestAnimationFrame` method
For this test, I used `requestAnimationFrame` to catch the time when
render and commit are done. It aligns very well with the numbers
reported by Chrome DevTools performance profiling. For example, in one
run, the numbers reported by my method are
<img width="464" alt="image"
src="https://user-images.githubusercontent.com/1001890/184060228-990a4c75-f594-411a-9f85-fa5532ec8c37.png">
They are very close to the numbers reported by Chrome profiling:
<img width="456" alt="image"
src="https://user-images.githubusercontent.com/1001890/184060355-a15d1ec5-c296-4016-9c83-03e761f387e3.png">
<img width="354" alt="image"
src="https://user-images.githubusercontent.com/1001890/184060375-19029010-3aed-4a23-890e-397cdba86d9e.png">
`<Profiler>` is not able to catch this issue here.
If you are aware of a better way to do this, please kindly share with
me.
Made a couple of fixes to the `devtools-test-shell`
* test selectors aren't available in > React v18.0 either, so we'll need to mock the test selector functions there as well
* `react-dom/client` should map to `react-dom/client` and not `react-dom`
This PR adds an e2e regression app to the react-devtools-shell package. This app:
* Has an app.js and an appLegacy.js entrypoint because apps prior to React 18 need to use ReactDOM.render. These files will create and render multiple test apps (though they currently only render the List)
* Moved the ListApp out of the e2e folder and into an e2e-apps folder so that both e2e and e2e-regression can use the same test apps
* Creates a ListAppLegacy app because prior to React 16.8 hooks didn't exist.
* Added a devtools file for the e2e-regression
* Modifies the webpack config so that the e2e-regression React app can use different a different React version than DevTools
* Move createRoot/hydrateRoot to /client
We want these APIs ideally to be imported separately from things you
might use in arbitrary components (like flushSync). Those other methods
are "isomorphic" to how the ReactDOM tree is rendered. Similar to hooks.
E.g. importing flushSync into a component that only uses it on the client
should ideally not also pull in the entry client implementation on the
server.
This also creates a nicer parity with /server where the roots are in a
separate entry point.
Unfortunately, I can't quite do this yet because we have some legacy APIs
that we plan on removing (like findDOMNode) and we also haven't implemented
flushSync using a flag like startTransition does yet.
Another problem is that we currently encourage these APIs to be aliased by
/profiling (or unstable_testing). In the future you don't have to alias
them because you can just change your roots to just import those APIs and
they'll still work with the isomorphic forms. Although we might also just
use export conditions for them.
For that all to work, I went with a different strategy for now where the
real API is in / but it comes with a warning if you use it. If you instead
import /client it disables the warning in a wrapper. That means that if you
alias / then import /client that will inturn import the alias and it'll
just work.
In a future breaking changes (likely when we switch to ESM) we can just
remove createRoot/hydrateRoot from / and move away from the aliasing
strategy.
* Update tests to import from react-dom/client
* Fix fixtures
* Update warnings
* Add test for the warning
* Update devtools
* Change order of react-dom, react-dom/client alias
I think the order matters here. The first one takes precedence.
* Require react-dom through client so it can be aliased
Co-authored-by: Andrew Clark <git@andrewclark.io>
This is being done so that we can embed DevTools within the new React (beta) docs.
The primary changes here are to `react-devtools-inline/backend`:
* Add a new `createBridge` API
* Add an option to the `activate` method to support passing in the custom bridge object.
The `react-devtools-inline` README has been updated to include these new methods.
To verify these changes, this commit also updates the test shell to add a new entry-point for multiple DevTools.
This commit also replaces two direct calls to `window.postMessage()` with `bridge.send()` (and adds the related Flow types).
This change adds a new "react-dom/unstable_testing" entry point but I believe its contents will exactly match "react-dom/index" for the stable build. (The experimental build will have the added new selector APIs.)
Update all our local scripts to use `build` instead of `build2`.
There are still downstream scripts that depend on `build2`, though, so
we can't remove it yet.
This commit builds on PR #22260 and makes the following changes:
* Adds a DevTools feature flag for named hooks support. (This allows us to disable it entirely for a build via feature flag.)
* Adds a new Suspense cache for dynamically imported modules. (This allows a component to suspend while importing an external code chunk– like the hook names parsing code).
* DevTools supports a hookNamesModuleLoaderFunction param to import the hook names module. I wish this could be handles as part of the react-devtools-shared package, but I'm not sure how to configure Webpack (4) to serve the chunk from react-devtools-inline. This seemed like a reasonable workaround.
The PR also contains an additional unrelated change:
* Removes pre-fetch optimization (added in DevTools: Improve named hooks network caching #22198). This optimization was mostly only important for cases where sources needed to be re-downloaded, something which we can now avoid in most cases¹ thanks to using cached responses already loaded by the page. (I tested this locally on Facebook and this change has no negative performance impact. There is still some overhead from serializing the JS through the Bridge but that's constant between the two approaches.)
¹ The case where we don't benefit from cached responses is when DevTools are opened after the page has already loaded certain scripts. This seems uncommon enough that I don't think it justified the added complexity of prefetching.
React currently suppress console logs in StrictMode during double rendering. However, this causes a lot of confusion. This PR moves the console suppression logic from React into React Devtools. Now by default, we no longer suppress console logs. Instead, we gray out the logs in console during double render. We also add a setting in React Devtools to allow developers to hide console logs during double render if they choose.
Also resolve an uncaught error in extension build (#18843).
Co-authored-by: Brian Vaughn <brian.david.vaughn@gmail.com>
Co-authored-by: Brian Vaughn <bvaughn@fb.com>
DevTools isn't being downloaded like typical JavaScript, so bundle size concerns don't apply. Parsing is still a consideration (so I'm open for discussion here) but I think this change would provide a couple of benefits:
* People are more likely to *actually read* non-minified source code when e.g. a breakpoint is hit (as with the recent debugger statement)
* Component stacks will be easier to parse on bug reports
This commit adds a new tab to the Settings modal: Debugging
This new tab has the append component stacks feature and a new one: break on warn
This new feature adds a debugger statement into the console override
* DevTools console override handles new component stack format
DevTools does not attempt to mimic the default browser console format for its component stacks but it does properly detect the new format for Chrome, Firefox, and Safari.
Mengdi Chen
