@samitouri / QOS-React / commits / 377c5175f7

DevTools: fix backend activation (#26779)

## 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 https://github.com/facebook/react/pull/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.

Ruslan Lesiutin committed May 4, 2023 at 17:34 UTC 377c5175f78e47a3f01d323ad6528a696c88b76e
5 files changed +82 -66
packages/react-devtools-extensions/src/backendManager.js
+7
@@ -61,6 +61,13 @@ function setup(hook: ?DevToolsHook) {
61 hook.renderers.forEach(renderer => {
62 registerRenderer(renderer);
63 });
64 +
65 + // Activate and remove from required all present backends, registered within the hook
66 + hook.backends.forEach((_, backendVersion) => {
67 + requiredBackends.delete(backendVersion);
68 + activateBackend(backendVersion, hook);
69 + });
70 +
71 updateRequiredBackends();
72
73 // register renderers that inject themselves later.
packages/react-devtools-extensions/src/background.js
+4 -3
@@ -28,9 +28,10 @@ async function dynamicallyInjectContentScripts() {
28 // For some reason dynamically injected scripts might be already registered
29 // Registering them again will fail, which will result into
30 // __REACT_DEVTOOLS_GLOBAL_HOOK__ hook not being injected
31 - await chrome.scripting.unregisterContentScripts({
32 - ids: contentScriptsToInject.map(s => s.id),
33 - });
31 +
32 + // Not specifying ids, because Chrome throws an error
33 + // if id of non-injected script is provided
34 + await chrome.scripting.unregisterContentScripts();
35
36 // equivalent logic for Firefox is in prepareInjection.js
37 // Manifest V3 method of injecting content script
packages/react-devtools-shared/src/backend/index.js
+2 -6
@@ -15,7 +15,7 @@ import {hasAssignedBackend} from './utils';
15
16 import type {DevToolsHook, ReactRenderer, RendererInterface} from './types';
17
18 -// this is the backend that is compactible with all older React versions
18 +// this is the backend that is compatible with all older React versions
19 function isMatchingRender(version: string): boolean {
20 return !hasAssignedBackend(version);
21 }
@@ -31,6 +31,7 @@ export function initBackend(
31 // DevTools didn't get injected into this page (maybe b'c of the contentType).
32 return () => {};
33 }
34 +
35 const subs = [
36 hook.sub(
37 'renderer-attached',
@@ -64,10 +65,6 @@ export function initBackend(
65 ];
66
67 const attachRenderer = (id: number, renderer: ReactRenderer) => {
67 - // skip if already attached
68 - if (renderer.attached) {
69 - return;
70 - }
68 // only attach if the renderer is compatible with the current version of the backend
69 if (!isMatchingRender(renderer.reconcilerVersion || renderer.version)) {
70 return;
@@ -102,7 +99,6 @@ export function initBackend(
99 } else {
100 hook.emit('unsupported-renderer-version', id);
101 }
105 - renderer.attached = true;
102 };
103
104 // Connect renderers that have already injected themselves.
packages/react-devtools-shared/src/backend/types.js
-2
@@ -171,8 +171,6 @@ export type ReactRenderer = {
171 // 18.0+
172 injectProfilingHooks?: (profilingHooks: DevToolsProfilingHooks) => void,
173 getLaneLabelMap?: () => Map<Lane, string> | null,
174 - // set by backend after successful attaching
175 - attached?: boolean,
174 ...
175 };
176
packages/react-devtools-shared/src/devtools/store.js
+69 -55
@@ -169,7 +169,7 @@ export default class Store extends EventEmitter<{
169 // Renderer ID is needed to support inspection fiber props, state, and hooks.
170 _rootIDToRendererID: Map<number, number> = new Map();
171
172 - // These options may be initially set by a confiugraiton option when constructing the Store.
172 + // These options may be initially set by a configuration option when constructing the Store.
173 _supportsNativeInspection: boolean = true;
174 _supportsProfiling: boolean = false;
175 _supportsReloadAndProfile: boolean = false;
@@ -486,7 +486,7 @@ export default class Store extends EventEmitter<{
486 }
487
488 containsElement(id: number): boolean {
489 - return this._idToElement.get(id) != null;
489 + return this._idToElement.has(id);
490 }
491
492 getElementAtIndex(index: number): Element | null {
@@ -539,13 +539,13 @@ export default class Store extends EventEmitter<{
539 }
540
541 getElementIDAtIndex(index: number): number | null {
542 - const element: Element | null = this.getElementAtIndex(index);
542 + const element = this.getElementAtIndex(index);
543 return element === null ? null : element.id;
544 }
545
546 getElementByID(id: number): Element | null {
547 const element = this._idToElement.get(id);
548 - if (element == null) {
548 + if (element === undefined) {
549 console.warn(`No element found with id "${id}"`);
550 return null;
551 }
@@ -607,7 +607,10 @@ export default class Store extends EventEmitter<{
607 let currentID = element.parentID;
608 let index = 0;
609 while (true) {
610 - const current = ((this._idToElement.get(currentID): any): Element);
610 + const current = this._idToElement.get(currentID);
611 + if (current === undefined) {
612 + return null;
613 + }
614
615 const {children} = current;
616 for (let i = 0; i < children.length; i++) {
@@ -615,7 +618,12 @@ export default class Store extends EventEmitter<{
618 if (childID === previousID) {
619 break;
620 }
618 - const child = ((this._idToElement.get(childID): any): Element);
621 +
622 + const child = this._idToElement.get(childID);
623 + if (child === undefined) {
624 + return null;
625 + }
626 +
627 index += child.isCollapsed ? 1 : child.weight;
628 }
629
@@ -637,7 +645,12 @@ export default class Store extends EventEmitter<{
645 if (rootID === currentID) {
646 break;
647 }
640 - const root = ((this._idToElement.get(rootID): any): Element);
648 +
649 + const root = this._idToElement.get(rootID);
650 + if (root === undefined) {
651 + return null;
652 + }
653 +
654 index += root.weight;
655 }
656
@@ -647,7 +660,7 @@ export default class Store extends EventEmitter<{
660 getOwnersListForElement(ownerID: number): Array<Element> {
661 const list: Array<Element> = [];
662 const element = this._idToElement.get(ownerID);
650 - if (element != null) {
663 + if (element !== undefined) {
664 list.push({
665 ...element,
666 depth: 0,
@@ -665,8 +678,8 @@ export default class Store extends EventEmitter<{
678 // Seems better to defer the cost, since the set of ids is probably pretty small.
679 const sortedIDs = Array.from(unsortedIDs).sort(
680 (idA, idB) =>
668 - ((this.getIndexOfElementID(idA): any): number) -
669 - ((this.getIndexOfElementID(idB): any): number),
681 + (this.getIndexOfElementID(idA) || 0) -
682 + (this.getIndexOfElementID(idB) || 0),
683 );
684
685 // Next we need to determine the appropriate depth for each element in the list.
@@ -677,7 +690,7 @@ export default class Store extends EventEmitter<{
690 // at which point, our depth is just the depth of that node plus one.
691 sortedIDs.forEach(id => {
692 const innerElement = this._idToElement.get(id);
680 - if (innerElement != null) {
693 + if (innerElement !== undefined) {
694 let parentID = innerElement.parentID;
695
696 let depth = 0;
@@ -689,7 +702,7 @@ export default class Store extends EventEmitter<{
702 break;
703 }
704 const parent = this._idToElement.get(parentID);
692 - if (parent == null) {
705 + if (parent === undefined) {
706 break;
707 }
708 parentID = parent.parentID;
@@ -710,7 +723,7 @@ export default class Store extends EventEmitter<{
723
724 getRendererIDForElement(id: number): number | null {
725 let current = this._idToElement.get(id);
713 - while (current != null) {
726 + while (current !== undefined) {
727 if (current.parentID === 0) {
728 const rendererID = this._rootIDToRendererID.get(current.id);
729 return rendererID == null ? null : rendererID;
@@ -723,7 +736,7 @@ export default class Store extends EventEmitter<{
736
737 getRootIDForElement(id: number): number | null {
738 let current = this._idToElement.get(id);
726 - while (current != null) {
739 + while (current !== undefined) {
740 if (current.parentID === 0) {
741 return current.id;
742 } else {
@@ -765,10 +778,8 @@ export default class Store extends EventEmitter<{
778
779 const weightDelta = 1 - element.weight;
780
768 - let parentElement: void | Element = ((this._idToElement.get(
769 - element.parentID,
770 - ): any): Element);
771 - while (parentElement != null) {
781 + let parentElement = this._idToElement.get(element.parentID);
782 + while (parentElement !== undefined) {
783 // We don't need to break on a collapsed parent in the same way as the expand case below.
784 // That's because collapsing a node doesn't "bubble" and affect its parents.
785 parentElement.weight += weightDelta;
@@ -776,7 +787,7 @@ export default class Store extends EventEmitter<{
787 }
788 }
789 } else {
779 - let currentElement = element;
790 + let currentElement: ?Element = element;
791 while (currentElement != null) {
792 const oldWeight = currentElement.isCollapsed
793 ? 1
@@ -791,10 +802,8 @@ export default class Store extends EventEmitter<{
802 : currentElement.weight;
803 const weightDelta = newWeight - oldWeight;
804
794 - let parentElement: void | Element = ((this._idToElement.get(
795 - currentElement.parentID,
796 - ): any): Element);
797 - while (parentElement != null) {
805 + let parentElement = this._idToElement.get(currentElement.parentID);
806 + while (parentElement !== undefined) {
807 parentElement.weight += weightDelta;
808 if (parentElement.isCollapsed) {
809 // It's important to break on a collapsed parent when expanding nodes.
@@ -808,10 +817,8 @@ export default class Store extends EventEmitter<{
817
818 currentElement =
819 currentElement.parentID !== 0
811 - ? // $FlowFixMe[incompatible-type] found when upgrading Flow
812 - this.getElementByID(currentElement.parentID)
813 - : // $FlowFixMe[incompatible-type] found when upgrading Flow
814 - null;
820 + ? this.getElementByID(currentElement.parentID)
821 + : null;
822 }
823 }
824
@@ -833,7 +840,7 @@ export default class Store extends EventEmitter<{
840 }
841
842 _adjustParentTreeWeight: (
836 - parentElement: Element | null,
843 + parentElement: ?Element,
844 weightDelta: number,
845 ) => void = (parentElement, weightDelta) => {
846 let isInsideCollapsedSubTree = false;
@@ -848,9 +855,7 @@ export default class Store extends EventEmitter<{
855 break;
856 }
857
851 - parentElement = ((this._idToElement.get(
852 - parentElement.parentID,
853 - ): any): Element);
858 + parentElement = this._idToElement.get(parentElement.parentID);
859 }
860
861 // Additions and deletions within a collapsed subtree should not affect the overall number of elements.
@@ -906,13 +911,16 @@ export default class Store extends EventEmitter<{
911 const stringTable: Array<string | null> = [
912 null, // ID = 0 corresponds to the null string.
913 ];
909 - const stringTableSize = operations[i++];
914 + const stringTableSize = operations[i];
915 + i++;
916 +
917 const stringTableEnd = i + stringTableSize;
918 +
919 while (i < stringTableEnd) {
912 - const nextLength = operations[i++];
913 - const nextString = utfDecodeString(
914 - (operations.slice(i, i + nextLength): any),
915 - );
920 + const nextLength = operations[i];
921 + i++;
922 +
923 + const nextString = utfDecodeString(operations.slice(i, i + nextLength));
924 stringTable.push(nextString);
925 i += nextLength;
926 }
@@ -921,7 +929,7 @@ export default class Store extends EventEmitter<{
929 const operation = operations[i];
930 switch (operation) {
931 case TREE_OPERATION_ADD: {
924 - const id = ((operations[i + 1]: any): number);
932 + const id = operations[i + 1];
933 const type = ((operations[i + 2]: any): ElementType);
934
935 i += 3;
@@ -934,8 +942,6 @@ export default class Store extends EventEmitter<{
942 );
943 }
944
937 - let ownerID: number = 0;
938 - let parentID: number = ((null: any): number);
945 if (type === ElementTypeRoot) {
946 if (__DEBUG__) {
947 debug('Add', `new root node ${id}`);
@@ -997,10 +1003,10 @@ export default class Store extends EventEmitter<{
1003
1004 haveRootsChanged = true;
1005 } else {
1000 - parentID = ((operations[i]: any): number);
1006 + const parentID = operations[i];
1007 i++;
1008
1003 - ownerID = ((operations[i]: any): number);
1009 + const ownerID = operations[i];
1010 i++;
1011
1012 const displayNameStringID = operations[i];
@@ -1018,17 +1024,17 @@ export default class Store extends EventEmitter<{
1024 );
1025 }
1026
1021 - if (!this._idToElement.has(parentID)) {
1027 + const parentElement = this._idToElement.get(parentID);
1028 + if (parentElement === undefined) {
1029 this._throwAndEmitError(
1030 Error(
1031 `Cannot add child "${id}" to parent "${parentID}" because parent node was not found in the Store.`,
1032 ),
1033 );
1034 +
1035 + continue;
1036 }
1037
1029 - const parentElement = ((this._idToElement.get(
1030 - parentID,
1031 - ): any): Element);
1038 parentElement.children.push(id);
1039
1040 const [displayNameWithoutHOCs, hocDisplayNames] =
@@ -1065,23 +1071,25 @@ export default class Store extends EventEmitter<{
1071 break;
1072 }
1073 case TREE_OPERATION_REMOVE: {
1068 - const removeLength = ((operations[i + 1]: any): number);
1074 + const removeLength = operations[i + 1];
1075 i += 2;
1076
1077 for (let removeIndex = 0; removeIndex < removeLength; removeIndex++) {
1072 - const id = ((operations[i]: any): number);
1078 + const id = operations[i];
1079 + const element = this._idToElement.get(id);
1080
1074 - if (!this._idToElement.has(id)) {
1081 + if (element === undefined) {
1082 this._throwAndEmitError(
1083 Error(
1084 `Cannot remove node "${id}" because no matching node was found in the Store.`,
1085 ),
1086 );
1087 +
1088 + continue;
1089 }
1090
1091 i += 1;
1092
1084 - const element = ((this._idToElement.get(id): any): Element);
1093 const {children, ownerID, parentID, weight} = element;
1094 if (children.length > 0) {
1095 this._throwAndEmitError(
@@ -1091,7 +1099,7 @@ export default class Store extends EventEmitter<{
1099
1100 this._idToElement.delete(id);
1101
1094 - let parentElement = null;
1102 + let parentElement: ?Element = null;
1103 if (parentID === 0) {
1104 if (__DEBUG__) {
1105 debug('Remove', `node ${id} root`);
@@ -1106,14 +1114,18 @@ export default class Store extends EventEmitter<{
1114 if (__DEBUG__) {
1115 debug('Remove', `node ${id} from parent ${parentID}`);
1116 }
1109 - parentElement = ((this._idToElement.get(parentID): any): Element);
1117 +
1118 + parentElement = this._idToElement.get(parentID);
1119 if (parentElement === undefined) {
1120 this._throwAndEmitError(
1121 Error(
1122 `Cannot remove node "${id}" from parent "${parentID}" because no matching node was found in the Store.`,
1123 ),
1124 );
1125 +
1126 + continue;
1127 }
1128 +
1129 const index = parentElement.children.indexOf(id);
1130 parentElement.children.splice(index, 1);
1131 }
@@ -1167,19 +1179,21 @@ export default class Store extends EventEmitter<{
1179 break;
1180 }
1181 case TREE_OPERATION_REORDER_CHILDREN: {
1170 - const id = ((operations[i + 1]: any): number);
1171 - const numChildren = ((operations[i + 2]: any): number);
1182 + const id = operations[i + 1];
1183 + const numChildren = operations[i + 2];
1184 i += 3;
1185
1174 - if (!this._idToElement.has(id)) {
1186 + const element = this._idToElement.get(id);
1187 + if (element === undefined) {
1188 this._throwAndEmitError(
1189 Error(
1190 `Cannot reorder children for node "${id}" because no matching node was found in the Store.`,
1191 ),
1192 );
1193 +
1194 + continue;
1195 }
1196
1182 - const element = ((this._idToElement.get(id): any): Element);
1197 const children = element.children;
1198 if (children.length !== numChildren) {
1199 this._throwAndEmitError(