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.
1.
https://github.com/bvaughn/react/commit/9fc04eaf3fb701cdc14f57d5aed48f3126af6c94#diff-2c5e1f5e80e74154e65b2813cf1c3638f85034530e99dae24809ab4ad70d0143
introduced a vulnerability: we listen to `'fetch-file-with-cache'` event
from `window` to fetch sources of the file, in which we want to parse
hook names. We send this event via `window`, which means any page can
also use this and manipulate the extension to perform some `fetch()`
calls. With these changes, instead of transporting message via `window`,
we have a distinct content script, which is responsible for fetching
sources. It is notified via `chrome.runtime.sendMessage` api, so it
can't be manipulated.
2. Consistent structure of messages `{source: string, payload: object}`
in different parts of the extension
3. Added some wrappers around `chrome.scripting.executeScript` API in
`packages/react-devtools-extensions/src/background/executeScript.js`,
which support custom flow for Firefox, to simulate support of
`ExecutionWorld.MAIN`.
Changes:
1. Refactored react polling logic, now each `.eval()` call is wrapped in
Promise, so we can chain them properly.
2. When user has browser DevTools opened and React DevTools panels were
mounted, user might navigate to the page, which doesn't have React
running. Previously, we would show just blank white page, now we will
show disclaimer. Disclaimer appears after 5 failed attempts to find
React. We will also show this disclaimer if it takes too long to load
the page, but once any React instance is loaded and registered, we will
update the panels.
3. Dark theme support for this disclaimer and popups in Firefox &
Chromium-based browsers
**Important**: this is only valid for case when React DevTools panels
were already created, like when user started debugging React app and
then switched to non-React page. If user starts to debug non-React app
(by opening browser DevTools for it), we will not create these panels,
just like before.
Q: "Why do we poll to get information about react?"
A: To handle case when react is loaded after the page has been loaded,
some sandboxes for example.
| Before | After |
| --- | --- |
| <img width="1840" alt="Screenshot 2023-09-14 at 15 37 37"
src="https://github.com/facebook/react/assets/28902667/2e6ffb39-5698-461d-bfd6-be2defb41aad">
| <img width="1840" alt="Screenshot 2023-09-14 at 15 26 16"
src="https://github.com/facebook/react/assets/28902667/1c8ad2b7-0955-41c5-b8cc-d0fdb03e13ca">
|
- Instead of reconnecting ports from devtools page and proxy content
script, now handling their disconnection properly
- `proxy.js` is now dynamically registered as a content script, which
loaded for each page. This will probably not work well for Firefox,
since we are still on manifest v2, I will try to fix this in the next
few PRs.
- Handling the case when devtools page port was reconnected and bridge
is still present. This could happen if user switches the tab and Chrome
decides to kill service worker, devtools page port gets disconnected,
and then user returns back to the tab. When port is reconnected, we
check if bridge message listener is present, connecting them if so.
- Added simple debounce when evaluating if page has react application
running. We start this check in `chrome.network.onNavigated` listener,
which is asynchronous. Also, this check itself is asynchronous, so
previously we could mount React DevTools multiple times if navigates
multiple times while `chrome.devtools.inspectedWindow.eval` (which is
also asynchronous) can be executed.
https://github.com/hoxyq/react/blob/00b7c4331819289548b40714aea12335368e10f4/packages/react-devtools-extensions/src/main/index.js#L575-L583https://github.com/facebook/react/assets/28902667/9d519a77-145e-413c-b142-b5063223d073
`chrome.devtools.inspectedWindow.eval` is asynchronous, so using it in
`setInterval` is a mistake.
Sometimes this results into mounting React DevTools twice, and user sees
errors about duplicated fibers in store.
With these changes, `executeIfReactHasLoaded` executed recursively with
a threshold (in case if page doesn't have react).
Although we minimize the risk of mounting DevTools twice here, this
approach is not the best way to have this problem solved. Dumping some
thoughts and ideas that I've tried, but which are out of the scope for
this release, because they can be too risky and time-consuming.
Potential changes:
- Have 2 content scripts:
- One `prepareInjection` to notify service worker on renderer attached
- One which runs on `document_idle` to finalize check, in case if there
is no react
- Service worker will notify devtools page that it is ready to mount
React DevTools panels or should show that there is no React to be found
- Extension port from devtools page should be persistent and connected
when `main.js` is executed
- Might require refactoring the logic of how we connect devtools and
proxy ports
Some corner cases:
- Navigating to restricted pages, like `chrome://<something>` and back
- When react is lazily loaded, like in an attached iframe, or just
opened modal
- In-tab navigation with pre-cached pages, I think only Chrome does it
- Firefox is still on manifest v2 and it doesn't allow running content
scripts in ExecutionWorld.MAIN, so it requires a different approach
Multiple `chrome.panels.create` calls result into having duplicate
panels created in Firefox, these changes fix that.
Now calling `chrome.panels.create` only if there are no panels created
yet.
This is mostly hotfix for https://github.com/facebook/react/pull/27215.
Contains 3 fixes:
- Handle cases when `react` is not loaded yet and user performs in-tab
navigation. Previously, because of the uncleared interval we would try
to mount DevTools twice, resulting into multiple errors.
- Handle case when extension port disconnected (probably by the browser
or just due to its lifetime)
- Removed duplicate `render()` call on line 327
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.
Vitali Zaidman
Ruslan Lesiutin