Skip to content

Commit

Permalink
DevTools: merge element fields in TreeStateContext
Browse files Browse the repository at this point in the history
  • Loading branch information
@hoxyq
hoxyq committed Dec 23, 2024
1 parent be4a4cc commit b1ea47d
Show file tree
Hide file tree
Showing 9 changed files with 230 additions and 266 deletions.
25 changes: 13 additions & 12 deletions packages/react-devtools-shared/src/__tests__/inspectedElement-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,16 +115,15 @@ describe('InspectedElement', () => {

const Contexts = ({
children,
defaultSelectedElementID = null,
defaultSelectedElementIndex = null,
defaultInspectedElementID = null,
defaultInspectedElementIndex = null,
}) => (
<BridgeContext.Provider value={bridge}>
<StoreContext.Provider value={store}>
<SettingsContextController>
<TreeContextController
defaultSelectedElementID={defaultSelectedElementID}
defaultSelectedElementIndex={defaultSelectedElementIndex}
defaultInspectedElementID={defaultSelectedElementID}>
defaultInspectedElementID={defaultInspectedElementID}
defaultInspectedElementIndex={defaultInspectedElementIndex}>
<InspectedElementContextController>
{children}
</InspectedElementContextController>
Expand Down Expand Up @@ -167,8 +166,8 @@ describe('InspectedElement', () => {
testRendererInstance.update(
<ErrorBoundary>
<Contexts
defaultSelectedElementID={id}
defaultSelectedElementIndex={index}>
defaultInspectedElementID={id}
defaultInspectedElementIndex={index}>
<React.Suspense fallback={null}>
<Suspender id={id} index={index} />
</React.Suspense>
Expand Down Expand Up @@ -355,7 +354,7 @@ describe('InspectedElement', () => {
const {index, shouldHaveLegacyContext} = cases[i];

// HACK: Recreate TestRenderer instance because we rely on default state values
// from props like defaultSelectedElementID and it's easier to reset here than
// from props like defaultInspectedElementID and it's easier to reset here than
// to read the TreeDispatcherContext and update the selected ID that way.
// We're testing the inspected values here, not the context wiring, so that's ok.
withErrorsOrWarningsIgnored(
Expand Down Expand Up @@ -2069,7 +2068,7 @@ describe('InspectedElement', () => {
}, false);

// HACK: Recreate TestRenderer instance because we rely on default state values
// from props like defaultSelectedElementID and it's easier to reset here than
// from props like defaultInspectedElementID and it's easier to reset here than
// to read the TreeDispatcherContext and update the selected ID that way.
// We're testing the inspected values here, not the context wiring, so that's ok.
withErrorsOrWarningsIgnored(
Expand Down Expand Up @@ -2129,7 +2128,7 @@ describe('InspectedElement', () => {
}, false);

// HACK: Recreate TestRenderer instance because we rely on default state values
// from props like defaultSelectedElementID and it's easier to reset here than
// from props like defaultInspectedElementID and it's easier to reset here than
// to read the TreeDispatcherContext and update the selected ID that way.
// We're testing the inspected values here, not the context wiring, so that's ok.
withErrorsOrWarningsIgnored(
Expand Down Expand Up @@ -2408,8 +2407,8 @@ describe('InspectedElement', () => {
await utils.actAsync(() => {
root = TestRenderer.create(
<Contexts
defaultSelectedElementID={id}
defaultSelectedElementIndex={index}>
defaultInspectedElementID={id}
defaultInspectedElementIndex={index}>
<React.Suspense fallback={null}>
<Suspender target={id} />
</React.Suspense>
Expand Down Expand Up @@ -3101,6 +3100,7 @@ describe('InspectedElement', () => {

await utils.actAsync(() => {
store.componentFilters = [utils.createDisplayNameFilter('Wrapper')];
jest.runOnlyPendingTimers();
}, false);

expect(state).toMatchInlineSnapshot(`
Expand All @@ -3120,6 +3120,7 @@ describe('InspectedElement', () => {

await utils.actAsync(() => {
store.componentFilters = [];
jest.runOnlyPendingTimers();
}, false);
expect(state).toMatchInlineSnapshot(`
✕ 0, ⚠ 2
Expand Down
61 changes: 34 additions & 27 deletions packages/react-devtools-shared/src/__tests__/profilerContext-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,14 @@ describe('ProfilerContext', () => {

const Contexts = ({
children = null,
defaultSelectedElementID = null,
defaultSelectedElementIndex = null,
defaultInspectedElementID = null,
defaultInspectedElementIndex = null,
}: any) => (
<BridgeContext.Provider value={bridge}>
<StoreContext.Provider value={store}>
<TreeContextController
defaultSelectedElementID={defaultSelectedElementID}
defaultSelectedElementIndex={defaultSelectedElementIndex}>
defaultInspectedElementID={defaultInspectedElementID}
defaultInspectedElementIndex={defaultInspectedElementIndex}>
<ProfilerContextController>{children}</ProfilerContextController>
</TreeContextController>
</StoreContext.Provider>
Expand Down Expand Up @@ -225,8 +225,8 @@ describe('ProfilerContext', () => {
await utils.actAsync(() =>
TestRenderer.create(
<Contexts
defaultSelectedElementID={store.getElementIDAtIndex(3)}
defaultSelectedElementIndex={3}>
defaultInspectedElementID={store.getElementIDAtIndex(3)}
defaultInspectedElementIndex={3}>
<ContextReader />
</Contexts>,
),
Expand Down Expand Up @@ -276,8 +276,8 @@ describe('ProfilerContext', () => {
await utils.actAsync(() =>
TestRenderer.create(
<Contexts
defaultSelectedElementID={store.getElementIDAtIndex(3)}
defaultSelectedElementIndex={3}>
defaultInspectedElementID={store.getElementIDAtIndex(3)}
defaultInspectedElementIndex={3}>
<ContextReader />
</Contexts>,
),
Expand Down Expand Up @@ -323,8 +323,8 @@ describe('ProfilerContext', () => {
await utils.actAsync(() =>
TestRenderer.create(
<Contexts
defaultSelectedElementID={store.getElementIDAtIndex(3)}
defaultSelectedElementIndex={3}>
defaultInspectedElementID={store.getElementIDAtIndex(3)}
defaultInspectedElementIndex={3}>
<ContextReader />
</Contexts>,
),
Expand Down Expand Up @@ -374,8 +374,8 @@ describe('ProfilerContext', () => {
await utils.actAsync(() =>
TestRenderer.create(
<Contexts
defaultSelectedElementID={store.getElementIDAtIndex(3)}
defaultSelectedElementIndex={3}>
defaultInspectedElementID={store.getElementIDAtIndex(3)}
defaultInspectedElementIndex={3}>
<ContextReader />
</Contexts>,
),
Expand Down Expand Up @@ -415,11 +415,12 @@ describe('ProfilerContext', () => {

let context: Context = ((null: any): Context);
let dispatch: DispatcherContext = ((null: any): DispatcherContext);
let selectedElementID = null;
let inspectedElementID = null;
function ContextReader() {
context = React.useContext(ProfilerContext);
dispatch = React.useContext(TreeDispatcherContext);
selectedElementID = React.useContext(TreeStateContext).selectedElementID;
inspectedElementID =
React.useContext(TreeStateContext).inspectedElementID;
return null;
}

Expand All @@ -428,13 +429,15 @@ describe('ProfilerContext', () => {
// Select an element within the second root.
await utils.actAsync(() =>
TestRenderer.create(
<Contexts defaultSelectedElementID={id} defaultSelectedElementIndex={3}>
<Contexts
defaultInspectedElementID={id}
defaultInspectedElementIndex={3}>
<ContextReader />
</Contexts>,
),
);

expect(selectedElementID).toBe(id);
expect(inspectedElementID).toBe(id);

// Profile and record more updates to both roots
await utils.actAsync(() => store.profilerStore.startProfiling());
Expand All @@ -448,7 +451,7 @@ describe('ProfilerContext', () => {
utils.act(() => dispatch({type: 'SELECT_ELEMENT_AT_INDEX', payload: 0}));

// Verify that the initial Profiler root selection is maintained.
expect(selectedElementID).toBe(otherID);
expect(inspectedElementID).toBe(otherID);
expect(context).not.toBeNull();
expect(context.rootID).toBe(store.getRootIDForElement(id));
});
Expand Down Expand Up @@ -484,11 +487,12 @@ describe('ProfilerContext', () => {

let context: Context = ((null: any): Context);
let dispatch: DispatcherContext = ((null: any): DispatcherContext);
let selectedElementID = null;
let inspectedElementID = null;
function ContextReader() {
context = React.useContext(ProfilerContext);
dispatch = React.useContext(TreeDispatcherContext);
selectedElementID = React.useContext(TreeStateContext).selectedElementID;
inspectedElementID =
React.useContext(TreeStateContext).inspectedElementID;
return null;
}

Expand All @@ -497,13 +501,15 @@ describe('ProfilerContext', () => {
// Select an element within the second root.
await utils.actAsync(() =>
TestRenderer.create(
<Contexts defaultSelectedElementID={id} defaultSelectedElementIndex={3}>
<Contexts
defaultInspectedElementID={id}
defaultInspectedElementIndex={3}>
<ContextReader />
</Contexts>,
),
);

expect(selectedElementID).toBe(id);
expect(inspectedElementID).toBe(id);

// Profile and record more updates to both roots
await utils.actAsync(() => store.profilerStore.startProfiling());
Expand All @@ -517,7 +523,7 @@ describe('ProfilerContext', () => {
utils.act(() => dispatch({type: 'SELECT_ELEMENT_AT_INDEX', payload: 0}));

// Verify that the initial Profiler root selection is maintained.
expect(selectedElementID).toBe(otherID);
expect(inspectedElementID).toBe(otherID);
expect(context).not.toBeNull();
expect(context.rootID).toBe(store.getRootIDForElement(id));
});
Expand Down Expand Up @@ -553,10 +559,11 @@ describe('ProfilerContext', () => {
`);

let context: Context = ((null: any): Context);
let selectedElementID = null;
let inspectedElementID = null;
function ContextReader() {
context = React.useContext(ProfilerContext);
selectedElementID = React.useContext(TreeStateContext).selectedElementID;
inspectedElementID =
React.useContext(TreeStateContext).inspectedElementID;
return null;
}

Expand All @@ -567,14 +574,14 @@ describe('ProfilerContext', () => {
</Contexts>,
),
);
expect(selectedElementID).toBeNull();
expect(inspectedElementID).toBeNull();

// Select an element in the Profiler tab and verify that the selection is synced to the Components tab.
await utils.actAsync(() => context.selectFiber(parentID, 'Parent'));
expect(selectedElementID).toBe(parentID);
expect(inspectedElementID).toBe(parentID);

// Select an unmounted element and verify no Components tab selection doesn't change.
await utils.actAsync(() => context.selectFiber(childID, 'Child'));
expect(selectedElementID).toBe(parentID);
expect(inspectedElementID).toBe(parentID);
});
});
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/devtools/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export function printStore(
if (state === null) {
return '';
}
return state.selectedElementIndex === index ? `→` : ' ';
return state.inspectedElementIndex === index ? `→` : ' ';
}

function printErrorsAndWarnings(element: Element): string {
Expand Down
4 changes: 2 additions & 2 deletions packages/react-devtools-shared/src/devtools/views/Components/Element.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type Props = {

export default function Element({data, index, style}: Props): React.Node {
const store = useContext(StoreContext);
const {ownerFlatTree, ownerID, selectedElementID} =
const {ownerFlatTree, ownerID, inspectedElementID} =
useContext(TreeStateContext);
const dispatch = useContext(TreeDispatcherContext);

Expand All @@ -46,7 +46,7 @@ export default function Element({data, index, style}: Props): React.Node {

const {isNavigatingWithKeyboard, onElementMouseEnter, treeFocused} = data;
const id = element === null ? null : element.id;
const isSelected = selectedElementID === id;
const isSelected = inspectedElementID === id;

const errorsAndWarningsSubscription = useMemo(
() => ({
Expand Down
18 changes: 9 additions & 9 deletions packages/react-devtools-shared/src/devtools/views/Components/NativeStyleEditor/context.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,10 @@ function NativeStyleContextController({children}: Props): React.Node {
[store],
);

// It's very important that this context consumes selectedElementID and not NativeStyleID.
// It's very important that this context consumes inspectedElementID and not NativeStyleID.
// Otherwise the effect that sends the "inspect" message across the bridge-
// would itself be blocked by the same render that suspends (waiting for the data).
const {selectedElementID} = useContext<StateContext>(TreeStateContext);
const {inspectedElementID} = useContext<StateContext>(TreeStateContext);

const [currentStyleAndLayout, setCurrentStyleAndLayout] =
useState<StyleAndLayoutFrontend | null>(null);
Expand All @@ -128,7 +128,7 @@ function NativeStyleContextController({children}: Props): React.Node {
resource.write(element, styleAndLayout);

// Schedule update with React if the currently-selected element has been invalidated.
if (id === selectedElementID) {
if (id === inspectedElementID) {
setCurrentStyleAndLayout(styleAndLayout);
}
}
Expand All @@ -141,15 +141,15 @@ function NativeStyleContextController({children}: Props): React.Node {
'NativeStyleEditor_styleAndLayout',
onStyleAndLayout,
);
}, [bridge, currentStyleAndLayout, selectedElementID, store]);
}, [bridge, currentStyleAndLayout, inspectedElementID, store]);

// This effect handler polls for updates on the currently selected element.
useEffect(() => {
if (selectedElementID === null) {
if (inspectedElementID === null) {
return () => {};
}

const rendererID = store.getRendererIDForElement(selectedElementID);
const rendererID = store.getRendererIDForElement(inspectedElementID);

let timeoutID: TimeoutID | null = null;

Expand All @@ -158,7 +158,7 @@ function NativeStyleContextController({children}: Props): React.Node {

if (rendererID !== null) {
bridge.send('NativeStyleEditor_measure', {
id: selectedElementID,
id: inspectedElementID,
rendererID,
});
}
Expand All @@ -170,7 +170,7 @@ function NativeStyleContextController({children}: Props): React.Node {

const onStyleAndLayout = ({id}: StyleAndLayoutBackend) => {
// If this is the element we requested, wait a little bit and then ask for another update.
if (id === selectedElementID) {
if (id === inspectedElementID) {
if (timeoutID !== null) {
clearTimeout(timeoutID);
}
Expand All @@ -190,7 +190,7 @@ function NativeStyleContextController({children}: Props): React.Node {
clearTimeout(timeoutID);
}
};
}, [bridge, selectedElementID, store]);
}, [bridge, inspectedElementID, store]);

const value = useMemo(
() => ({getStyleAndLayout}),
Expand Down
10 changes: 5 additions & 5 deletions packages/react-devtools-shared/src/devtools/views/Components/SelectedTreeHighlight.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,19 @@ export default function SelectedTreeHighlight(_: {}): React.Node {
const {lineHeight} = useContext(SettingsContext);
const store = useContext(StoreContext);
const treeFocused = useContext(TreeFocusedContext);
const {ownerID, selectedElementID} = useContext(TreeStateContext);
const {ownerID, inspectedElementID} = useContext(TreeStateContext);

const subscription = useMemo(
() => ({
getCurrentValue: () => {
if (
selectedElementID === null ||
store.isInsideCollapsedSubTree(selectedElementID)
inspectedElementID === null ||
store.isInsideCollapsedSubTree(inspectedElementID)
) {
return null;
}

const element = store.getElementByID(selectedElementID);
const element = store.getElementByID(inspectedElementID);
if (
element === null ||
element.isCollapsed ||
Expand Down Expand Up @@ -83,7 +83,7 @@ export default function SelectedTreeHighlight(_: {}): React.Node {
};
},
}),
[selectedElementID, store],
[inspectedElementID, store],
);
const data = useSubscription<Data | null>(subscription);

Expand Down
Loading

0 comments on commit b1ea47d

Please sign in to comment.