Skip to content

Commit

Permalink
DevTools: fix backend activation (facebook#26779)
Browse files Browse the repository at this point in the history
## 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
facebook#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.
  • Loading branch information
hoxyq authored and AndyPengc12 committed Apr 15, 2024
1 parent 4e658e9 commit 6a1bd52
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 66 deletions.
7 changes: 7 additions & 0 deletions packages/react-devtools-extensions/src/backendManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,13 @@ function setup(hook: ?DevToolsHook) {
hook.renderers.forEach(renderer => {
registerRenderer(renderer);
});

// Activate and remove from required all present backends, registered within the hook
hook.backends.forEach((_, backendVersion) => {
requiredBackends.delete(backendVersion);
activateBackend(backendVersion, hook);
});

updateRequiredBackends();

// register renderers that inject themselves later.
Expand Down
7 changes: 4 additions & 3 deletions packages/react-devtools-extensions/src/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@ async function dynamicallyInjectContentScripts() {
// For some reason dynamically injected scripts might be already registered
// Registering them again will fail, which will result into
// __REACT_DEVTOOLS_GLOBAL_HOOK__ hook not being injected
await chrome.scripting.unregisterContentScripts({
ids: contentScriptsToInject.map(s => s.id),
});

// Not specifying ids, because Chrome throws an error
// if id of non-injected script is provided
await chrome.scripting.unregisterContentScripts();

// equivalent logic for Firefox is in prepareInjection.js
// Manifest V3 method of injecting content script
Expand Down
8 changes: 2 additions & 6 deletions packages/react-devtools-shared/src/backend/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {hasAssignedBackend} from './utils';

import type {DevToolsHook, ReactRenderer, RendererInterface} from './types';

// this is the backend that is compactible with all older React versions
// this is the backend that is compatible with all older React versions
function isMatchingRender(version: string): boolean {
return !hasAssignedBackend(version);
}
Expand All @@ -31,6 +31,7 @@ export function initBackend(
// DevTools didn't get injected into this page (maybe b'c of the contentType).
return () => {};
}

const subs = [
hook.sub(
'renderer-attached',
Expand Down Expand Up @@ -64,10 +65,6 @@ export function initBackend(
];

const attachRenderer = (id: number, renderer: ReactRenderer) => {
// skip if already attached
if (renderer.attached) {
return;
}
// only attach if the renderer is compatible with the current version of the backend
if (!isMatchingRender(renderer.reconcilerVersion || renderer.version)) {
return;
Expand Down Expand Up @@ -102,7 +99,6 @@ export function initBackend(
} else {
hook.emit('unsupported-renderer-version', id);
}
renderer.attached = true;
};

// Connect renderers that have already injected themselves.
Expand Down
2 changes: 0 additions & 2 deletions packages/react-devtools-shared/src/backend/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,6 @@ export type ReactRenderer = {
// 18.0+
injectProfilingHooks?: (profilingHooks: DevToolsProfilingHooks) => void,
getLaneLabelMap?: () => Map<Lane, string> | null,
// set by backend after successful attaching
attached?: boolean,
...
};

Expand Down
Loading

0 comments on commit 6a1bd52

Please sign in to comment.