From d8abecdcf997390eaf22dc1bb20fa3a0e81adf0b Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 23 Apr 2019 15:25:55 +0100 Subject: [PATCH 1/8] Persist and restore selection in agent This implements the infrastructure for saving and restoring renderer-specific selection state in the session storage. Note this doesn't actually implement the calculation and tracking of paths in the renderer. It only simulates that the renderer can do it. The actual implementation will come in a later commit. --- src/backend/agent.js | 98 +++++++++++++++++++++++++++++++++++++++-- src/backend/renderer.js | 44 +++++++++++++++++- src/backend/types.js | 14 ++++++ src/constants.js | 3 ++ 4 files changed, 154 insertions(+), 5 deletions(-) diff --git a/src/backend/agent.js b/src/backend/agent.js index 378a70f271..9f83fff38c 100644 --- a/src/backend/agent.js +++ b/src/backend/agent.js @@ -3,10 +3,19 @@ import EventEmitter from 'events'; import memoize from 'memoize-one'; import throttle from 'lodash.throttle'; -import { LOCAL_STORAGE_RELOAD_AND_PROFILE_KEY, __DEBUG__ } from '../constants'; +import { + LOCAL_STORAGE_RELOAD_AND_PROFILE_KEY, + SESSION_STORAGE_LAST_SELECTION_KEY, + __DEBUG__, +} from '../constants'; import { hideOverlay, showOverlay } from './views/Highlighter'; -import type { RendererID, RendererInterface } from './types'; +import type { + PathFrame, + PathMatch, + RendererID, + RendererInterface, +} from './types'; import type { Bridge } from '../types'; const debug = (methodName, ...args) => { @@ -20,6 +29,11 @@ const debug = (methodName, ...args) => { } }; +type OperationsParams = {| + operations: Uint32Array, + rendererID: number, +|}; + type InspectSelectParams = {| id: number, rendererID: number, @@ -46,10 +60,17 @@ type OverrideSuspenseParams = {| forceFallback: boolean, |}; +type PersistedSelection = {| + rendererID: number, + path: Array, +|}; + export default class Agent extends EventEmitter { _bridge: Bridge = ((null: any): Bridge); _isProfiling: boolean = false; _rendererInterfaces: { [key: RendererID]: RendererInterface } = {}; + _persistedSelection: PersistedSelection | null = null; + _persistedSelectionMatch: PathMatch | null = null; constructor() { super(); @@ -59,6 +80,13 @@ export default class Agent extends EventEmitter { localStorage.removeItem(LOCAL_STORAGE_RELOAD_AND_PROFILE_KEY); } + + const persistedSelectionString = sessionStorage.getItem( + SESSION_STORAGE_LAST_SELECTION_KEY + ); + if (persistedSelectionString != null) { + this._persistedSelection = JSON.parse(persistedSelectionString); + } } addBridge(bridge: Bridge) { @@ -316,6 +344,19 @@ export default class Agent extends EventEmitter { } else { renderer.selectElement(id); this._bridge.send('selectElement'); + + // When user selects an element, stop trying to restore the selection, + // and instead remember the current selection for the next reload. + if ( + this._persistedSelectionMatch === null || + this._persistedSelectionMatch.id !== id + ) { + this._persistedSelection = null; + this._persistedSelectionMatch = null; + renderer.setTrackedPath(null); + this._throttledPersistSelection(rendererID, id); + } + // TODO: If there was a way to change the selected DOM element // in native Elements tab without forcing a switch to it, we'd do it here. // For now, it doesn't seem like there is a way to do that: @@ -388,6 +429,14 @@ export default class Agent extends EventEmitter { if (this._isProfiling) { rendererInterface.startProfiling(); } + + // When the renderer is attached, we need to tell it whether + // we remember the previous selection that we'd like to restore. + // It'll start tracking mounts for matches to the last selection path. + const selection = this._persistedSelection; + if (selection !== null && selection.rendererID === rendererID) { + rendererInterface.setTrackedPath(selection.path); + } } syncSelectionFromNativeElementsPanel = () => { @@ -454,7 +503,7 @@ export default class Agent extends EventEmitter { } }; - onHookOperations = (operations: Uint32Array) => { + onHookOperations = ({ operations, rendererID }: OperationsParams) => { if (__DEBUG__) { debug('onHookOperations', operations); } @@ -480,6 +529,31 @@ export default class Agent extends EventEmitter { // // this._bridge.send('operations', operations, [operations.buffer]); this._bridge.send('operations', operations); + + if (this._persistedSelection !== null) { + if (this._persistedSelection.rendererID === rendererID) { + // Check if we can select a deeper match for the persisted selection. + const renderer = this._rendererInterfaces[rendererID]; + const prevMatch = this._persistedSelectionMatch; + const nextMatch = renderer.getBestMatchForTrackedPath(); + this._persistedSelectionMatch = nextMatch; + const prevMatchID = prevMatch !== null ? prevMatch.id : null; + const nextMatchID = nextMatch !== null ? nextMatch.id : null; + if (prevMatchID !== nextMatchID) { + if (nextMatchID !== null) { + // We moved forward, unlocking a deeper node. + this._bridge.send('selectFiber', nextMatchID); + } + } + if (nextMatch !== null && nextMatch.isFullMatch) { + // We've just unlocked the innermost selected node. + // There's no point tracking it further. + this._persistedSelection = null; + this._persistedSelectionMatch = null; + renderer.setTrackedPath(null); + } + } + } }; _onClick = (event: MouseEvent) => { @@ -530,4 +604,22 @@ export default class Agent extends EventEmitter { // because those are usually unintentional as you lift the cursor. { leading: false } ); + + _throttledPersistSelection = throttle((rendererID: number, id: number) => { + // This is throttled, so both renderer and selected ID + // might not be available by the time we read them. + // This is why we need the defensive checks here. + const renderer = this._rendererInterfaces[rendererID]; + if (renderer == null) { + return; + } + const path = renderer.getPathForElement(id); + if (path === null) { + return; + } + sessionStorage.setItem( + SESSION_STORAGE_LAST_SELECTION_KEY, + JSON.stringify(({ rendererID, path }: PersistedSelection)) + ); + }, 1000); } diff --git a/src/backend/renderer.js b/src/backend/renderer.js index e41ea782ff..caf165dd8e 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -36,6 +36,8 @@ import type { Interaction, Interactions, InteractionWithCommits, + PathFrame, + PathMatch, ProfilingSummary, ReactRenderer, RendererInterface, @@ -664,7 +666,10 @@ export function attach( pendingOperationsQueue.push(ops); } else { // If we've already connected to the frontend, just pass the operations through. - hook.emit('operations', ops); + hook.emit('operations', { + operations: ops, + rendererID, + }); } pendingOperations.length = 0; @@ -1108,7 +1113,10 @@ export function attach( // We may have already queued up some operations before the frontend connected // If so, let the frontend know about them. localPendingOperationsQueue.forEach(ops => { - hook.emit('operations', ops); + hook.emit('operations', { + operations: ops, + rendererID, + }); }); } else { // If we have not been profiling, then we can just walk the tree and build up its current state as-is. @@ -1979,14 +1987,45 @@ export function attach( scheduleUpdate(fiber); } + let trackedPath: Array | null = null; + + function setTrackedPath(path: Array | null) { + trackedPath = path; + } + + function getPathForElement(id: number): Array { + // TODO: this is not a real path. + return [ + { + index: id, + key: null, + displayName: null, + }, + ]; + } + + function getBestMatchForTrackedPath(): PathMatch | null { + // TODO: this is not a real lookup. + if (trackedPath !== null) { + const id = trackedPath[0].index; + const fiber = idToFiberMap.get(id); + if (fiber !== null) { + return { id, isFullMatch: true }; + } + } + return null; + } + return { cleanup, flushInitialOperations, + getBestMatchForTrackedPath, getCommitDetails, getFiberIDFromNative, getFiberCommits, getInteractions, findNativeByFiberID, + getPathForElement, getProfilingDataForDownload, getProfilingSummary, handleCommitFiberRoot, @@ -2001,6 +2040,7 @@ export function attach( setInHook, setInProps, setInState, + setTrackedPath, startProfiling, stopProfiling, }; diff --git a/src/backend/types.js b/src/backend/types.js index bd4562eef7..d77a6c1145 100644 --- a/src/backend/types.js +++ b/src/backend/types.js @@ -90,10 +90,22 @@ export type ProfilingSummary = {| rootID: number, |}; +export type PathFrame = {| + key: string | null, + index: number, + displayName: string | null, +|}; + +export type PathMatch = {| + id: number, + isFullMatch: boolean, +|}; + export type RendererInterface = { cleanup: () => void, findNativeByFiberID: (id: number) => ?NativeType, flushInitialOperations: () => void, + getBestMatchForTrackedPath: () => PathMatch | null, getCommitDetails: (rootID: number, commitIndex: number) => CommitDetails, getFiberIDFromNative: ( component: NativeType, @@ -103,6 +115,7 @@ export type RendererInterface = { getInteractions: (rootID: number) => Interactions, getProfilingDataForDownload: (rootID: number) => Object, getProfilingSummary: (rootID: number) => ProfilingSummary, + getPathForElement: (id: number) => Array, handleCommitFiberRoot: (fiber: Object) => void, handleCommitFiberUnmount: (fiber: Object) => void, inspectElement: (id: number) => InspectedElement | null, @@ -120,6 +133,7 @@ export type RendererInterface = { ) => void, setInProps: (id: number, path: Array, value: any) => void, setInState: (id: number, path: Array, value: any) => void, + setTrackedPath: (path: Array | null) => void, startProfiling: () => void, stopProfiling: () => void, }; diff --git a/src/constants.js b/src/constants.js index d64cc5da32..588c8d4219 100644 --- a/src/constants.js +++ b/src/constants.js @@ -8,4 +8,7 @@ export const TREE_OPERATION_UPDATE_TREE_BASE_DURATION = 4; export const LOCAL_STORAGE_RELOAD_AND_PROFILE_KEY = 'React::DevTools::reloadAndProfile'; +export const SESSION_STORAGE_LAST_SELECTION_KEY = + 'React::DevTools::lastSelection'; + export const __DEBUG__ = false; From b1a7007cc5194d769610abf9de796eb152f161b1 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 23 Apr 2019 15:42:51 +0100 Subject: [PATCH 2/8] Keep track of root insertion order in the renderer Normally, Fibers have key or index which we'll use to match things up between reloads. However, roots don't have such a concept. We'll use their insertion order as an approximation. If it's consistent, we'll be able to restore the selection. --- src/backend/renderer.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/backend/renderer.js b/src/backend/renderer.js index caf165dd8e..7275386270 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -539,6 +539,11 @@ export function attach( // When a mount or update is in progress, this value tracks the root that is being operated on. let currentRootID: number = -1; + // Track the order in which roots were added. + // We will use it to disambiguate roots when restoring selection between reloads. + let nextRootIndex = 0; + const rootInsertionOrder: Map = new Map(); + function getFiberID(primaryFiber: Fiber): number { if (!fiberToIDMap.has(primaryFiber)) { const id = getUID(); @@ -690,6 +695,7 @@ export function attach( const hasOwnerMetadata = fiber.hasOwnProperty('_debugOwner'); if (isRoot) { + rootInsertionOrder.set(id, nextRootIndex++); pushOperation(TREE_OPERATION_ADD); pushOperation(id); pushOperation(ElementTypeRoot); @@ -777,6 +783,7 @@ export function attach( } const id = getFiberID(primaryFiber); if (isRoot) { + rootInsertionOrder.delete(id); // Removing a root needs to happen at the end // so we don't batch it with other unmounts. pushOperation(TREE_OPERATION_REMOVE); From 3bb837b683fcd41f48ca522b09ad7918e7fca6f7 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 23 Apr 2019 16:01:31 +0100 Subject: [PATCH 3/8] Implement getPathForElement to serialize the selected path Note this doesn't restore the selection yet. --- src/backend/renderer.js | 69 +++++++++++++++++++++++++++++++---------- src/backend/types.js | 5 +-- 2 files changed, 56 insertions(+), 18 deletions(-) diff --git a/src/backend/renderer.js b/src/backend/renderer.js index 7275386270..a3cc6272d4 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -324,7 +324,7 @@ export function attach( // TODO: we might want to change the data structure once we no longer suppport Stack versions of `getData`. // TODO: Keep in sync with getElementType() function getDataForFiber(fiber: Fiber): FiberData { - const { elementType, type, key, tag } = fiber; + const { elementType, type, key, index, tag } = fiber; // This is to support lazy components with a Promise as the type. // see https://github.com/facebook/react/pull/13397 @@ -345,6 +345,7 @@ export function attach( fiberData = { displayName: getDisplayName(resolvedType), key, + index, type: ElementTypeClass, }; break; @@ -353,6 +354,7 @@ export function attach( fiberData = { displayName: getDisplayName(resolvedType), key, + index, type: ElementTypeFunction, }; break; @@ -360,6 +362,7 @@ export function attach( fiberData = { displayName: null, key, + index, type: ElementTypeEventComponent, }; break; @@ -376,6 +379,7 @@ export function attach( fiberData = { displayName, key, + index, type: ElementTypeEventTarget, }; break; @@ -388,6 +392,7 @@ export function attach( fiberData = { displayName, key, + index, type: ElementTypeForwardRef, }; break; @@ -395,6 +400,7 @@ export function attach( return { displayName: null, key: null, + index: 0, type: ElementTypeRoot, }; case HostPortal: @@ -404,6 +410,7 @@ export function attach( return { displayName: null, key, + index, type: ElementTypeOtherOrUnknown, }; case MemoComponent: @@ -417,6 +424,7 @@ export function attach( fiberData = { displayName, key, + index, type: ElementTypeMemo, }; break; @@ -430,6 +438,7 @@ export function attach( return { displayName: null, key: null, + index, type: ElementTypeOtherOrUnknown, }; case CONTEXT_PROVIDER_NUMBER: @@ -444,6 +453,7 @@ export function attach( fiberData = { displayName, key, + index, type: ElementTypeContext, }; break; @@ -462,6 +472,7 @@ export function attach( fiberData = { displayName, key, + index, type: ElementTypeContext, }; break; @@ -470,6 +481,7 @@ export function attach( fiberData = { displayName: null, key, + index, type: ElementTypeOtherOrUnknown, }; break; @@ -479,6 +491,7 @@ export function attach( fiberData = { displayName: 'Suspense', key, + index, type: ElementTypeSuspense, }; break; @@ -487,6 +500,7 @@ export function attach( fiberData = { displayName: `Profiler(${fiber.memoizedProps.id})`, key, + index, type: ElementTypeProfiler, }; break; @@ -496,6 +510,7 @@ export function attach( fiberData = { displayName: null, key, + index, type: ElementTypeOtherOrUnknown, }; break; @@ -2000,26 +2015,48 @@ export function attach( trackedPath = path; } - function getPathForElement(id: number): Array { - // TODO: this is not a real path. - return [ - { - index: id, - key: null, - displayName: null, - }, - ]; + function getPathFrame(fiber: Fiber): PathFrame { + let { displayName, key, index } = getDataForFiber(fiber); + if (fiber.tag === HostRoot) { + // Roots don't have a real index. + // Instead, we'll use the order in which it mounted. + const id = getFiberID(getPrimaryFiber(fiber)); + const order = rootInsertionOrder.get(id); + if (typeof order !== 'number') { + throw new Error('Expected mounted root to have known insertion order.'); + } + index = order; + } + return { + displayName, + key, + index, + }; + } + + // Produces a serializable representation that does a best effort + // of identifying a particular Fiber between page reloads. + // The return path will contain Fibers that are "invisible" to the store + // because their keys and indexes are important to restoring the selection. + function getPathForElement(id: number): Array | null { + let fiber = idToFiberMap.get(id); + if (fiber == null) { + return null; + } + const keyPath = []; + while (fiber !== null) { + keyPath.push(getPathFrame(fiber)); + fiber = fiber.return; + } + keyPath.reverse(); + return keyPath; } function getBestMatchForTrackedPath(): PathMatch | null { - // TODO: this is not a real lookup. if (trackedPath !== null) { - const id = trackedPath[0].index; - const fiber = idToFiberMap.get(id); - if (fiber !== null) { - return { id, isFullMatch: true }; - } + console.log('Looking for match for path: ', trackedPath); } + // TODO: implement this. return null; } diff --git a/src/backend/types.js b/src/backend/types.js index d77a6c1145..b9f0998e15 100644 --- a/src/backend/types.js +++ b/src/backend/types.js @@ -14,7 +14,8 @@ export type Fiber = Object; // (e.g. props, state, context, hooks) then we could add a bitmask field for this // to keep the number of attributes small. export type FiberData = {| - key: React$Key | null, + key: string | null, + index: number, displayName: string | null, type: ElementType, |}; @@ -115,7 +116,7 @@ export type RendererInterface = { getInteractions: (rootID: number) => Interactions, getProfilingDataForDownload: (rootID: number) => Object, getProfilingSummary: (rootID: number) => ProfilingSummary, - getPathForElement: (id: number) => Array, + getPathForElement: (id: number) => Array | null, handleCommitFiberRoot: (fiber: Object) => void, handleCommitFiberUnmount: (fiber: Object) => void, inspectElement: (id: number) => InspectedElement | null, From 7eb7c5a350f10a29c3629a72d08e2183ea2d9920 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 23 Apr 2019 16:56:24 +0100 Subject: [PATCH 4/8] Match Fibers to saved selection as they mount This implements matching Fibers against the tracked selection path. The algorithm is optimized to do as little checks as possible: * When not trying to restore selection, we don't do anything * When restoring selection, we only check .return pointers of new mounts * Only when .return pointers match our current deepest match, we compare the frames --- src/backend/renderer.js | 132 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 126 insertions(+), 6 deletions(-) diff --git a/src/backend/renderer.js b/src/backend/renderer.js index a3cc6272d4..9d751756ae 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -710,7 +710,6 @@ export function attach( const hasOwnerMetadata = fiber.hasOwnProperty('_debugOwner'); if (isRoot) { - rootInsertionOrder.set(id, nextRootIndex++); pushOperation(TREE_OPERATION_ADD); pushOperation(id); pushOperation(ElementTypeRoot); @@ -784,6 +783,18 @@ export function attach( } function recordUnmount(fiber: Fiber, isSimulated: boolean) { + if (trackedPathMatchFiber !== null) { + // We're in the process of trying to restore previous selection. + // If this fiber matched but is being unmounted, there's no use trying. + // Reset the state so we don't keep holding onto it. + if ( + fiber === trackedPathMatchFiber || + fiber === trackedPathMatchFiber.alternate + ) { + setTrackedPath(null); + } + } + const isRoot = fiber.tag === HostRoot; const primaryFiber = getPrimaryFiber(fiber); if (!fiberToIDMap.has(primaryFiber)) { @@ -798,7 +809,6 @@ export function attach( } const id = getFiberID(primaryFiber); if (isRoot) { - rootInsertionOrder.delete(id); // Removing a root needs to happen at the end // so we don't batch it with other unmounts. pushOperation(TREE_OPERATION_REMOVE); @@ -834,6 +844,12 @@ export function attach( debug('mountFiberRecursively()', fiber, parentFiber); } + // If we have the tree selection from previous reload, try to match this Fiber. + // Also remember whether to do the same for siblings. + const mightSiblingsBeOnTrackedPath = updateTrackedPathStateBeforeMount( + fiber + ); + const shouldIncludeInTree = !shouldFilterFiber(fiber); if (shouldIncludeInTree) { recordMount(fiber, parentFiber); @@ -867,6 +883,10 @@ export function attach( } } + // We're exiting this Fiber now, and entering its siblings. + // If we have selection to restore, we might need to re-activate tracking. + updateTrackedPathStateAfterMount(mightSiblingsBeOnTrackedPath); + if (traverseSiblings && fiber.sibling !== null) { mountFiberRecursively(fiber.sibling, parentFiber, true); } @@ -1141,9 +1161,15 @@ export function attach( }); }); } else { + // Before the traversals, remember to start tracking + // our path in case we have selection to restore. + if (trackedPath !== null) { + mightBeOnTrackedPath = true; + } // If we have not been profiling, then we can just walk the tree and build up its current state as-is. hook.getFiberRoots(rendererID).forEach(root => { currentRootID = getFiberID(getPrimaryFiber(root.current)); + rootInsertionOrder.set(currentRootID, nextRootIndex++); if (isProfiling) { // If profiling is active, store commit time and duration, and the current interactions. @@ -1181,6 +1207,12 @@ export function attach( currentRootID = getFiberID(getPrimaryFiber(current)); + // Before the traversals, remember to start tracking + // our path in case we have selection to restore. + if (trackedPath !== null) { + mightBeOnTrackedPath = true; + } + if (isProfiling) { // If profiling is active, store commit time and duration, and the current interactions. // The frontend may request this information after profiling has stopped. @@ -1206,16 +1238,19 @@ export function attach( current.memoizedState != null && current.memoizedState.element != null; if (!wasMounted && isMounted) { // Mount a new root. + rootInsertionOrder.set(currentRootID, nextRootIndex++); mountFiberRecursively(current, null); } else if (wasMounted && isMounted) { // Update an existing root. updateFiberRecursively(current, alternate, null); } else if (wasMounted && !isMounted) { // Unmount an existing root. + rootInsertionOrder.delete(currentRootID); recordUnmount(current, false); } } else { // Mount a new root. + rootInsertionOrder.set(currentRootID, nextRootIndex++); mountFiberRecursively(current, null); } @@ -2009,12 +2044,82 @@ export function attach( scheduleUpdate(fiber); } + // Remember if we're trying to restore the selection after reload. + // In that case, we'll do some extra checks for matching mounts. let trackedPath: Array | null = null; + let trackedPathMatchFiber: Fiber | null = null; + let trackedPathMatchDepth = -1; + let mightBeOnTrackedPath = false; function setTrackedPath(path: Array | null) { + if (path === null) { + trackedPathMatchFiber = null; + trackedPathMatchDepth = -1; + mightBeOnTrackedPath = false; + } else if (trackedPath !== null) { + throw new Error('Tracked path can only be set once.'); + } trackedPath = path; } + // We call this before traversing a new mount. + // It remembers whether this Fiber is the next best match for tracked path. + // The return value signals whether we should keep matching siblings or not. + function updateTrackedPathStateBeforeMount(fiber: Fiber): boolean { + if (trackedPath === null || !mightBeOnTrackedPath) { + // Fast path: there's nothing to track so do nothing and ignore siblings. + return false; + } + const returnFiber = fiber.return; + const returnAlternate = returnFiber !== null ? returnFiber.alternate : null; + // By now we know there's some selection to restore, and this is a new Fiber. + // Is this newly mounted Fiber a direct child of the current best match? + // (This will also be true for new roots if we haven't matched anything yet.) + if ( + trackedPathMatchFiber === returnFiber || + (trackedPathMatchFiber === returnAlternate && returnAlternate !== null) + ) { + // Is this the next Fiber we should select? Let's compare the frames. + const actualFrame = getPathFrame(fiber); + const expectedFrame = trackedPath[trackedPathMatchDepth + 1]; + if (expectedFrame === undefined) { + throw new Error('Expected to see a frame at the next depth.'); + } + if ( + actualFrame.index === expectedFrame.index && + actualFrame.key === expectedFrame.key && + actualFrame.displayName === expectedFrame.displayName + ) { + // We have our next match. + trackedPathMatchFiber = fiber; + trackedPathMatchDepth++; + // Are we out of frames to match? + if (trackedPathMatchDepth === trackedPath.length - 1) { + // There's nothing that can possibly match afterwards. + // Don't check the children. + mightBeOnTrackedPath = false; + } else { + // Check the children, as they might reveal the next match. + mightBeOnTrackedPath = true; + } + // In either case, since we have a match, we don't need + // to check the siblings. They'll never match. + return false; + } + } + // This Fiber's parent is on the path, but this Fiber itself isn't. + // There's no need to check its children--they won't be on the path either. + mightBeOnTrackedPath = false; + // However, one of its siblings may be on the path so keep searching. + return true; + } + + function updateTrackedPathStateAfterMount(mightSiblingsBeOnTrackedPath) { + // updateTrackedPathStateBeforeMount() told us whether to match siblings. + // Now that we're entering siblings, let's use that information. + mightBeOnTrackedPath = mightSiblingsBeOnTrackedPath; + } + function getPathFrame(fiber: Fiber): PathFrame { let { displayName, key, index } = getDataForFiber(fiber); if (fiber.tag === HostRoot) { @@ -2053,11 +2158,26 @@ export function attach( } function getBestMatchForTrackedPath(): PathMatch | null { - if (trackedPath !== null) { - console.log('Looking for match for path: ', trackedPath); + if (trackedPath === null) { + // Nothing to match. + return null; } - // TODO: implement this. - return null; + if (trackedPathMatchFiber === null) { + // We didn't find anything. + return null; + } + // Find the closest Fiber store is aware of. + let fiber = trackedPathMatchFiber; + while (fiber !== null && shouldFilterFiber(fiber)) { + fiber = fiber.return; + } + if (fiber === null) { + return null; + } + return { + id: getFiberID(getPrimaryFiber(fiber)), + isFullMatch: trackedPathMatchDepth === trackedPath.length - 1, + }; } return { From 26162988aa7f923b95b413ce9c6ec1503113162a Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 23 Apr 2019 17:10:19 +0100 Subject: [PATCH 5/8] Fix tests --- src/backend/agent.js | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/backend/agent.js b/src/backend/agent.js index 9f83fff38c..39a1f38381 100644 --- a/src/backend/agent.js +++ b/src/backend/agent.js @@ -81,11 +81,13 @@ export default class Agent extends EventEmitter { localStorage.removeItem(LOCAL_STORAGE_RELOAD_AND_PROFILE_KEY); } - const persistedSelectionString = sessionStorage.getItem( - SESSION_STORAGE_LAST_SELECTION_KEY - ); - if (persistedSelectionString != null) { - this._persistedSelection = JSON.parse(persistedSelectionString); + if (typeof sessionStorage !== 'undefined') { + const persistedSelectionString = sessionStorage.getItem( + SESSION_STORAGE_LAST_SELECTION_KEY + ); + if (persistedSelectionString != null) { + this._persistedSelection = JSON.parse(persistedSelectionString); + } } } @@ -617,9 +619,11 @@ export default class Agent extends EventEmitter { if (path === null) { return; } - sessionStorage.setItem( - SESSION_STORAGE_LAST_SELECTION_KEY, - JSON.stringify(({ rendererID, path }: PersistedSelection)) - ); + if (typeof sessionStorage !== 'undefined') { + sessionStorage.setItem( + SESSION_STORAGE_LAST_SELECTION_KEY, + JSON.stringify(({ rendererID, path }: PersistedSelection)) + ); + } }, 1000); } From e050529c7d54af391330c991817c614ee6ef66dd Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 24 Apr 2019 15:00:46 +0100 Subject: [PATCH 6/8] Read renderer ID from operations --- src/backend/agent.js | 8 ++------ src/backend/renderer.js | 10 ++-------- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/backend/agent.js b/src/backend/agent.js index 39a1f38381..dee69506cb 100644 --- a/src/backend/agent.js +++ b/src/backend/agent.js @@ -29,11 +29,6 @@ const debug = (methodName, ...args) => { } }; -type OperationsParams = {| - operations: Uint32Array, - rendererID: number, -|}; - type InspectSelectParams = {| id: number, rendererID: number, @@ -505,7 +500,7 @@ export default class Agent extends EventEmitter { } }; - onHookOperations = ({ operations, rendererID }: OperationsParams) => { + onHookOperations = (operations: Uint32Array) => { if (__DEBUG__) { debug('onHookOperations', operations); } @@ -533,6 +528,7 @@ export default class Agent extends EventEmitter { this._bridge.send('operations', operations); if (this._persistedSelection !== null) { + const rendererID = operations[0]; if (this._persistedSelection.rendererID === rendererID) { // Check if we can select a deeper match for the persisted selection. const renderer = this._rendererInterfaces[rendererID]; diff --git a/src/backend/renderer.js b/src/backend/renderer.js index 9d751756ae..f83dfcc6f9 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -686,10 +686,7 @@ export function attach( pendingOperationsQueue.push(ops); } else { // If we've already connected to the frontend, just pass the operations through. - hook.emit('operations', { - operations: ops, - rendererID, - }); + hook.emit('operations', ops); } pendingOperations.length = 0; @@ -1155,10 +1152,7 @@ export function attach( // We may have already queued up some operations before the frontend connected // If so, let the frontend know about them. localPendingOperationsQueue.forEach(ops => { - hook.emit('operations', { - operations: ops, - rendererID, - }); + hook.emit('operations', ops); }); } else { // Before the traversals, remember to start tracking From 5d2d4213ec3d100b3245f8a3788bbc62c7361cef Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 24 Apr 2019 15:02:52 +0100 Subject: [PATCH 7/8] Delete stale paths --- src/backend/agent.js | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/backend/agent.js b/src/backend/agent.js index dee69506cb..976b838c88 100644 --- a/src/backend/agent.js +++ b/src/backend/agent.js @@ -608,18 +608,16 @@ export default class Agent extends EventEmitter { // might not be available by the time we read them. // This is why we need the defensive checks here. const renderer = this._rendererInterfaces[rendererID]; - if (renderer == null) { - return; - } - const path = renderer.getPathForElement(id); - if (path === null) { - return; - } + const path = renderer != null ? renderer.getPathForElement(id) : null; if (typeof sessionStorage !== 'undefined') { - sessionStorage.setItem( - SESSION_STORAGE_LAST_SELECTION_KEY, - JSON.stringify(({ rendererID, path }: PersistedSelection)) - ); + if (path !== null) { + sessionStorage.setItem( + SESSION_STORAGE_LAST_SELECTION_KEY, + JSON.stringify(({ rendererID, path }: PersistedSelection)) + ); + } else { + sessionStorage.removeItem(SESSION_STORAGE_LAST_SELECTION_KEY); + } } }, 1000); } From f123763fe1b3d8c9bf188249afcc32baff3cc3fa Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 24 Apr 2019 15:05:33 +0100 Subject: [PATCH 8/8] Read index off the Fiber --- src/backend/renderer.js | 20 +++----------------- src/backend/types.js | 1 - 2 files changed, 3 insertions(+), 18 deletions(-) diff --git a/src/backend/renderer.js b/src/backend/renderer.js index f83dfcc6f9..cc3cae5c60 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -324,7 +324,7 @@ export function attach( // TODO: we might want to change the data structure once we no longer suppport Stack versions of `getData`. // TODO: Keep in sync with getElementType() function getDataForFiber(fiber: Fiber): FiberData { - const { elementType, type, key, index, tag } = fiber; + const { elementType, type, key, tag } = fiber; // This is to support lazy components with a Promise as the type. // see https://github.com/facebook/react/pull/13397 @@ -345,7 +345,6 @@ export function attach( fiberData = { displayName: getDisplayName(resolvedType), key, - index, type: ElementTypeClass, }; break; @@ -354,7 +353,6 @@ export function attach( fiberData = { displayName: getDisplayName(resolvedType), key, - index, type: ElementTypeFunction, }; break; @@ -362,7 +360,6 @@ export function attach( fiberData = { displayName: null, key, - index, type: ElementTypeEventComponent, }; break; @@ -379,7 +376,6 @@ export function attach( fiberData = { displayName, key, - index, type: ElementTypeEventTarget, }; break; @@ -392,7 +388,6 @@ export function attach( fiberData = { displayName, key, - index, type: ElementTypeForwardRef, }; break; @@ -400,7 +395,6 @@ export function attach( return { displayName: null, key: null, - index: 0, type: ElementTypeRoot, }; case HostPortal: @@ -410,7 +404,6 @@ export function attach( return { displayName: null, key, - index, type: ElementTypeOtherOrUnknown, }; case MemoComponent: @@ -424,7 +417,6 @@ export function attach( fiberData = { displayName, key, - index, type: ElementTypeMemo, }; break; @@ -438,7 +430,6 @@ export function attach( return { displayName: null, key: null, - index, type: ElementTypeOtherOrUnknown, }; case CONTEXT_PROVIDER_NUMBER: @@ -453,7 +444,6 @@ export function attach( fiberData = { displayName, key, - index, type: ElementTypeContext, }; break; @@ -472,7 +462,6 @@ export function attach( fiberData = { displayName, key, - index, type: ElementTypeContext, }; break; @@ -481,7 +470,6 @@ export function attach( fiberData = { displayName: null, key, - index, type: ElementTypeOtherOrUnknown, }; break; @@ -491,7 +479,6 @@ export function attach( fiberData = { displayName: 'Suspense', key, - index, type: ElementTypeSuspense, }; break; @@ -500,7 +487,6 @@ export function attach( fiberData = { displayName: `Profiler(${fiber.memoizedProps.id})`, key, - index, type: ElementTypeProfiler, }; break; @@ -510,7 +496,6 @@ export function attach( fiberData = { displayName: null, key, - index, type: ElementTypeOtherOrUnknown, }; break; @@ -2115,7 +2100,8 @@ export function attach( } function getPathFrame(fiber: Fiber): PathFrame { - let { displayName, key, index } = getDataForFiber(fiber); + const { displayName, key } = getDataForFiber(fiber); + let index = fiber.index; if (fiber.tag === HostRoot) { // Roots don't have a real index. // Instead, we'll use the order in which it mounted. diff --git a/src/backend/types.js b/src/backend/types.js index b9f0998e15..0bd1648233 100644 --- a/src/backend/types.js +++ b/src/backend/types.js @@ -15,7 +15,6 @@ export type Fiber = Object; // to keep the number of attributes small. export type FiberData = {| key: string | null, - index: number, displayName: string | null, type: ElementType, |};