While testing the recently-launched named hooks feature, I noticed that one of the two big performance bottlenecks is fetching the source file. This was unexpected since the source file has already been loaded by the page. (After all, DevTools is inspecting a component defined in that same file.)
To address this, I made the following changes:
- [x] Keep CPU bound work (parsing source map and AST) in a worker so it doesn't block the main thread but move I/O bound code (fetching files) to the main thread.
- [x] Inject a function into the page (as part of the content script) to fetch cached files for the extension. Communicate with this function using `eval()` (to send it messages) and `chrome.runtime.sendMessage()` to return its responses to the extension).
With the above changes in place, the extension gets cached responses from a lot of sites- but not Facebook. This seems to be due to the following:
* Facebook's response headers include [`vary: 'Origin'`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Vary).
* The `fetch` made from the content script does not include an `Origin` request header.
To reduce the impact of cases where we can't re-use the Network cache, this PR also makes additional changes:
- [x] Use `devtools.network.onRequestFinished` to (pre)cache resources as the page loads them. This allows us to avoid requesting a resource that's already been loaded in most cases.
- [x] In case DevTools was opened _after_ some requests were made, we also now pre-fetch (and cache in memory) source files when a component is selected (if it has hooks). If the component's hooks are later evaluated, the source map will be faster to access. (Note that in many cases, this prefetch is very fast since it is returned from the disk cache.)
With the above changes, we've reduced the time spent in `loadSourceFiles` to nearly nothing.
lunaruan commented 3 days ago •
This PR adds separate DevTools feature flag configurations for react-devtools-core. It also breaks the builds down into facebook specific and open source flags so we can experiment in React Native.
Tested yarn build:standalone, yarn build:backend, yarn build:standalone:fb, and yarn build:backend:fb and inspected the output to make sure each package used the correct feature flags (the first two use core-oss and the latter two use fb-oss.
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.
We use an LRU for this rather than a WeakMap because of how the "no-change" optimization works.
When the frontend polls the backend for an update on the element that's currently inspected, the backend will send a "no-change" message if the element hasn't updated (rendered) since the last time it was asked. In thid case, the frontend cache should reuse the previous (cached) value. Using a WeakMap keyed on Element generally works well for this, since Elements are mutable and stable in the Store. This doens't work properly though when component filters are changed, because this will cause the Store to dump all roots and re-initialize the tree (recreating the Element objects).
So instead we key on Element ID (which is stable in this case) and use an LRU for eviction.
Also highlight events that have synchronous updates inside of them. (We may want to relax this highlighting later to not warn about event handlers that are still fast enough.)
In legacy roots, if an update originates outside of `batchedUpdates`,
check if it's inside an `act` scope; if so, treat it as if it were
batched. This is only necessary in legacy roots because in concurrent
roots, updates are batched by default.
With this change, the Test Utils and Test Renderer versions of `act` are
nothing more than aliases of the isomorphic API (still not exposed, but
will likely be the recommended API that replaces the others).
* Re-land recent flushSync changes
Adds back #21776 and #21775, which were removed due to an internal
e2e test failure.
Will attempt to fix in subsequent commits.
* Failing test: Legacy mode sync passive effects
In concurrent roots, if a render is synchronous, we flush its passive
effects synchronously. In legacy roots, we don't do this because all
updates are synchronous — so we need to flush at the beginning of the
next event. This is how `discreteUpdates` worked.
* Flush legacy passive effects at beginning of event
Fixes test added in previous commit.
Made several changes to the hooks name cache to avoid losing cached data between selected elements:
1. No longer use React-managed cache. This had the unfortunate side effect of the inspected element cache also clearing the hook names cache. For now, instead, a module-level WeakMap cache is used. This isn't great but we can revisit it later.
2. Hooks are no longer the cache keys (since hook objects get recreated between element inspections). Instead a hook key string made of fileName + line number + column number is used.
3. If hook names have already been loaded for a component, skip showing the load button and just show the hook names by default when selecting the component.
* Add named hooks test case built with Rollup
* Fix prepareStackTrace unpatching, remove sourceURL
* Prettier
* Resolve source map URL/path relative to the script
* Add failing tests for multi-module bundle
* Parse hook names from multiple modules in a bundle
* Create a HookSourceData per location key (file, line, column).
* Cache the source map per runtime URL ( = file part of location key).
* Don't store sourceMapContents - only store a consumer instance.
* Look up original source URLs in the source map correctly.
* Cache the code + AST per original URL.
* Fix off-by-one column number lookup.
* Some naming and typing tweaks related to the above.
* Stop storing the consumer outside the with() callback, which is a bug.
* Lint fix for 8d8dd25
* Added devDependencies to react-devtools-extensions package.json
* Added some debug logging and TODO comments
* Added additional DEBUG logging to hook names cache
Co-authored-by: Brian Vaughn <bvaughn@fb.com>
There's a weird quirk leftover from the old Stack (pre-Fiber)
implementation where the initial mount of a leagcy (ReactDOM.render)
root is flushed synchronously even inside `batchedUpdates`.
The original workaround for this was an internal method called
`unbatchedUpdates`. We've since added another API that works almost the
same way, `flushSync`.
The only difference is that `unbatchedUpdates` would not cause other
pending updates to flush too, only the newly mounted root. `flushSync`
flushes all pending sync work across all roots. This was to preserve
the exact behavior of the Stack implementation.
But since it's close enough, let's just use `flushSync`. It's unlikely
anyone's app accidentally relies on this subtle difference, and the
legacy API is deprecated in 18, anyway.
* Move internal version of act to shared module
No reason to have three different copies of this anymore.
I've left the the renderer-specific `act` entry points because legacy
mode tests need to also be wrapped in `batchedUpdates`. Next, I'll update
the tests to use `batchedUpdates` manually when needed.
* Migrates tests to use internal module directly
Instead of the `unstable_concurrentAct` exports. Now we can drop those
from the public builds.
I put it in the jest-react package since that's where we put our other
testing utilities (like `toFlushAndYield`). Not so much so it can be
consumed publicly (nobody uses that package except us), but so it works
with our build tests.
* Remove unused internal fields
These were used by the old act implementation. No longer needed.
* Use existing test warning filter for server tests
We have a warning filter for our internal tests to ignore warnings
that are too noisy or that we haven't removed from our test suite yet:
shouldIgnoreConsoleError.
Many of our server rendering tests don't use this filter, though,
because it has its own special of asserting warnings.
So I added the warning filter to the server tests, too.
* Deprecate ReactDOM.render and ReactDOM.hydrate
These are no longer supported in React 18. They are replaced by the
`createRoot` API.
The warning includes a link to documentation of the new API. Currently
it redirects to the corresponding working group post. Here's the PR to
set up the redirect: https://github.com/reactjs/reactjs.org/pull/3730
Many of our tests still use ReactDOM.render. We will need to gradually
migrate them over to createRoot.
In the meantime, I added the warnings to our internal warning filter.
