Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DevTools] Further Refactoring of Unmounts #30658

Merged
merged 8 commits into from
Aug 12, 2024
140 changes: 137 additions & 3 deletions packages/react-devtools-shared/src/__tests__/inspectedElement-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ describe('InspectedElement', () => {
let SettingsContextController;
let StoreContext;
let TreeContextController;
let TreeStateContext;
let TreeDispatcherContext;

let TestUtilsAct;
let TestRendererAct;
Expand Down Expand Up @@ -73,6 +75,10 @@ describe('InspectedElement', () => {
require('react-devtools-shared/src/devtools/views/context').StoreContext;
TreeContextController =
require('react-devtools-shared/src/devtools/views/Components/TreeContext').TreeContextController;
TreeStateContext =
require('react-devtools-shared/src/devtools/views/Components/TreeContext').TreeStateContext;
TreeDispatcherContext =
require('react-devtools-shared/src/devtools/views/Components/TreeContext').TreeDispatcherContext;

// Used by inspectElementAtIndex() helper function
utils.act(() => {
Expand Down Expand Up @@ -2142,7 +2148,7 @@ describe('InspectedElement', () => {
"context": null,
"events": undefined,
"hooks": null,
"id": 2,
"id": 4,
"owners": null,
"props": {},
"rootType": "createRoot()",
Expand Down Expand Up @@ -2893,15 +2899,15 @@ describe('InspectedElement', () => {
"compiledWithForget": false,
"displayName": "Child",
"hocDisplayNames": null,
"id": 5,
"id": 8,
"key": null,
"type": 5,
},
{
"compiledWithForget": false,
"displayName": "App",
"hocDisplayNames": null,
"id": 4,
"id": 7,
"key": null,
"type": 5,
},
Expand Down Expand Up @@ -3016,4 +3022,132 @@ describe('InspectedElement', () => {
);
});
});

it('should properly handle when components filters are updated', async () => {
const Wrapper = ({children}) => children;

let state;
let dispatch;
const Capture = () => {
dispatch = React.useContext(TreeDispatcherContext);
state = React.useContext(TreeStateContext);
return null;
};

function Child({logError = false, logWarning = false}) {
if (logError === true) {
console.error('test-only: error');
}
if (logWarning === true) {
console.warn('test-only: warning');
}
return null;
}

async function selectNextErrorOrWarning() {
await utils.actAsync(
() =>
dispatch({type: 'SELECT_NEXT_ELEMENT_WITH_ERROR_OR_WARNING_IN_TREE'}),
false,
);
}

async function selectPreviousErrorOrWarning() {
await utils.actAsync(
() =>
dispatch({
type: 'SELECT_PREVIOUS_ELEMENT_WITH_ERROR_OR_WARNING_IN_TREE',
}),
false,
);
}

withErrorsOrWarningsIgnored(['test-only:'], () =>
utils.act(() =>
render(
<React.Fragment>
<Wrapper>
<Child logWarning={true} />
</Wrapper>
<Wrapper>
<Wrapper>
<Child logWarning={true} />
</Wrapper>
</Wrapper>
</React.Fragment>,
),
),
);

utils.act(() =>
TestRenderer.create(
<Contexts>
<Capture />
</Contexts>,
),
);
expect(state).toMatchInlineSnapshot(`
✕ 0, ⚠ 2
[root]
▾ <Wrapper>
<Child> ⚠
▾ <Wrapper>
▾ <Wrapper>
<Child> ⚠
`);

await selectNextErrorOrWarning();
expect(state).toMatchInlineSnapshot(`
✕ 0, ⚠ 2
[root]
▾ <Wrapper>
→ <Child> ⚠
▾ <Wrapper>
▾ <Wrapper>
<Child> ⚠
`);

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

expect(state).toMatchInlineSnapshot(`
✕ 0, ⚠ 2
[root]
→ <Child> ⚠
<Child> ⚠
`);

await selectNextErrorOrWarning();
expect(state).toMatchInlineSnapshot(`
✕ 0, ⚠ 2
[root]
<Child> ⚠
→ <Child> ⚠
`);

await utils.actAsync(() => {
store.componentFilters = [];
}, false);
expect(state).toMatchInlineSnapshot(`
✕ 0, ⚠ 2
[root]
▾ <Wrapper>
<Child> ⚠
▾ <Wrapper>
▾ <Wrapper>
→ <Child> ⚠
`);

await selectPreviousErrorOrWarning();
expect(state).toMatchInlineSnapshot(`
✕ 0, ⚠ 2
[root]
▾ <Wrapper>
→ <Child> ⚠
▾ <Wrapper>
▾ <Wrapper>
<Child> ⚠
`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ describe('OwnersListContext', () => {
expect(await getOwnersListForOwner(firstChild)).toMatchInlineSnapshot(`
[
"Grandparent",
"Parent",
"Child",
]
`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1251,17 +1251,17 @@ describe('ProfilingCache', () => {
Map {
1 => 16,
2 => 16,
3 => 1,
4 => 1,
6 => 1,
}
`);

expect(commitData[0].fiberSelfDurations).toMatchInlineSnapshot(`
Map {
1 => 0,
2 => 10,
3 => 1,
4 => 1,
6 => 1,
}
`);
});
Expand Down Expand Up @@ -1322,13 +1322,13 @@ describe('ProfilingCache', () => {
`);
expect(commitData[1].fiberActualDurations).toMatchInlineSnapshot(`
Map {
7 => 3,
5 => 3,
3 => 3,
}
`);
expect(commitData[1].fiberSelfDurations).toMatchInlineSnapshot(`
Map {
7 => 3,
5 => 3,
3 => 0,
}
`);
Expand Down
85 changes: 0 additions & 85 deletions packages/react-devtools-shared/src/__tests__/treeContext-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2286,91 +2286,6 @@ describe('TreeListContext', () => {
`);
});

it('should properly handle when components filters are updated', () => {
const Wrapper = ({children}) => children;

withErrorsOrWarningsIgnored(['test-only:'], () =>
utils.act(() =>
render(
<React.Fragment>
<Wrapper>
<Child logWarning={true} />
</Wrapper>
<Wrapper>
<Wrapper>
<Child logWarning={true} />
</Wrapper>
</Wrapper>
</React.Fragment>,
),
),
);

utils.act(() => TestRenderer.create(<Contexts />));
expect(state).toMatchInlineSnapshot(`
✕ 0, ⚠ 2
[root]
▾ <Wrapper>
<Child> ⚠
▾ <Wrapper>
▾ <Wrapper>
<Child> ⚠
`);

selectNextErrorOrWarning();
expect(state).toMatchInlineSnapshot(`
✕ 0, ⚠ 2
[root]
▾ <Wrapper>
→ <Child> ⚠
▾ <Wrapper>
▾ <Wrapper>
<Child> ⚠
`);

utils.act(() => {
store.componentFilters = [utils.createDisplayNameFilter('Wrapper')];
});
expect(state).toMatchInlineSnapshot(`
✕ 0, ⚠ 2
[root]
→ <Child> ⚠
<Child> ⚠
`);

selectNextErrorOrWarning();
expect(state).toMatchInlineSnapshot(`
✕ 0, ⚠ 2
[root]
<Child> ⚠
→ <Child> ⚠
`);

utils.act(() => {
store.componentFilters = [];
});
expect(state).toMatchInlineSnapshot(`
✕ 0, ⚠ 2
[root]
▾ <Wrapper>
<Child> ⚠
▾ <Wrapper>
▾ <Wrapper>
→ <Child> ⚠
`);

selectPreviousErrorOrWarning();
expect(state).toMatchInlineSnapshot(`
✕ 0, ⚠ 2
[root]
▾ <Wrapper>
→ <Child> ⚠
▾ <Wrapper>
▾ <Wrapper>
<Child> ⚠
`);
});

it('should preserve errors for fibers even if they are filtered out of the tree initially', () => {
const Wrapper = ({children}) => children;

Expand Down
15 changes: 14 additions & 1 deletion packages/react-devtools-shared/src/backend/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -795,10 +795,23 @@ export default class Agent extends EventEmitter<{

updateComponentFilters: (componentFilters: Array<ComponentFilter>) => void =
componentFilters => {
for (const rendererID in this._rendererInterfaces) {
for (const rendererIDString in this._rendererInterfaces) {
const rendererID = +rendererIDString;
const renderer = ((this._rendererInterfaces[
(rendererID: any)
]: any): RendererInterface);
if (this._lastSelectedRendererID === rendererID) {
// Changing component filters will unmount and remount the DevTools tree.
// Track the last selection's path so we can restore the selection.
const path = renderer.getPathForElement(this._lastSelectedElementID);
if (path !== null) {
renderer.setTrackedPath(path);
this._persistedSelection = {
rendererID,
path,
};
}
}
renderer.updateComponentFilters(componentFilters);
}
};
Expand Down
Loading
Loading