Clean up implementation

Author: chancancode

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,

packages/@glimmer/interfaces/lib/dom/attributes.d.ts

@@ -37,8 +37,8 @@ export interface DOMStack {
     element: SimpleElement,
     guid: string,
     insertBefore: Maybe<SimpleNode>
-  ): Nullable<RemoteLiveBlock>;
-  popRemoteElement(): void;
+  ): RemoteLiveBlock;
+  popRemoteElement(): RemoteLiveBlock;
   popElement(): void;
   openElement(tag: string, _operations?: ElementOperations): SimpleElement;
   flushElement(modifiers: Nullable<ModifierInstance[]>): void;

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) => {

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<SimpleNode>
   ): RemoteLiveBlock {
-    const block = this.__pushRemoteElement(element, guid, insertBefore);
-    if (this.env.debugRenderTree) {
-      const namedArgs: Record<string, Reference> = {};
-      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<SimpleNode> = null): void {