Skip to content

Commit f670ff9

Browse files
lunaruanzhengjitf
authored andcommitted
fix inspecting an element in a nested renderer bug (facebook#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.
1 parent fd561b0 commit f670ff9

File tree

2 files changed

+67
-11
lines changed

2 files changed

+67
-11
lines changed

packages/react-devtools-shared/src/__tests__/inspectedElement-test.js

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2734,6 +2734,62 @@ describe('InspectedElement', () => {
27342734
});
27352735
});
27362736

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+
27372793
describe('error boundary', () => {
27382794
it('can toggle error', async () => {
27392795
class LocalErrorBoundary extends React.Component<any> {

packages/react-devtools-shared/src/backend/renderer.js

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,17 @@ export function getInternalReactConstants(
548548
};
549549
}
550550

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+
551562
export function attach(
552563
hook: DevToolsHook,
553564
rendererID: number,
@@ -1085,17 +1096,6 @@ export function attach(
10851096
}
10861097
}
10871098

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-
10991099
// When profiling is supported, we store the latest tree base durations for each Fiber.
11001100
// This is so that we can quickly capture a snapshot of those values if profiling starts.
11011101
// If we didn't store these values, we'd have to crawl the tree when profiling started,

0 commit comments

Comments
 (0)