Skip to content

Commit 11cfe0d

Browse files
sebmarkbagesebmarkbage
committed
Restore selection from last inspected when updating component filters
This uses the tracked path to select the nearest one instead of relying on a specific id still existing since it'll be remounted. It might get filtered so it's best to select nearest path anyway. I need to move the test into inspected element since we rely on the inspected element being changed to track the current selected path.
1 parent a76ce18 commit 11cfe0d

File tree

4 files changed

+155
-86
lines changed

4 files changed

+155
-86
lines changed

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

+134
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ describe('InspectedElement', () => {
3434
let SettingsContextController;
3535
let StoreContext;
3636
let TreeContextController;
37+
let TreeStateContext;
38+
let TreeDispatcherContext;
3739

3840
let TestUtilsAct;
3941
let TestRendererAct;
@@ -73,6 +75,10 @@ describe('InspectedElement', () => {
7375
require('react-devtools-shared/src/devtools/views/context').StoreContext;
7476
TreeContextController =
7577
require('react-devtools-shared/src/devtools/views/Components/TreeContext').TreeContextController;
78+
TreeStateContext =
79+
require('react-devtools-shared/src/devtools/views/Components/TreeContext').TreeStateContext;
80+
TreeDispatcherContext =
81+
require('react-devtools-shared/src/devtools/views/Components/TreeContext').TreeDispatcherContext;
7682

7783
// Used by inspectElementAtIndex() helper function
7884
utils.act(() => {
@@ -3016,4 +3022,132 @@ describe('InspectedElement', () => {
30163022
);
30173023
});
30183024
});
3025+
3026+
it('should properly handle when components filters are updated', async () => {
3027+
const Wrapper = ({children}) => children;
3028+
3029+
let state;
3030+
let dispatch;
3031+
const Capture = () => {
3032+
dispatch = React.useContext(TreeDispatcherContext);
3033+
state = React.useContext(TreeStateContext);
3034+
return null;
3035+
};
3036+
3037+
function Child({logError = false, logWarning = false}) {
3038+
if (logError === true) {
3039+
console.error('test-only: error');
3040+
}
3041+
if (logWarning === true) {
3042+
console.warn('test-only: warning');
3043+
}
3044+
return null;
3045+
}
3046+
3047+
async function selectNextErrorOrWarning() {
3048+
await utils.actAsync(
3049+
() =>
3050+
dispatch({type: 'SELECT_NEXT_ELEMENT_WITH_ERROR_OR_WARNING_IN_TREE'}),
3051+
false,
3052+
);
3053+
}
3054+
3055+
async function selectPreviousErrorOrWarning() {
3056+
await utils.actAsync(
3057+
() =>
3058+
dispatch({
3059+
type: 'SELECT_PREVIOUS_ELEMENT_WITH_ERROR_OR_WARNING_IN_TREE',
3060+
}),
3061+
false,
3062+
);
3063+
}
3064+
3065+
withErrorsOrWarningsIgnored(['test-only:'], () =>
3066+
utils.act(() =>
3067+
render(
3068+
<React.Fragment>
3069+
<Wrapper>
3070+
<Child logWarning={true} />
3071+
</Wrapper>
3072+
<Wrapper>
3073+
<Wrapper>
3074+
<Child logWarning={true} />
3075+
</Wrapper>
3076+
</Wrapper>
3077+
</React.Fragment>,
3078+
),
3079+
),
3080+
);
3081+
3082+
utils.act(() =>
3083+
TestRenderer.create(
3084+
<Contexts>
3085+
<Capture />
3086+
</Contexts>,
3087+
),
3088+
);
3089+
expect(state).toMatchInlineSnapshot(`
3090+
✕ 0, ⚠ 2
3091+
[root]
3092+
▾ <Wrapper>
3093+
<Child> ⚠
3094+
▾ <Wrapper>
3095+
▾ <Wrapper>
3096+
<Child> ⚠
3097+
`);
3098+
3099+
await selectNextErrorOrWarning();
3100+
expect(state).toMatchInlineSnapshot(`
3101+
✕ 0, ⚠ 2
3102+
[root]
3103+
▾ <Wrapper>
3104+
→ <Child> ⚠
3105+
▾ <Wrapper>
3106+
▾ <Wrapper>
3107+
<Child> ⚠
3108+
`);
3109+
3110+
await utils.actAsync(() => {
3111+
store.componentFilters = [utils.createDisplayNameFilter('Wrapper')];
3112+
}, false);
3113+
3114+
expect(state).toMatchInlineSnapshot(`
3115+
✕ 0, ⚠ 2
3116+
[root]
3117+
→ <Child> ⚠
3118+
<Child> ⚠
3119+
`);
3120+
3121+
await selectNextErrorOrWarning();
3122+
expect(state).toMatchInlineSnapshot(`
3123+
✕ 0, ⚠ 2
3124+
[root]
3125+
<Child> ⚠
3126+
→ <Child> ⚠
3127+
`);
3128+
3129+
await utils.actAsync(() => {
3130+
store.componentFilters = [];
3131+
}, false);
3132+
expect(state).toMatchInlineSnapshot(`
3133+
✕ 0, ⚠ 2
3134+
[root]
3135+
▾ <Wrapper>
3136+
<Child> ⚠
3137+
▾ <Wrapper>
3138+
▾ <Wrapper>
3139+
→ <Child> ⚠
3140+
`);
3141+
3142+
await selectPreviousErrorOrWarning();
3143+
expect(state).toMatchInlineSnapshot(`
3144+
✕ 0, ⚠ 2
3145+
[root]
3146+
▾ <Wrapper>
3147+
→ <Child> ⚠
3148+
▾ <Wrapper>
3149+
▾ <Wrapper>
3150+
<Child> ⚠
3151+
`);
3152+
});
30193153
});

‎packages/react-devtools-shared/src/__tests__/treeContext-test.js

-85
Original file line numberDiff line numberDiff line change
@@ -2286,91 +2286,6 @@ describe('TreeListContext', () => {
22862286
`);
22872287
});
22882288

2289-
it('should properly handle when components filters are updated', () => {
2290-
const Wrapper = ({children}) => children;
2291-
2292-
withErrorsOrWarningsIgnored(['test-only:'], () =>
2293-
utils.act(() =>
2294-
render(
2295-
<React.Fragment>
2296-
<Wrapper>
2297-
<Child logWarning={true} />
2298-
</Wrapper>
2299-
<Wrapper>
2300-
<Wrapper>
2301-
<Child logWarning={true} />
2302-
</Wrapper>
2303-
</Wrapper>
2304-
</React.Fragment>,
2305-
),
2306-
),
2307-
);
2308-
2309-
utils.act(() => TestRenderer.create(<Contexts />));
2310-
expect(state).toMatchInlineSnapshot(`
2311-
✕ 0, ⚠ 2
2312-
[root]
2313-
▾ <Wrapper>
2314-
<Child> ⚠
2315-
▾ <Wrapper>
2316-
▾ <Wrapper>
2317-
<Child> ⚠
2318-
`);
2319-
2320-
selectNextErrorOrWarning();
2321-
expect(state).toMatchInlineSnapshot(`
2322-
✕ 0, ⚠ 2
2323-
[root]
2324-
▾ <Wrapper>
2325-
→ <Child> ⚠
2326-
▾ <Wrapper>
2327-
▾ <Wrapper>
2328-
<Child> ⚠
2329-
`);
2330-
2331-
utils.act(() => {
2332-
store.componentFilters = [utils.createDisplayNameFilter('Wrapper')];
2333-
});
2334-
expect(state).toMatchInlineSnapshot(`
2335-
✕ 0, ⚠ 2
2336-
[root]
2337-
→ <Child> ⚠
2338-
<Child> ⚠
2339-
`);
2340-
2341-
selectNextErrorOrWarning();
2342-
expect(state).toMatchInlineSnapshot(`
2343-
✕ 0, ⚠ 2
2344-
[root]
2345-
<Child> ⚠
2346-
→ <Child> ⚠
2347-
`);
2348-
2349-
utils.act(() => {
2350-
store.componentFilters = [];
2351-
});
2352-
expect(state).toMatchInlineSnapshot(`
2353-
✕ 0, ⚠ 2
2354-
[root]
2355-
▾ <Wrapper>
2356-
<Child> ⚠
2357-
▾ <Wrapper>
2358-
▾ <Wrapper>
2359-
→ <Child> ⚠
2360-
`);
2361-
2362-
selectPreviousErrorOrWarning();
2363-
expect(state).toMatchInlineSnapshot(`
2364-
✕ 0, ⚠ 2
2365-
[root]
2366-
▾ <Wrapper>
2367-
→ <Child> ⚠
2368-
▾ <Wrapper>
2369-
▾ <Wrapper>
2370-
<Child> ⚠
2371-
`);
2372-
});
2373-
23742289
it('should preserve errors for fibers even if they are filtered out of the tree initially', () => {
23752290
const Wrapper = ({children}) => children;
23762291

‎packages/react-devtools-shared/src/backend/agent.js

+14-1
Original file line numberDiff line numberDiff line change
@@ -795,10 +795,23 @@ export default class Agent extends EventEmitter<{
795795

796796
updateComponentFilters: (componentFilters: Array<ComponentFilter>) => void =
797797
componentFilters => {
798-
for (const rendererID in this._rendererInterfaces) {
798+
for (const rendererIDString in this._rendererInterfaces) {
799+
const rendererID = +rendererIDString;
799800
const renderer = ((this._rendererInterfaces[
800801
(rendererID: any)
801802
]: any): RendererInterface);
803+
if (this._lastSelectedRendererID === rendererID) {
804+
// Changing component filters will unmount and remount the DevTools tree.
805+
// Track the last selection's path so we can restore the selection.
806+
const path = renderer.getPathForElement(this._lastSelectedElementID);
807+
if (path !== null) {
808+
renderer.setTrackedPath(path);
809+
this._persistedSelection = {
810+
rendererID,
811+
path,
812+
};
813+
}
814+
}
802815
renderer.updateComponentFilters(componentFilters);
803816
}
804817
};

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

+7
Original file line numberDiff line numberDiff line change
@@ -1169,6 +1169,13 @@ export function attach(
11691169
if (alternate) {
11701170
fiberToFiberInstanceMap.set(alternate, newRoot);
11711171
}
1172+
1173+
// Before the traversals, remember to start tracking
1174+
// our path in case we have selection to restore.
1175+
if (trackedPath !== null) {
1176+
mightBeOnTrackedPath = true;
1177+
}
1178+
11721179
currentRootID = newRoot.id;
11731180
setRootPseudoKey(currentRootID, root.current);
11741181
mountFiberRecursively(root.current, false);

0 commit comments

Comments
 (0)