diff --git a/.changeset/bright-rabbits-nail.md b/.changeset/bright-rabbits-nail.md new file mode 100644 index 00000000..44604dc4 --- /dev/null +++ b/.changeset/bright-rabbits-nail.md @@ -0,0 +1,5 @@ +--- +"@solid-devtools/debugger": minor +--- + +Require `ElementInterface.getChildren` to return `ArrayLike`. (76ab4096a2ad531ae015e35c2475b78f08ac45a7) diff --git a/.changeset/orange-islands-drop.md b/.changeset/orange-islands-drop.md new file mode 100644 index 00000000..1af11ae1 --- /dev/null +++ b/.changeset/orange-islands-drop.md @@ -0,0 +1,5 @@ +--- +"@solid-devtools/debugger": patch +--- + +Improve tree walking algorithm for mapping elements. (#348) diff --git a/examples/sandbox/src/App.tsx b/examples/sandbox/src/App.tsx index 9ee40c77..c39a9631 100644 --- a/examples/sandbox/src/App.tsx +++ b/examples/sandbox/src/App.tsx @@ -163,69 +163,67 @@ const App: s.Component = () => { const [showBroken, setShowBroken] = s.createSignal(false) - return ( - <> - {header()} - {objmemo().subheader} -
-
-
- - - {s.createComponent(() => <> - - , {})} - - - {/* -
*/} - - <> - {err.toString()} - - } - > - - - - -
-
-
- - - -
- - {s.untrack(() => { - const MARGIN = '24px' - return <> -
- -
-
- -
- - })} - - - ) + return <> + {header()} + {objmemo().subheader} +
+
+
+ + Count is very odd}> + {s.createComponent(() => <> + + , {})} + + + {/* +
*/} + + <> + {err.toString()} + + } + > + + + + +
+
+
+ + + +
+ + {s.untrack(() => { + const MARGIN = '24px' + return <> +
+ +
+
+ +
+ + })} + + } const CountingComponent = () => { diff --git a/packages/debugger/src/main/id.ts b/packages/debugger/src/main/id.ts index b0d4ca1f..77437f7b 100644 --- a/packages/debugger/src/main/id.ts +++ b/packages/debugger/src/main/id.ts @@ -49,6 +49,13 @@ export function getSdtId(obj: ValueMap[T], objType: T): No return id } +export const get_id_owner = (o: Solid.Owner): NodeID => getSdtId(o, ObjectType.Owner) +export const get_id_el = (o: object): NodeID => getSdtId(o, ObjectType.Element) +export const get_id_signal = (o: Solid.Signal): NodeID => getSdtId(o, ObjectType.Signal) +export const get_id_store = (o: Solid.Store): NodeID => getSdtId(o, ObjectType.Store) +export const get_id_store_node = (o: Solid.StoreNode): NodeID => getSdtId(o, ObjectType.StoreNode) +export const get_id_custom_value = (o: Solid.SourceMapValue): NodeID => getSdtId(o, ObjectType.CustomValue) + export function getObjectById(id: NodeID, objType: T): ValueMap[T] | null { const ref = RefMapMap[objType].get(id) return ref?.deref() ?? null diff --git a/packages/debugger/src/main/types.ts b/packages/debugger/src/main/types.ts index e1dab1d9..d71b5217 100644 --- a/packages/debugger/src/main/types.ts +++ b/packages/debugger/src/main/types.ts @@ -143,6 +143,8 @@ export type Rect = { height: number } +export type ElementChildren = Iterable & ArrayLike + /** * When using a custom solid renderer, you should provide a custom element interface. * By default the debugger assumes that rendered elements are DOM elements. @@ -151,7 +153,7 @@ export type ElementInterface = { isElement: (obj: object | T) => obj is T, getElementAt: (e: MouseEvent) => T | null, getName: (el: T) => string | null, - getChildren: (el: T) => Iterable, + getChildren: (el: T) => ElementChildren, getParent: (el: T) => T | null, getLocation: (el: T) => SourceLocation | null, getRect: (el: T) => Rect | null, @@ -196,17 +198,14 @@ export const getValueItemId = ( export type ValueUpdateListener = (newValue: unknown, oldValue: unknown) => void export namespace Solid { - export type OwnerBase = import('solid-js').Owner - export type SourceMapValue = import('solid-js/types/reactive/signal.d.ts').SourceMapValue - export type Signal = import('solid-js/types/reactive/signal.d.ts').SignalState - export type Computation = import('solid-js/types/reactive/signal.d.ts').Computation - export type Memo = import('solid-js/types/reactive/signal.d.ts').Memo + export type OwnerBase = import('solid-js').Owner + export type SourceMapValue = import('solid-js/types/reactive/signal.d.ts').SourceMapValue + export type Signal = import('solid-js/types/reactive/signal.d.ts').SignalState + export type Computation = import('solid-js/types/reactive/signal.d.ts').Computation + export type Memo = import('solid-js/types/reactive/signal.d.ts').Memo export type RootFunction = import('solid-js/types/reactive/signal.d.ts').RootFunction - export type EffectFunction = - import('solid-js/types/reactive/signal.d.ts').EffectFunction - export type Component = import('solid-js/types/reactive/signal.d.ts').DevComponent<{ - [key: string]: unknown - }> + export type EffectFunction = import('solid-js/types/reactive/signal.d.ts').EffectFunction + export type Component = import('solid-js/types/reactive/signal.d.ts').DevComponent<{[key: string]: unknown}> export type CatchError = Omit & {fn: undefined} @@ -230,10 +229,10 @@ export namespace Solid { // STORE // - export type StoreNode = import('solid-js/store').StoreNode - export type NotWrappable = import('solid-js/store').NotWrappable + export type StoreNode = import('solid-js/store').StoreNode + export type NotWrappable = import('solid-js/store').NotWrappable export type OnStoreNodeUpdate = import('solid-js/store/types/store.d.ts').OnStoreNodeUpdate - export type Store = SourceMapValue & {value: StoreNode} + export type Store = SourceMapValue & {value: StoreNode} } declare module 'solid-js/types/reactive/signal.d.ts' { diff --git a/packages/debugger/src/structure/index.ts b/packages/debugger/src/structure/index.ts index 7b5b4767..0a3be2d7 100644 --- a/packages/debugger/src/structure/index.ts +++ b/packages/debugger/src/structure/index.ts @@ -58,13 +58,14 @@ export function createStructure(props: { let shouldUpdateAllRoots = true const onComputationUpdate: walker.ComputationUpdateHandler = ( - rootId, owner, changedStructure, + root_id, owner, changed_structure, ) => { // separate the callback from the computation queueMicrotask(() => { if (!props.enabled()) return - if (changedStructure) { - updateOwner(owner, rootId) + if (changed_structure) { + let owner_to_update = getClosestIncludedOwner(owner, treeWalkerMode) ?? owner + updateOwner(owner_to_update, root_id) } let id = getSdtId(owner, ObjectType.Owner) props.onNodeUpdate(id) diff --git a/packages/debugger/src/structure/walker.test.tsx b/packages/debugger/src/structure/walker.test.tsx index 98155d50..3c898b7c 100644 --- a/packages/debugger/src/structure/walker.test.tsx +++ b/packages/debugger/src/structure/walker.test.tsx @@ -362,10 +362,10 @@ test.describe('TreeWalkerMode.DOM', () => { let to_trigger: (() => void)[] = [] let test_components: Solid.Component[] = [] - + let el_header!: HTMLElement let el_h1!: HTMLHeadingElement - + let el_footer!: HTMLElement let el_main!: HTMLElement let el_h2!: HTMLHeadingElement @@ -392,7 +392,7 @@ test.describe('TreeWalkerMode.DOM', () => { Click me } - + const App = () => { return ( <> diff --git a/packages/debugger/src/structure/walker.ts b/packages/debugger/src/structure/walker.ts index a241adfb..3588d293 100644 --- a/packages/debugger/src/structure/walker.ts +++ b/packages/debugger/src/structure/walker.ts @@ -1,7 +1,8 @@ import {untrackedCallback} from '@solid-devtools/shared/primitives' -import {ObjectType, getSdtId} from '../main/id.ts' +import {ObjectType, getSdtId, get_id_el} from '../main/id.ts' import {observeComputationUpdate} from '../main/observe.ts' import { + type ElementChildren, type ElementInterface, type Mapped, type NodeID, @@ -112,37 +113,46 @@ const registerComponent = ( export const registerElement = ( - r: ComponentRegistry, - componentId: NodeID, - elementId: NodeID, - element: TEl, + r: ComponentRegistry, + component_id: NodeID | null, + element_id: NodeID, + element: TEl, ): void => { - let component = r.components.get(componentId) - if (!component) return + // TODO: also allow registering elements without a component + if (component_id == null) { + return + } - component.element_nodes.add(elementId) - r.element_nodes.set(elementId, {el: element as any as TEl, component}) + let component = r.components.get(component_id) + if (component == null) return + + component.element_nodes.add(element_id) + r.element_nodes.set(element_id, {el: element, component}) } export const getComponent = ( - r: ComponentRegistry, + r: ComponentRegistry, id: NodeID, ): {name: string | undefined; id: NodeID; elements: TEl[]} | null => { // provided if might be of an element node (in DOM mode) or component node // both need to be checked let component = r.components.get(id) - if (component) return { - name: component.name, - elements: [...component.elements].map(el => el as any as TEl), - id + if (component != null) return { + name: component.name, + elements: [...component.elements], + id: id, } - let elData = r.element_nodes.get(id) - return elData - ? {name: elData.component.name, id: elData.component.id, elements: [elData.el]} - : null + let el_data = r.element_nodes.get(id) + if (el_data == null) return null + + return { + name: el_data.component.name, + id: el_data.component.id, + elements: [el_data.el], + } } /** @@ -155,11 +165,13 @@ const getComponent = ( export const getComponentElement = ( r: ComponentRegistry, - elementId: NodeID, + element_id: NodeID, ): {name: string | undefined; id: NodeID; element: TEl} | undefined => { - let el_data = r.element_nodes.get(elementId) - if (el_data != null) { - return {name: el_data.component.name, id: el_data.component.id, element: el_data.el} + let el_data = r.element_nodes.get(element_id) + if (el_data != null) return { + name: el_data.component.name, + id: el_data.component.id, + element: el_data.el, } } @@ -200,13 +212,15 @@ const findComponent = ( } -export type ComputationUpdateHandler = ( - rootId: NodeID, - owner: Solid.Owner, - changedStructure: boolean, +export +type ComputationUpdateHandler = ( + rootId: NodeID, + owner: Solid.Owner, + changed_structure: boolean, ) => void -export type TreeWalkerConfig = { +export +type TreeWalkerConfig = { mode: TreeWalkerMode rootId: NodeID onUpdate: ComputationUpdateHandler @@ -214,8 +228,6 @@ export type TreeWalkerConfig = { eli: ElementInterface } -const ElementsMap = new Map() - const $WALKER = Symbol('tree-walker') function observeComputation( @@ -232,7 +244,7 @@ function observeComputation( // copy values in case config gets mutated let {rootId, onUpdate: onComputationUpdate, mode} = config - const handler = () => { + let handler = () => { let is_leaf = !comp.owned || comp.owned.length === 0 let changed_structure = was_leaf !== is_leaf || !is_leaf || mode === TreeWalkerMode.DOM was_leaf = is_leaf @@ -273,97 +285,54 @@ function pushResolvedElements(list: TEl[], value: unknown, e } } -let MappedOwnerNode: Mapped.Owner -let AddedToParentElements = false - -/** - * @param els elements to map - * @param parent_children parent owner children. - * Will be checked for existing elements, and if found, `MappedOwnerNode` will be injected in the place of the element. - * Passing `undefined` will skip this check. - */ -function mapElements( - els: Iterable, - parent_children: Mapped.Owner[] | undefined, - eli: ElementInterface, - out: Mapped.Owner[] = [], -): Mapped.Owner[] { - - els: for (let el of els) { - if (!eli.isElement(el)) continue - - if (parent_children) { - - // find el in parent els and remove it - let to_check = [parent_children] - let index = 0 - let el_nodes = to_check[index++] - - while (el_nodes) { - for (let i = 0; i < el_nodes.length; i++) { - let el_node = el_nodes[i]! - let el_node_data = ElementsMap.get(el_node) - if (el_node_data && el_node_data.el === el) { - if (AddedToParentElements) { - // if the element is already added to the parent, just remove the element - el_nodes.splice(i, 1) - } else { - // otherwise, we can just replace it with the component - el_nodes[i] = MappedOwnerNode - AddedToParentElements = true - } - out.push(el_node) - el_node_data.component = MappedOwnerNode - continue els - } - if (el_node.children.length) to_check.push(el_node.children) - } - el_nodes = to_check[index++] - } - } - - let el_json: Mapped.Owner = { - id: getSdtId(el, ObjectType.Element), - type: NodeType.Element, - name: eli.getName(el) ?? UNKNOWN, - children: [], - } - out.push(el_json) - ElementsMap.set(el_json, {el, component: MappedOwnerNode}) - - mapElements(eli.getChildren(el), parent_children, eli, el_json.children) - } - - return out -} - function mapChildren( - owner: Solid.Owner, - owner_map: Mapped.Owner | null, - config: TreeWalkerConfig, - children: Mapped.Owner[] = [], + owner: Solid.Owner, + component_id: NodeID | null, + config: TreeWalkerConfig, + children: Mapped.Owner[] = [], ): Mapped.Owner[] { for (let child of owner_each_child(owner)) { if (config.mode === TreeWalkerMode.Owners || markOwnerType(child) === NodeType.Component ) { - unwrap_append(children, mapOwner(child, owner_map, config)) + unwrap_append(children, mapOwner(child, component_id, config)) } else { if (isSolidComputation(child)) { observeComputation(child, owner, config) } - mapChildren(child, owner_map, config, children) + mapChildren(child, component_id, config, children) } } return children } +let els_seen = new Set() + +function add_new_el_json( + child_arr: Mapped.Owner[], + el: TEl, + comp_id: NodeID | null, + config: TreeWalkerConfig, +): Mapped.Owner { + let el_json: Mapped.Owner = { + id: get_id_el(el), + type: NodeType.Element, + name: config.eli.getName(el) ?? UNKNOWN, + children: [], + } + child_arr.push(el_json) + els_seen.add(el) + registerElement(config.registry, comp_id, el_json.id, el) + return el_json +} + function mapOwner( - owner: Solid.Owner, - parent: Mapped.Owner | null, - config: TreeWalkerConfig, + owner: Solid.Owner, + /** last seen component id - for registering elements to components */ + last_comp_id: NodeID | null, + config: TreeWalkerConfig, ): Mapped.Owner | undefined { let id = getSdtId(owner, ObjectType.Owner) @@ -390,11 +359,11 @@ function mapOwner( The provider component will be omitted */ if (name === 'provider' && - owner.owned && + owner.owned != null && owner.owned.length === 1 && markOwnerType(first_owned = owner.owned[0]!) === NodeType.Context ) { - return mapOwner(first_owned, parent, config) + return mapOwner(first_owned, last_comp_id, config) } // Register component to global map @@ -404,10 +373,13 @@ function mapOwner( // Refresh // omitting refresh memo — map it's children instead let refresh = getComponentRefreshNode(owner as Solid.Component) - if (refresh) { + if (refresh != null) { mapped.hmr = true owner = refresh } + + /* This owner is now last seen component */ + last_comp_id = id } // Computation else if (isSolidComputation(owner)) { @@ -417,40 +389,151 @@ function mapOwner( } } - AddedToParentElements = false as boolean - MappedOwnerNode = mapped - - // Map html elements in DOM mode + mapChildren(owner, last_comp_id, config, mapped.children) + + /* + | Merge element tree and owner tree depth-first, using elements as positional hints + | + | Component A +
-> Component A + | └─ Component B ├─ └─
+ | └─

├─ Component B + | │ └─ + | └─

+ | 1. Walk owner children first (depth-first) + | - child elements are already part of children tree + | 2. For each element in resolved tree: + | - If element matches current child owner → attach preceding children to element + | - If element not seen → add as new element node + | - Skip already processed elements + | 3. Use dual stacks to track position in both trees simultaneously + */ if (config.mode === TreeWalkerMode.DOM) { + // elements might already be resolved when mapping components resolved_els ??= resolveElements(owner.value, config.eli) - mapElements(resolved_els, parent?.children, config.eli, mapped.children) - } - // global `AddedToParentElements` will be changed in mapChildren - let addedToParent = AddedToParentElements + let stack_els_arr: ElementChildren[] = [resolved_els] + let stack_els_idx = [0] + let stack_els_own = [mapped] + let stack_els_len = 1 - mapChildren(owner, mapped, config, mapped.children) + let stack_child_arr = [mapped.children] + let stack_child_idx = [0] + let stack_child_len = 1 - return addedToParent ? undefined : mapped -} + while (stack_child_len > 0) { + let child_arr = stack_child_arr[stack_child_len-1]! + let child_idx = stack_child_idx[stack_child_len-1]! + if (child_idx >= child_arr.length) { + stack_child_len -= 1 + continue + } -export const walkSolidTree = /*#__PURE__*/ untrackedCallback(function ( - owner: Solid.Owner | Solid.Root, - config: TreeWalkerConfig, -): Mapped.Owner { + stack_child_idx[stack_child_len-1]! += 1 - const r = mapOwner(owner, null, config)! + let child = child_arr[child_idx]! - if (config.mode === TreeWalkerMode.DOM) { - // Register all mapped element nodes to their components - for (let [elNode, {el, component}] of ElementsMap) { - registerElement(config.registry, component.id, elNode.id, el as TEl) + /* Other children are already in mapped children so will be skipped */ + if (child.type !== NodeType.Element) { + stack_child_arr[stack_child_len] = child.children + stack_child_idx[stack_child_len] = 0 + stack_child_len += 1 + continue + } + + // Don't go over added element children + if (stack_child_len === 1) + continue + + /* Check each element and its children + - not seen -> add element to the owner + - a child of the current child -> add child to the owner + - already seen -> skip it + */ + while (stack_els_len > 0) { + let el_arr = stack_els_arr[stack_els_len-1]! + let el_idx = stack_els_idx[stack_els_len-1]! + let el_own = stack_els_own[stack_els_len-1]! + + if (el_idx >= el_arr.length) { + stack_els_len -= 1 + continue + } + + stack_els_idx[stack_els_len-1]! += 1 + + let el = el_arr[el_idx]! + + /* Child has this element */ + if (get_id_el(el) === child.id) { + /* + Push child to the owner + and previous ones that were not pushed yet + */ + el_own.children.push(...mapped.children.splice(0, stack_child_idx[0])) + stack_child_idx[0] = 0 + stack_child_len = 1 + + // Skip remaining elements from the child + for (let ci = child_idx + 1; ci < child_arr.length; ci++) { + let ei = stack_els_idx[stack_els_len-1]! + + if (ei >= el_arr.length || child_arr[ci]!.id !== get_id_el(el_arr[ei]!)) + break + + stack_els_idx[stack_els_len-1]! += 1 + } + + break + } + + /* Not seen yet */ + if (!els_seen.has(el)) { + stack_els_arr[stack_els_len] = config.eli.getChildren(el) + stack_els_idx[stack_els_len] = 0 + stack_els_own[stack_els_len] = add_new_el_json(el_own.children, el, last_comp_id, config) + stack_els_len += 1 + } + } } - ElementsMap.clear() + // append remaining elements to children + while (stack_els_len > 0) { + let el_arr = stack_els_arr[stack_els_len-1]! + let el_idx = stack_els_idx[stack_els_len-1]! + let el_own = stack_els_own[stack_els_len-1]! + + if (el_idx >= el_arr.length) { + stack_els_len -= 1 + continue + } + + stack_els_idx[stack_els_len-1]! += 1 + + let el = el_arr[el_idx]! + + if (!els_seen.has(el)) { + stack_els_arr[stack_els_len] = config.eli.getChildren(el) + stack_els_idx[stack_els_len] = 0 + stack_els_own[stack_els_len] = add_new_el_json(el_own.children, el, last_comp_id, config) + stack_els_len += 1 + } + } } + return mapped +} + +export +const walkSolidTree = /*#__PURE__*/ untrackedCallback(function ( + owner: Solid.Owner | Solid.Root, + config: TreeWalkerConfig, +): Mapped.Owner { + + let r = mapOwner(owner, null, config)! + + els_seen.clear() + return r })