Cleanup modifier debug render tree implementation

Author: chancancode

packages/@glimmer-workspace/benchmark-env/lib/benchmark/on-modifier.ts

@@ -29,7 +29,11 @@ class OnModifierManager implements InternalModifierManager<OnModifierState, obje
   }
 
   getDebugName() {
-    return 'on-modifier';
+    return 'on';
+  }
+
+  getDebugInstance() {
+    return null;
   }
 
   install(state: OnModifierState) {

packages/@glimmer-workspace/integration-tests/lib/modifiers.ts

@@ -44,8 +44,16 @@ export class TestModifierManager
     return tag;
   }
 
-  getDebugName() {
-    return '<unknown>';
+  getDebugName({ instance }: TestModifier) {
+    if (instance && 'name' in instance && typeof instance.name === 'string') {
+      return instance.name;
+    } else {
+      return '<unknown>';
+    }
+  }
+
+  getDebugInstance({ instance }: TestModifier) {
+    return instance;
   }
 
   install({ element, args, instance }: TestModifier) {

packages/@glimmer/interfaces/lib/managers/internal/modifier.d.ts

@@ -23,6 +23,7 @@ export interface InternalModifierManager<
   getTag(modifier: TModifierInstanceState): UpdatableTag | null;
 
   getDebugName(Modifier: TModifierDefinitionState): string;
+  getDebugInstance(Modifier: TModifierInstanceState): unknown;
 
   // At initial render, the modifier gets a chance to install itself on the
   // element it is managing. It can also return a bucket of state that

packages/@glimmer/manager/lib/public/modifier.ts

@@ -125,6 +125,10 @@ export class CustomModifierManager<O extends Owner, ModifierInstance>
     return debugName!;
   }
 
+  getDebugInstance({ modifier }: CustomModifierState<ModifierInstance>) {
+    return modifier;
+  }
+
   getTag({ tag }: CustomModifierState<ModifierInstance>) {
     return tag;
   }

packages/@glimmer/runtime/lib/compiled/opcodes/component.ts

@@ -37,7 +37,7 @@ import {
 } from '@glimmer/debug';
 import { registerDestructor } from '@glimmer/destroyable';
 import { managerHasCapability } from '@glimmer/manager';
-import { createUnboundRef, isConstRef, REFERENCE, valueForRef } from '@glimmer/reference';
+import { isConstRef, valueForRef } from '@glimmer/reference';
 import {
   assert,
   assign,
@@ -56,6 +56,7 @@ import type { UpdatingVM } from '../../vm';
 import type { InternalVM } from '../../vm/append';
 import type { BlockArgumentsImpl } from '../../vm/arguments';
 
+import { ConcreteBounds } from '../../bounds';
 import { hasCustomDebugRenderTreeLifecycle } from '../../component/interfaces';
 import { resolveComponent } from '../../component/resolve';
 import { isCurriedType, isCurriedValue, resolveCurriedValue } from '../../curried-value';
@@ -491,44 +492,43 @@ export class ComponentElementOperations implements ElementOperations {
 
   addModifier(vm: InternalVM, modifier: ModifierInstance, capturedArgs: CapturedArguments): void {
     this.modifiers.push(modifier);
-    const env = vm.env;
-    if (env.debugRenderTree) {
-      const element = vm.elements().constructing!;
-      const name =
-        modifier.definition.resolvedName ||
-        (modifier.state as any).debugName ||
-        (modifier.definition.state as any).name ||
-        'unknown-modifier';
-      const instance = (modifier.state as any).delegate || modifier.manager;
-      const args: any = {
-        positional: [],
-        named: {},
-      };
-      for (const value of capturedArgs.positional) {
-        if (value && value[REFERENCE]) {
-          args.positional.push(value);
-        } else {
-          args.positional.push(createUnboundRef(value, false));
-        }
-      }
-      for (const [key, value] of Object.entries(capturedArgs.named)) {
-        args.named[key] = createUnboundRef(value, false);
+
+    if (vm.env.debugRenderTree !== undefined) {
+      const { manager, definition, state } = modifier;
+
+      // TODO: we need a stable object for the debugRenderTree as the key, add support for
+      // the case where the state is a primitive, or if in practice we always have/require
+      // an object, then change the internal types to reflect that
+      if (state === null || (typeof state !== 'object' && typeof state !== 'function')) {
+        return;
       }
-      env.debugRenderTree.create(modifier.state as any, {
+
+      let { element, constructing } = vm.elements();
+      let name = manager.getDebugName(definition);
+      let instance = manager.getDebugInstance(state);
+
+      assert(constructing, `Expected a constructing element in addModifier`);
+
+      let bounds = new ConcreteBounds(element, constructing, constructing);
+
+      vm.env.debugRenderTree.create(state, {
         type: 'modifier',
         name,
-        args,
+        args: capturedArgs,
         instance,
       });
-      env.debugRenderTree?.didRender(modifier.state as any, {
-        parentElement: () => (element as any).parentElement,
-        firstNode: () => element,
-        lastNode: () => element,
+
+      vm.env.debugRenderTree.didRender(state, bounds);
+
+      // For tearing down the debugRenderTree
+      vm.associateDestroyable(state);
+
+      vm.updateWith(new DebugRenderTreeUpdateOpcode(state));
+      vm.updateWith(new DebugRenderTreeDidRenderOpcode(state, bounds));
+
+      registerDestructor(state, () => {
+        vm.env.debugRenderTree?.willDestroy(state);
       });
-      registerDestructor(
-        modifier.state as any,
-        () => vm.env.debugRenderTree?.willDestroy(modifier.state as any)
-      );
     }
   }
 
@@ -684,8 +684,6 @@ APPEND_OPCODES.add(Op.GetComponentSelf, (vm, { op1: _state, op2: _names }) => {
         instance: valueForRef(selfRef),
       });
 
-      vm.associateDestroyable(instance);
-
       registerDestructor(instance, () => {
         vm.env.debugRenderTree?.willDestroy(instance);
       });

packages/@glimmer/runtime/lib/modifiers/on.ts

@@ -312,6 +312,22 @@ class OnModifierManager implements InternalModifierManager<OnModifierState, obje
     return 'on';
   }
 
+  getDebugInstance(state: OnModifierState): unknown {
+    return {
+      get eventName() {
+        return state.listener?.eventName;
+      },
+
+      get listener() {
+        return state.listener?.userProvidedCallback;
+      },
+
+      get options() {
+        return state?.listener?.options;
+      },
+    };
+  }
+
   get counters(): { adds: number; removes: number } {
     return { adds, removes };
   }