fix inspecting an element in a nested renderer bug (#24116)
Fixes this issue, where inspecting components in nested renderers results in an error. The reason for this is because we have different fiberToIDMap instances for each renderer, and owners of a component could be in different renderers. This fix moves the fiberToIDMap and idToArbitraryFiberMap out of the attach method so there's only one instance of each for all renderers.
Luna Ruan committed
Mar 17, 2022 at 15:40 UTC
645ec5d6fce7926e1d23105a93621147ec3d7f17
2 files changed
+67
-11
packages/react-devtools-shared/src/__tests__/inspectedElement-test.js
+56
@@ -2734,6 +2734,62 @@ describe('InspectedElement', () => {
2734
});
2735
});
2736
2737
+ it('inspecting nested renderers should not throw', async () => {
2738
+ // Ignoring react art warnings
2739
+ spyOn(console, 'error');
2740
+ const ReactArt = require('react-art');
2741
+ const ArtSVGMode = require('art/modes/svg');
2742
+ const ARTCurrentMode = require('art/modes/current');
2743
+ store.componentFilters = [];
2744
+
2745
+ ARTCurrentMode.setCurrent(ArtSVGMode);
2746
+ const {Surface, Group} = ReactArt;
2747
+
2748
+ function Child() {
2749
+ return (
2750
+ <Surface width={1} height={1}>
2751
+ <Group />
2752
+ </Surface>
2753
+ );
2754
+ }
2755
+ function App() {
2756
+ return <Child />;
2757
+ }
2758
+
2759
+ await utils.actAsync(() => {
2760
+ legacyRender(<App />, document.createElement('div'));
2761
+ });
2762
+ expect(store).toMatchInlineSnapshot(`
2763
+ [root]
2764
+ ▾ <App>
2765
+ ▾ <Child>
2766
+ ▾ <Surface>
2767
+ <svg>
2768
+ [root]
2769
+ <Group>
2770
+ `);
2771
+
2772
+ const inspectedElement = await inspectElementAtIndex(4);
2773
+ expect(inspectedElement.owners).toMatchInlineSnapshot(`
2774
+ Array [
2775
+ Object {
2776
+ "displayName": "Child",
2777
+ "hocDisplayNames": null,
2778
+ "id": 3,
2779
+ "key": null,
2780
+ "type": 5,
2781
+ },
2782
+ Object {
2783
+ "displayName": "App",
2784
+ "hocDisplayNames": null,
2785
+ "id": 2,
2786
+ "key": null,
2787
+ "type": 5,
2788
+ },
2789
+ ]
2790
+ `);
2791
+ });
2792
+
2793
describe('error boundary', () => {
2794
it('can toggle error', async () => {
2795
class LocalErrorBoundary extends React.Component<any> {
packages/react-devtools-shared/src/backend/renderer.js
+11
-11
@@ -548,6 +548,17 @@ export function getInternalReactConstants(
548
};
549
}
550
551
+// Map of one or more Fibers in a pair to their unique id number.
552
+// We track both Fibers to support Fast Refresh,
553
+// which may forcefully replace one of the pair as part of hot reloading.
554
+// In that case it's still important to be able to locate the previous ID during subsequent renders.
555
+const fiberToIDMap: Map<Fiber, number> = new Map();
556
+
557
+// Map of id to one (arbitrary) Fiber in a pair.
558
+// This Map is used to e.g. get the display name for a Fiber or schedule an update,
559
+// operations that should be the same whether the current and work-in-progress Fiber is used.
560
+const idToArbitraryFiberMap: Map<number, Fiber> = new Map();
561
+
562
export function attach(
563
hook: DevToolsHook,
564
rendererID: number,
@@ -1085,17 +1096,6 @@ export function attach(
1096
}
1097
}
1098
1088
- // Map of one or more Fibers in a pair to their unique id number.
1089
- // We track both Fibers to support Fast Refresh,
1090
- // which may forcefully replace one of the pair as part of hot reloading.
1091
- // In that case it's still important to be able to locate the previous ID during subsequent renders.
1092
- const fiberToIDMap: Map<Fiber, number> = new Map();
1093
-
1094
- // Map of id to one (arbitrary) Fiber in a pair.
1095
- // This Map is used to e.g. get the display name for a Fiber or schedule an update,
1096
- // operations that should be the same whether the current and work-in-progress Fiber is used.
1097
- const idToArbitraryFiberMap: Map<number, Fiber> = new Map();
1098
-
1099
// When profiling is supported, we store the latest tree base durations for each Fiber.
1100
// This is so that we can quickly capture a snapshot of those values if profiling starts.
1101
// If we didn't store these values, we'd have to crawl the tree when profiling started,