From 7160f6f584d3eb487230fe64b4a76e5086eb7cec Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 9 May 2019 18:11:17 -0700 Subject: [PATCH] Fixed owners stack direction and added current element to stack --- .../ownersListContext-test.js.snap | 34 ++++++++++++-- src/__tests__/ownersListContext-test.js | 44 +++++++++++++++++++ src/backend/renderer.js | 27 ++++++++++-- src/devtools/views/Components/OwnersStack.js | 12 ++--- .../views/Components/SelectedElement.js | 2 +- src/devtools/views/Components/types.js | 2 +- 6 files changed, 106 insertions(+), 15 deletions(-) diff --git a/src/__tests__/__snapshots__/ownersListContext-test.js.snap b/src/__tests__/__snapshots__/ownersListContext-test.js.snap index 74c908afe8..0f1e213a05 100644 --- a/src/__tests__/__snapshots__/ownersListContext-test.js.snap +++ b/src/__tests__/__snapshots__/ownersListContext-test.js.snap @@ -9,13 +9,17 @@ exports[`OwnersListContext should fetch the owners list for the selected element exports[`OwnersListContext should fetch the owners list for the selected element that includes filtered components: owners for "Child" 1`] = ` Array [ + Object { + "displayName": "Grandparent", + "id": 7, + }, Object { "displayName": "Parent", "id": 9, }, Object { - "displayName": "Grandparent", - "id": 7, + "displayName": "Child", + "id": 8, }, ] `; @@ -30,13 +34,17 @@ exports[`OwnersListContext should fetch the owners list for the selected element exports[`OwnersListContext should fetch the owners list for the selected element: owners for "Child" 1`] = ` Array [ + Object { + "displayName": "Grandparent", + "id": 2, + }, Object { "displayName": "Parent", "id": 3, }, Object { - "displayName": "Grandparent", - "id": 2, + "displayName": "Child", + "id": 4, }, ] `; @@ -47,5 +55,23 @@ Array [ "displayName": "Grandparent", "id": 2, }, + Object { + "displayName": "Parent", + "id": 3, + }, +] +`; + +exports[`OwnersListContext should include the current element even if there are no other owners: mount 1`] = ` +[root] + +`; + +exports[`OwnersListContext should include the current element even if there are no other owners: owners for "Grandparent" 1`] = ` +Array [ + Object { + "displayName": "Grandparent", + "id": 5, + }, ] `; diff --git a/src/__tests__/ownersListContext-test.js b/src/__tests__/ownersListContext-test.js index 06464e663a..bf92977ed5 100644 --- a/src/__tests__/ownersListContext-test.js +++ b/src/__tests__/ownersListContext-test.js @@ -163,4 +163,48 @@ describe('OwnersListContext', () => { done(); }); + + it('should include the current element even if there are no other owners', async done => { + store.componentFilters = [utils.createDisplayNameFilter('^Parent$')]; + + const Grandparent = () => ; + const Parent = () => null; + + utils.act(() => + ReactDOM.render(, document.createElement('div')) + ); + + expect(store).toMatchSnapshot('mount'); + + const grandparent = ((store.getElementAtIndex(0): any): Element); + + let didFinish = false; + + function Suspender({ owner }) { + const read = React.useContext(OwnersListContext); + const owners = read(owner.id); + expect(owners).toMatchSnapshot( + `owners for "${(owner && owner.displayName) || ''}"` + ); + didFinish = true; + return null; + } + + await utils.actSuspense( + () => + TestRenderer.create( + + + + + + ), + 3 + ); + expect(didFinish).toBe(true); + + done(); + }); + + // TODO (owners) Verify that if an Element in the list is unmounted the stack is updated. }); diff --git a/src/backend/renderer.js b/src/backend/renderer.js index cc49385c8f..b76ce8a7d6 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -1696,12 +1696,17 @@ export function attach( const { _debugOwner } = fiber; - let owners = null; + const owners = [ + { + displayName: getDisplayNameForFiber(fiber) || 'Unknown', + id, + }, + ]; + if (_debugOwner) { - owners = []; let owner = _debugOwner; while (owner !== null) { - owners.push({ + owners.unshift({ displayName: getDisplayNameForFiber(owner) || 'Unknown', id: getFiberID(getPrimaryFiber(owner)), }); @@ -1719,6 +1724,7 @@ export function attach( } const { + _debugOwner, _debugSource, stateNode, memoizedProps, @@ -1790,6 +1796,19 @@ export function attach( context = { value: context }; } + let owners = null; + if (_debugOwner) { + owners = []; + let owner = _debugOwner; + while (owner !== null) { + owners.push({ + displayName: getDisplayNameForFiber(owner) || 'Unknown', + id: getFiberID(getPrimaryFiber(owner)), + }); + owner = owner._debugOwner || null; + } + } + const isTimedOutSuspense = tag === SuspenseComponent && memoizedState !== null; @@ -1825,7 +1844,7 @@ export function attach( state: usesHooks ? null : memoizedState, // List of owners - owners: getOwnersList(id), + owners, // Location of component in source coude. source: _debugSource, diff --git a/src/devtools/views/Components/OwnersStack.js b/src/devtools/views/Components/OwnersStack.js index 77ec8eed60..3883a14480 100644 --- a/src/devtools/views/Components/OwnersStack.js +++ b/src/devtools/views/Components/OwnersStack.js @@ -45,7 +45,7 @@ type State = {| function dialogReducer(state, action) { switch (action.type) { case 'UPDATE_OWNER_ID': - const selectedIndex = state.owners.findIndex( + const selectedIndex = action.owners.findIndex( owner => owner.id === action.ownerID ); return { @@ -71,10 +71,11 @@ export default function OwnerStack() { const [state, dispatch] = useReducer(dialogReducer, { ownerID: null, owners: [], - selectedIndex: -1, + selectedIndex: 0, }); - // TODO (owners) Explain this and use reducer with ownerID too to avoid inf. loop + // When an owner is selected, we either need to update the selected index, or we need to fetch a new list of owners. + // We use a reducer here so that we can avoid fetching a new list unless the owner ID has actually changed. if (ownerID === null) { dispatch({ type: 'UPDATE_OWNER_ID', @@ -82,11 +83,12 @@ export default function OwnerStack() { owners: [], }); } else if (ownerID !== state.ownerID) { - const isInList = state.owners.findIndex(owner => owner.id === ownerID) >= 0; + const isInStore = + state.owners.findIndex(owner => owner.id === ownerID) >= 0; dispatch({ type: 'UPDATE_OWNER_ID', ownerID, - owners: isInList ? state.owners : read(ownerID) || [], + owners: isInStore ? state.owners : read(ownerID) || [], }); } diff --git a/src/devtools/views/Components/SelectedElement.js b/src/devtools/views/Components/SelectedElement.js index 4ead00425a..b49ce87ad8 100644 --- a/src/devtools/views/Components/SelectedElement.js +++ b/src/devtools/views/Components/SelectedElement.js @@ -304,7 +304,7 @@ function InspectedElementView({ {owners.map(owner => ( diff --git a/src/devtools/views/Components/types.js b/src/devtools/views/Components/types.js index 0e33349a47..aaae8d378a 100644 --- a/src/devtools/views/Components/types.js +++ b/src/devtools/views/Components/types.js @@ -31,7 +31,7 @@ export type Element = {| |}; export type Owner = {| - displayName: string, + displayName: string | null, id: number, |};