In the extension, currently we do the following:
1. check whether there's at least one React renderer on the page
2. if yes, load the backend to the page
3. initialize the backend
To support multiple versions of backends, we are changing it to:
1. check the versions of React renders on the page
2. load corresponding React DevTools backends that are shipped with the
extension; if they are not contained (usually prod builds of
prereleases), show a UI to allow users to load them from UI
3. initialize each of the backends
To enable this workflow, a backend will ignore React renderers that does
not match its version
This PR adds a new file "backendManager" in the extension for this
purpose.
------
I've tested it on Chrome, Edge and Firefox extensions
## Summary
- #26234 is reverted and replaced with a better approach
- introduce a new global devtools variable to decouple the global hook's
dependency on backend/console.js, and add it to react-devtools-inline
and react-devtools-standalone
With this PR, I want to introduce a new principle to hook.js: we should
always be alert when editing this file and avoid importing from other
files.
In the past, we try to inline a lot of the implementation because we use
`.toString()` to inject this function from the extension (we still have
some old comments left). Although it is no longer inlined that way, it
has became now more important to keep it clean as it is a de facto
global API people are using (9.9K files contains it on Github search as
of today).
**File size change for extension:**
Before:
379K installHook.js
After:
21K installHook.js
363K renderer.js
## Summary
When looking into the compiled code of `installHook.js` of the extension
build, I noticed that it actually includes the large `attach` function
(from renderer.js). I don't think it was expected.
This is because `hook.js` imports from `backend/console.js` which
imports from `backend/renderer.js` for `getInternalReactConstants`
A straightforward way is to extract function
`getInternalReactConstants`. However, I think it's more simplified to
just merge these two files and save the 361K renderer.js from the
extension build since we have always been loading this code anyways.
I changed the execution check from `__REACT_DEVTOOLS_ATTACH__ ` to the
session storage.
## How did you test this change?
Everything works normal in my local build.
## Summary
Prettier was bumped recently. So any branch not including that bump,
might bring in outdated formatting (e.g.
https://github.com/facebook/react/pull/26068)
## How did you test this change?
- [x] `yarn prettier-all`
## Summary
This is to fix some edge cases I recently observed when developing and
using the extension:
- When you reload the page, there's a chance that a port (most likely
the devtools one) is not properly unloaded. In this case, the React
DevTools will stop working unless you create a new tab.
- For unknown reasons, Chrome sometimes spins up two service worker
processes. In this case, an error will be thrown "duplicate ID when
registering content script" and sometimes interrupt the execution of the
rest of service worker.
This is an attempt to make the logic more robust
- Automatically shutting down the double pipe if the message fails, and
allowing the runtime to rebuild the double pipe.
- Log the error message so Chrome believes we've handled it and will not
interrupt the execution.
This also seems to be helpful in fixing #25806.
The old version of prettier we were using didn't support the Flow syntax
to access properties in a type using `SomeType['prop']`. This updates
`prettier` and `rollup-plugin-prettier` to the latest versions.
I added the prettier config `arrowParens: "avoid"` to reduce the diff
size as the default has changed in Prettier 2.0. The largest amount of
changes comes from function expressions now having a space. This doesn't
have an option to preserve the old behavior, so we have to update this.
## Summary
resolves#24522
To upgrade to Manifest V3, one of the biggest issue is that we are no
longer allowed to add a script element with code in textContent so that
it would run synchronously. It's necessary for us because we need to
inject a global hook for react reconciler to detect whether devtools
exist.
To do that, we'll leverage a new API
`chrome.scripting.registerContentScripts` in V3. Particularly, we rely
on the "world" option (added in Chrome v102
[commit](https://chromium.googlesource.com/chromium/src/+/e5ad3451c17b21341b0b9019b074801c44c92c9f))
to run it in the "main world" on the page.
This PR also renames a few content script files so that it's easier to
tell them apart from other extension scripts and understand the purpose
of each of them.
Manifest V3 is not yet ready for Firefox, so we need to keep some code
for compatibility.
## How did you test this change?
`yarn build:chrome && yarn test:chrome`
`yarn build:edge && yarn test:edge`
`yarn build:firefox && yarn test:firefox`
* Revert "Only show DevTools warning about unrecognized build in Chrome (#22571)"
This reverts commit b72dc8e930.
* Revert "Show warning in UI when duplicate installations of DevTools extension are detected (#22563)"
This reverts commit 930c9e7eeb.
* Revert "Prevent errors/crashing when multiple installs of DevTools are present (#22517)"
This reverts commit 545d4c2de7.
* Remove all references to passing extensionId in postMessage
* Keep build changes
* lint
## Summary
This commit is a proposal for handling duplicate installation of DevTools, in particular scoped to duplicates such as a dev build or the internal versions of DevTools installed alongside the Chrome Web Store extension.
Specifically, this commit makes it so when another instance of the DevTools extension is installed alongside the extension installed from the Chrome Web Store, we don't produce a stream of errors or crash Chrome, which is what would usually happen in this case.
### Detecting Duplicate Installations
- First, we check what type of installation the extension is: from the Chrome Web Store, the internal build of the extension, or a local development build.
- If the extension is from the **Chrome Web Store**:
- During initialization, we first check if the internal or local builds of the extension have already been installed and are enabled. To do this, we send a [cross-extension message](https://developer.chrome.com/docs/extensions/mv3/messaging/#external) to the internal and local builds of the extension using their extension IDs.
- We can do this because the extension ID for the internal build (and for the Chrome Web Store) is a stable ID.
- For the local build, at build time we hardcode a [`key` in the `manifest.json`](https://developer.chrome.com/docs/extensions/mv2/manifest/key/) which allows us to have a stable ID even for local builds.
- If we detect that the internal or local extensions are already installed, then we skip initializing the current extension altogether so as to not conflict with the other versions. This means we don't initialize the frontend or the backend at all.
- If the extension is the **Internal Build**:
- During initialization, we first check if the local builds of the extension has already been installed and is enabled. To do this, we send a [cross-extension message](https://developer.chrome.com/docs/extensions/mv3/messaging/#external) to the local build of the extension using its extension ID.
- We can do this for the local build because at build time we hardcode a [`key` in the `manifest.json`](https://developer.chrome.com/docs/extensions/mv2/manifest/key/) which allows us to have a stable ID even for local builds.
- If we detect that the local extension is already installed, then we skip initializing the current extension altogether so as to not conflict with the that version. This means we don't initialize the frontend or the backend at all.
- If the extension is a **Local Dev Build**:
- Since other extensions check for the existence of this extension and disable themselves if they detect it, we don't need any special handling during initialization and assume that there are no duplicate extensions. This means that we will generally prefer keeping this extension enabled.
This behavior means that the order of priority for keeping an extension enabled is the following:
1. Local build
2. Internal build
3. Public build
### Preventing duplicate backend initialization
Note that the backend is injected and initialized by the content script listening to a message posted to the inspected window (via `postMessage`). Since the content script will be injected twice, once each by each instance of the extension, even if we initialize the extension once, both content scripts would still receive the single message posted from the single frontend, and it would then still inject and initialize the backend twice.
In order to prevent this, we also add the extension ID to the message for injecting the backend. That way each content script can check if the message comes from its own extension, and if not it can ignore the message and avoid double injecting the backend.
### Other approaches
- I considered using the [`chrome.management`](https://developer.chrome.com/docs/extensions/reference/management/) API generally to detect other installations, but that requires adding additional permissions to our production extension, which didn't seem ideal.
- I also considered a few options of writing a special flag to the inspected window and checking for it before initializing the extension. However, it's hard to avoid race conditions in that case, and it seemed more reliable to check specifically for the WebStore extension, which is realistically where we would encounter the overlap.
### Rollout
- This commit needs to be published and rolled out to the Chrome Web Store first.
- After this commit is published to the Chrome Web Store, any duplicate instances of the extension that are built and installed after this commit will no longer conflict with the Chrome Web Store version.
### Next Steps
- In a subsequent PR, I will extend this code to show a warning when duplicate extensions have been detected.
Part of #22486
## How did you test this change?
### Basic Testing
- yarn flow
- yarn test
- yarn test-build-devtools
### Double installation testing
Testing double-installed extensions for this commit is tricky because we are relying on the extension ID of the internal and Chrome Web Store extensions, but we obviously can't actually test the Web Store version (since we can't modify the already published version).
In order to simulate duplicate extensions installed, I did the following process:
- Built separate extensions where I hardcoded a constant for whether the extension is internal or public (e.g. `EXTENSION_INSTALLATION_TYPE = 'internal'`). Then I installed these built extensions corresponding to the "internal" and "Web Store" builds.
- Build and run the regular development extension (with `yarn build:chrome:dev && yarn test:chrome`), using the extension IDs of the previously built extensions as the "internal" and "public" extension IDs.
With this set up in place, I tested the following on pages both with and without React:
- When only the local extension enabled, DevTools works normally.
- When only the "internal" extension enabled, DevTools works normally.
- When only the "public" extension enabled, DevTools works normally.
- When "internal" and "public" extensions are installed, "public" extension is disabled and "internal" extension works normally.
- When the local extension runs alongside the other extensions, other extensions disable themselves and local build works normally.
- When we can't recognize what type of build the extension corresponds to, we show an error.
- When all 3 extensions are installed and enabled in all different combinations, DevTools no longer produces errors or crashes Chrome, and works normally.
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.
Mengdi Chen
Sebastian Silbermann
Jan Kassens
Juan
Brian Vaughn
Saikat Guha
