From 2b20072e802eb8fec230529989b697f86dd3ce31 Mon Sep 17 00:00:00 2001 From: Patrick Pircher Date: Sun, 4 Feb 2024 11:10:47 +0100 Subject: [PATCH 1/3] add in element to debug render tree My learnings from hacking on this in inspector https://github.com/emberjs/ember-inspector/pull/2549/files Adding modifiers is also easy, will add it in another pr --- .../test/debug-render-tree-test.ts | 54 +++++++++++++++++-- .../lib/runtime/debug-render-tree.d.ts | 2 +- .../@glimmer/node/lib/serialize-builder.ts | 2 +- .../runtime/lib/vm/element-builder.ts | 42 ++++++++++++--- .../runtime/lib/vm/rehydrate-builder.ts | 2 +- 5 files changed, 88 insertions(+), 14 deletions(-) diff --git a/packages/@glimmer-workspace/integration-tests/test/debug-render-tree-test.ts b/packages/@glimmer-workspace/integration-tests/test/debug-render-tree-test.ts index 0a1b3c8885..52ee3cce86 100644 --- a/packages/@glimmer-workspace/integration-tests/test/debug-render-tree-test.ts +++ b/packages/@glimmer-workspace/integration-tests/test/debug-render-tree-test.ts @@ -283,6 +283,54 @@ class DebugRenderTreeTest extends RenderTest { ]); } + @test 'in-element in tree'() { + this.registerComponent('Glimmer', 'HiWorld', 'Hi World'); + this.registerComponent( + 'Glimmer', + 'HelloWorld', + '{{#in-element this.destinationElement}}{{/in-element}}', + class extends GlimmerishComponent { + get destinationElement() { + return document.getElementById('target'); + } + } + ); + + this.render(`
`); + + this.assertRenderTree([ + { + type: 'component', + name: 'HelloWorld', + args: { positional: [], named: { arg: 'first' } }, + instance: (instance: GlimmerishComponent) => instance.args['arg'] === 'first', + template: '(unknown template module)', + bounds: this.nodeBounds(this.element.firstChild!.nextSibling), + children: [ + { + type: 'keyword', + name: 'in-element', + args: { positional: [this.element.firstChild], named: {} }, + instance: (instance: GlimmerishComponent) => instance === null, + template: null, + bounds: this.nodeBounds(this.element.firstChild!.firstChild, this.element), + children: [ + { + type: 'component', + name: 'HiWorld', + args: { positional: [], named: {} }, + instance: (instance: GlimmerishComponent) => instance, + template: '(unknown template module)', + bounds: this.nodeBounds(this.element.firstChild!.firstChild), + children: [], + }, + ], + }, + ], + }, + ]); + } + @test 'getDebugCustomRenderTree works'() { let bucket1 = {}; let instance1 = {}; @@ -471,12 +519,12 @@ class DebugRenderTreeTest extends RenderTest { assert.deepEqual(this.delegate.getCapturedRenderTree(), [], 'there was no output'); } - nodeBounds(_node: SimpleNode | null): CapturedBounds { + nodeBounds(_node: SimpleNode | null, parent?: SimpleNode): CapturedBounds { let node = expect(_node, 'BUG: Expected node'); return { parentElement: expect( - node.parentNode, + parent || node.parentNode, 'BUG: detached node' ) as unknown as SimpleNode as SimpleElement, firstNode: node as unknown as SimpleNode, @@ -532,7 +580,7 @@ class DebugRenderTreeTest extends RenderTest { this.assertRenderNode(actualNode, expected, `${actualNode.type}:${actualNode.name}`); }); } else { - this.assert.deepEqual(actual, [], path); + this.assert.deepEqual(actual, expectedNodes, path); } } diff --git a/packages/@glimmer/interfaces/lib/runtime/debug-render-tree.d.ts b/packages/@glimmer/interfaces/lib/runtime/debug-render-tree.d.ts index f9f8ae5943..a2edcedce5 100644 --- a/packages/@glimmer/interfaces/lib/runtime/debug-render-tree.d.ts +++ b/packages/@glimmer/interfaces/lib/runtime/debug-render-tree.d.ts @@ -3,7 +3,7 @@ import type { SimpleElement, SimpleNode } from '@simple-dom/interface'; import type { Bounds } from '../dom/bounds.js'; import type { Arguments, CapturedArguments } from './arguments.js'; -export type RenderNodeType = 'outlet' | 'engine' | 'route-template' | 'component'; +export type RenderNodeType = 'outlet' | 'engine' | 'route-template' | 'component' | 'keyword'; export interface RenderNode { type: RenderNodeType; diff --git a/packages/@glimmer/node/lib/serialize-builder.ts b/packages/@glimmer/node/lib/serialize-builder.ts index e161956c36..baa8c1dacc 100644 --- a/packages/@glimmer/node/lib/serialize-builder.ts +++ b/packages/@glimmer/node/lib/serialize-builder.ts @@ -129,7 +129,7 @@ class SerializeBuilder extends NewElementBuilder implements ElementBuilder { element: SimpleElement, cursorId: string, insertBefore: Maybe = null - ): Nullable { + ): RemoteLiveBlock { let { dom } = this; let script = dom.createElement('script'); script.setAttribute('glmr', cursorId); diff --git a/packages/@glimmer/runtime/lib/vm/element-builder.ts b/packages/@glimmer/runtime/lib/vm/element-builder.ts index e4e6d30012..f0b0dd90a8 100644 --- a/packages/@glimmer/runtime/lib/vm/element-builder.ts +++ b/packages/@glimmer/runtime/lib/vm/element-builder.ts @@ -1,6 +1,7 @@ import type { AttrNamespace, Bounds, + CapturedArguments, Cursor, CursorStackSymbol, ElementBuilder, @@ -12,6 +13,7 @@ import type { Maybe, ModifierInstance, Nullable, + Reference, SimpleComment, SimpleDocumentFragment, SimpleElement, @@ -20,6 +22,7 @@ import type { UpdatableBlock, } from '@glimmer/interfaces'; import { destroy, registerDestructor } from '@glimmer/destroyable'; +import { createConstRef } from '@glimmer/reference'; import { assert, expect, Stack } from '@glimmer/util'; import type { DynamicAttribute } from './attributes/dynamic'; @@ -100,7 +103,6 @@ export class NewElementBuilder implements ElementBuilder { constructor(env: Environment, parentNode: SimpleElement, nextSibling: Nullable) { this.pushElement(parentNode, nextSibling); - this.env = env; this.dom = env.getAppendOperations(); this.updateOperations = env.getDOM(); @@ -214,15 +216,31 @@ export class NewElementBuilder implements ElementBuilder { element: SimpleElement, guid: string, insertBefore: Maybe - ): Nullable { - return this.__pushRemoteElement(element, guid, insertBefore); + ): RemoteLiveBlock { + const block = this.__pushRemoteElement(element, guid, insertBefore); + if (this.env.debugRenderTree) { + const namedArgs: Record = {}; + if (insertBefore !== undefined) { + namedArgs['insertBefore'] = createConstRef(insertBefore, false); + } + this.env.debugRenderTree.create(block, { + type: 'keyword', + name: 'in-element', + args: { + named: namedArgs, + positional: [createConstRef(element, false)], + } as CapturedArguments, + instance: null, + }); + } + return block; } __pushRemoteElement( element: SimpleElement, _guid: string, insertBefore: Maybe - ): Nullable { + ): RemoteLiveBlock { this.pushElement(element, insertBefore); if (insertBefore === undefined) { @@ -236,12 +254,20 @@ export class NewElementBuilder implements ElementBuilder { return this.pushLiveBlock(block, true); } - popRemoteElement() { - this.popBlock(); + popRemoteElement(): void { + const block = this.popBlock(); this.popElement(); + const parentElement = this.element; + if (this.env.debugRenderTree) { + this.env.debugRenderTree?.didRender(block, { + parentElement: () => parentElement, + firstNode: () => block.firstNode(), + lastNode: () => block.lastNode(), + }); + } } - protected pushElement(element: SimpleElement, nextSibling: Maybe = null) { + protected pushElement(element: SimpleElement, nextSibling: Maybe = null): void { this[CURSOR_STACK].push(new CursorImpl(element, nextSibling)); } @@ -268,7 +294,7 @@ export class NewElementBuilder implements ElementBuilder { return element; } - willCloseElement() { + willCloseElement(): void { this.block().closeElement(); } diff --git a/packages/@glimmer/runtime/lib/vm/rehydrate-builder.ts b/packages/@glimmer/runtime/lib/vm/rehydrate-builder.ts index dbb91bd300..66501924e4 100644 --- a/packages/@glimmer/runtime/lib/vm/rehydrate-builder.ts +++ b/packages/@glimmer/runtime/lib/vm/rehydrate-builder.ts @@ -455,7 +455,7 @@ export class RehydrateBuilder extends NewElementBuilder implements ElementBuilde element: SimpleElement, cursorId: string, insertBefore: Maybe - ): Nullable { + ): RemoteLiveBlock { const marker = this.getMarker(castToBrowser(element, 'HTML'), cursorId); assert( From 33d130ec28d4fa38765e1eaa122b7ab313f25d59 Mon Sep 17 00:00:00 2001 From: patrick Date: Tue, 27 Feb 2024 11:13:44 +0100 Subject: [PATCH 2/3] add registerDestructor --- packages/@glimmer/runtime/lib/vm/element-builder.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/@glimmer/runtime/lib/vm/element-builder.ts b/packages/@glimmer/runtime/lib/vm/element-builder.ts index f0b0dd90a8..29ce5318c8 100644 --- a/packages/@glimmer/runtime/lib/vm/element-builder.ts +++ b/packages/@glimmer/runtime/lib/vm/element-builder.ts @@ -232,6 +232,9 @@ export class NewElementBuilder implements ElementBuilder { } as CapturedArguments, instance: null, }); + registerDestructor(block, () => { + this.env.debugRenderTree?.willDestroy(block); + }); } return block; } From b472b006beadd8598cdb41bace9aabab1fea08dd Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Fri, 8 Mar 2024 14:13:24 -0800 Subject: [PATCH 3/3] Clean up implementation --- .../test/debug-render-tree-test.ts | 6 ++-- .../interfaces/lib/dom/attributes.d.ts | 4 +-- .../runtime/lib/compiled/opcodes/dom.ts | 31 ++++++++++++++-- .../runtime/lib/vm/element-builder.ts | 36 +++---------------- 4 files changed, 38 insertions(+), 39 deletions(-) diff --git a/packages/@glimmer-workspace/integration-tests/test/debug-render-tree-test.ts b/packages/@glimmer-workspace/integration-tests/test/debug-render-tree-test.ts index 52ee3cce86..f1acdc5d49 100644 --- a/packages/@glimmer-workspace/integration-tests/test/debug-render-tree-test.ts +++ b/packages/@glimmer-workspace/integration-tests/test/debug-render-tree-test.ts @@ -313,7 +313,7 @@ class DebugRenderTreeTest extends RenderTest { args: { positional: [this.element.firstChild], named: {} }, instance: (instance: GlimmerishComponent) => instance === null, template: null, - bounds: this.nodeBounds(this.element.firstChild!.firstChild, this.element), + bounds: this.elementBounds(this.element.firstChild! as unknown as Element), children: [ { type: 'component', @@ -519,12 +519,12 @@ class DebugRenderTreeTest extends RenderTest { assert.deepEqual(this.delegate.getCapturedRenderTree(), [], 'there was no output'); } - nodeBounds(_node: SimpleNode | null, parent?: SimpleNode): CapturedBounds { + nodeBounds(_node: SimpleNode | null): CapturedBounds { let node = expect(_node, 'BUG: Expected node'); return { parentElement: expect( - parent || node.parentNode, + node.parentNode, 'BUG: detached node' ) as unknown as SimpleNode as SimpleElement, firstNode: node as unknown as SimpleNode, diff --git a/packages/@glimmer/interfaces/lib/dom/attributes.d.ts b/packages/@glimmer/interfaces/lib/dom/attributes.d.ts index 370671a937..82adb3bcb6 100644 --- a/packages/@glimmer/interfaces/lib/dom/attributes.d.ts +++ b/packages/@glimmer/interfaces/lib/dom/attributes.d.ts @@ -37,8 +37,8 @@ export interface DOMStack { element: SimpleElement, guid: string, insertBefore: Maybe - ): Nullable; - popRemoteElement(): void; + ): RemoteLiveBlock; + popRemoteElement(): RemoteLiveBlock; popElement(): void; openElement(tag: string, _operations?: ElementOperations): SimpleElement; flushElement(modifiers: Nullable): void; diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts index d6c2a377dc..94542550d0 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts @@ -19,7 +19,7 @@ import { CheckOption, CheckString, } from '@glimmer/debug'; -import { associateDestroyableChild, destroy } from '@glimmer/destroyable'; +import { associateDestroyableChild, destroy, registerDestructor } from '@glimmer/destroyable'; import { getInternalModifierManager } from '@glimmer/manager'; import { createComputeRef, isConstRef, valueForRef } from '@glimmer/reference'; import { debugToString, expect, isObject } from '@glimmer/util'; @@ -32,6 +32,7 @@ import type { DynamicAttribute } from '../../vm/attributes/dynamic'; import { isCurriedType, resolveCurriedValue } from '../../curried-value'; import { APPEND_OPCODES } from '../../opcodes'; import { CONSTANTS } from '../../symbols'; +import { createCapturedArgs } from '../../vm/arguments'; import { CheckArguments, CheckOperations, CheckReference } from './-debug-strip'; import { Assert } from './vm'; @@ -71,10 +72,36 @@ APPEND_OPCODES.add(Op.PushRemoteElement, (vm) => { let block = vm.elements().pushRemoteElement(element, guid, insertBefore); if (block) vm.associateDestroyable(block); + + if (vm.env.debugRenderTree !== undefined) { + // Note that there is nothing to update – when the args for an + // {{#in-element}} changes it gets torn down and a new one is + // re-created/rendered in its place (see the `Assert`s above) + let args = createCapturedArgs( + insertBefore === undefined ? {} : { insertBefore: insertBeforeRef }, + [elementRef] + ); + + vm.env.debugRenderTree.create(block, { + type: 'keyword', + name: 'in-element', + args, + instance: null, + }); + + registerDestructor(block, () => { + vm.env.debugRenderTree?.willDestroy(block); + }); + } }); APPEND_OPCODES.add(Op.PopRemoteElement, (vm) => { - vm.elements().popRemoteElement(); + let bounds = vm.elements().popRemoteElement(); + + if (vm.env.debugRenderTree !== undefined) { + // The RemoteLiveBlock is also its bounds + vm.env.debugRenderTree.didRender(bounds, bounds); + } }); APPEND_OPCODES.add(Op.FlushElement, (vm) => { diff --git a/packages/@glimmer/runtime/lib/vm/element-builder.ts b/packages/@glimmer/runtime/lib/vm/element-builder.ts index 29ce5318c8..f79273694f 100644 --- a/packages/@glimmer/runtime/lib/vm/element-builder.ts +++ b/packages/@glimmer/runtime/lib/vm/element-builder.ts @@ -1,7 +1,6 @@ import type { AttrNamespace, Bounds, - CapturedArguments, Cursor, CursorStackSymbol, ElementBuilder, @@ -13,7 +12,6 @@ import type { Maybe, ModifierInstance, Nullable, - Reference, SimpleComment, SimpleDocumentFragment, SimpleElement, @@ -22,7 +20,6 @@ import type { UpdatableBlock, } from '@glimmer/interfaces'; import { destroy, registerDestructor } from '@glimmer/destroyable'; -import { createConstRef } from '@glimmer/reference'; import { assert, expect, Stack } from '@glimmer/util'; import type { DynamicAttribute } from './attributes/dynamic'; @@ -217,26 +214,7 @@ export class NewElementBuilder implements ElementBuilder { guid: string, insertBefore: Maybe ): RemoteLiveBlock { - const block = this.__pushRemoteElement(element, guid, insertBefore); - if (this.env.debugRenderTree) { - const namedArgs: Record = {}; - if (insertBefore !== undefined) { - namedArgs['insertBefore'] = createConstRef(insertBefore, false); - } - this.env.debugRenderTree.create(block, { - type: 'keyword', - name: 'in-element', - args: { - named: namedArgs, - positional: [createConstRef(element, false)], - } as CapturedArguments, - instance: null, - }); - registerDestructor(block, () => { - this.env.debugRenderTree?.willDestroy(block); - }); - } - return block; + return this.__pushRemoteElement(element, guid, insertBefore); } __pushRemoteElement( @@ -257,17 +235,11 @@ export class NewElementBuilder implements ElementBuilder { return this.pushLiveBlock(block, true); } - popRemoteElement(): void { + popRemoteElement(): RemoteLiveBlock { const block = this.popBlock(); + assert(block instanceof RemoteLiveBlock, '[BUG] expecting a RemoteLiveBlock'); this.popElement(); - const parentElement = this.element; - if (this.env.debugRenderTree) { - this.env.debugRenderTree?.didRender(block, { - parentElement: () => parentElement, - firstNode: () => block.firstNode(), - lastNode: () => block.lastNode(), - }); - } + return block; } protected pushElement(element: SimpleElement, nextSibling: Maybe = null): void {