Merge pull request #1559 from patricklx/add-modifiers-to-debug-render-tree

NullVoxPopuli · web-flow · commit 9bfc4dde5758 · 2024-03-08T22:12:23.000-05:00
add modifiers to debug render tree
diff --git a/packages/@glimmer-workspace/benchmark-env/lib/benchmark/on-modifier.ts b/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) {
diff --git a/packages/@glimmer-workspace/integration-tests/lib/modifiers.ts b/packages/@glimmer-workspace/integration-tests/lib/modifiers.ts
@@ -44,8 +44,12 @@ export class TestModifierManager
     return tag;
   }
 
-  getDebugName() {
-    return '<unknown>';
+  getDebugName({ Klass }: TestModifierDefinitionState) {
+    return Klass?.name || '<unknown>';
+  }
+
+  getDebugInstance({ instance }: TestModifier) {
+    return instance;
   }
 
   install({ element, args, instance }: TestModifier) {
diff --git a/packages/@glimmer-workspace/integration-tests/lib/setup-harness.ts b/packages/@glimmer-workspace/integration-tests/lib/setup-harness.ts
@@ -63,6 +63,19 @@ export async function setupQunit() {
     qunit.moduleDone(pause);
   }
 
+  // @ts-expect-error missing in types, does exist: https://api.qunitjs.com/callbacks/QUnit.on/#the-testend-event
+  QUnit.on('testEnd', (testEnd) => {
+    if (testEnd.status === 'failed') {
+      testEnd.errors.forEach((assertion: any) => {
+        console.error(assertion.stack);
+        // message: speedometer
+        // actual: 75
+        // expected: 88
+        // stack: at dmc.test.js:12
+      });
+    }
+  });
+
   qunit.done(({ failed }) => {
     if (failed > 0) {
       console.log('[HARNESS] fail');
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
@@ -9,7 +9,7 @@ import type {
   SimpleNode,
 } from '@glimmer/interfaces';
 import type { TemplateOnlyComponent } from '@glimmer/runtime';
-import { setComponentTemplate } from '@glimmer/manager';
+import { modifierCapabilities, setComponentTemplate, setModifierManager } from '@glimmer/manager';
 import { EMPTY_ARGS, templateOnlyComponent, TemplateOnlyComponentManager } from '@glimmer/runtime';
 import { assign, expect } from '@glimmer/util';
 
@@ -331,6 +331,188 @@ class DebugRenderTreeTest extends RenderTest {
     ]);
   }
 
+  @test modifiers() {
+    this.registerComponent('Glimmer', 'HelloWorld', 'Hello World');
+    const didInsert = () => null;
+
+    class DidInsertModifier {
+      element?: SimpleElement;
+      didInsertElement() {}
+      didUpdate() {}
+      willDestroyElement() {}
+    }
+
+    this.registerModifier('did-insert', DidInsertModifier);
+
+    class MyCustomModifier {}
+
+    setModifierManager(
+      () => ({
+        capabilities: modifierCapabilities('3.22'),
+        createModifier() {
+          return new MyCustomModifier();
+        },
+        installModifier() {},
+        updateModifier() {},
+        destroyModifier() {},
+      }),
+      MyCustomModifier
+    );
+
+    const foo = Symbol('foo');
+    const bar = Symbol('bar');
+
+    this.render(
+      `<div {{on 'click' this.didInsert}} {{did-insert this.foo bar=this.bar}} {{this.modifier this.bar foo=this.foo}}
+      ><HelloWorld />
+      {{~#if this.more~}}
+        <div {{on 'click' this.didInsert passive=true}}></div>
+      {{~/if~}}
+      </div>`,
+      {
+        didInsert: didInsert,
+        modifier: MyCustomModifier,
+        foo,
+        bar,
+        more: false,
+      }
+    );
+
+    this.assertRenderTree([
+      {
+        type: 'modifier',
+        name: 'on',
+        args: { positional: ['click', didInsert], named: {} },
+        instance: null,
+        template: null,
+        bounds: this.nodeBounds(this.element.firstChild),
+        children: [],
+      },
+      {
+        type: 'modifier',
+        name: 'DidInsertModifier',
+        args: { positional: [foo], named: { bar } },
+        instance: (instance: unknown) => instance instanceof DidInsertModifier,
+        template: null,
+        bounds: this.nodeBounds(this.element.firstChild),
+        children: [],
+      },
+      {
+        type: 'modifier',
+        name: 'MyCustomModifier',
+        args: { positional: [bar], named: { foo } },
+        instance: (instance: unknown) => instance instanceof MyCustomModifier,
+        template: null,
+        bounds: this.nodeBounds(this.element.firstChild),
+        children: [],
+      },
+      {
+        type: 'component',
+        name: 'HelloWorld',
+        args: { positional: [], named: {} },
+        instance: (instance: any) => instance !== undefined,
+        template: '(unknown template module)',
+        bounds: this.nodeBounds(this.element.firstChild!.firstChild),
+        children: [],
+      },
+    ]);
+
+    this.rerender({
+      more: true,
+    });
+
+    this.assertRenderTree([
+      {
+        type: 'modifier',
+        name: 'on',
+        args: { positional: ['click', didInsert], named: {} },
+        instance: null,
+        template: null,
+        bounds: this.nodeBounds(this.element.firstChild),
+        children: [],
+      },
+      {
+        type: 'modifier',
+        name: 'DidInsertModifier',
+        args: { positional: [foo], named: { bar } },
+        instance: (instance: unknown) => instance instanceof DidInsertModifier,
+        template: null,
+        bounds: this.nodeBounds(this.element.firstChild),
+        children: [],
+      },
+      {
+        type: 'modifier',
+        name: 'MyCustomModifier',
+        args: { positional: [bar], named: { foo } },
+        instance: (instance: unknown) => instance instanceof MyCustomModifier,
+        template: null,
+        bounds: this.nodeBounds(this.element.firstChild),
+        children: [],
+      },
+      {
+        type: 'component',
+        name: 'HelloWorld',
+        args: { positional: [], named: {} },
+        instance: (instance: any) => instance !== undefined,
+        template: '(unknown template module)',
+        bounds: this.nodeBounds(this.element.firstChild!.firstChild),
+        children: [],
+      },
+      {
+        type: 'modifier',
+        name: 'on',
+        args: { positional: ['click', didInsert], named: { passive: true } },
+        instance: null,
+        template: null,
+        bounds: this.nodeBounds(this.element.firstChild!.lastChild),
+        children: [],
+      },
+    ]);
+
+    this.rerender({
+      more: false,
+    });
+
+    this.assertRenderTree([
+      {
+        type: 'modifier',
+        name: 'on',
+        args: { positional: ['click', didInsert], named: {} },
+        instance: null,
+        template: null,
+        bounds: this.nodeBounds(this.element.firstChild),
+        children: [],
+      },
+      {
+        type: 'modifier',
+        name: 'DidInsertModifier',
+        args: { positional: [foo], named: { bar } },
+        instance: (instance: unknown) => instance instanceof DidInsertModifier,
+        template: null,
+        bounds: this.nodeBounds(this.element.firstChild),
+        children: [],
+      },
+      {
+        type: 'modifier',
+        name: 'MyCustomModifier',
+        args: { positional: [bar], named: { foo } },
+        instance: (instance: unknown) => instance instanceof MyCustomModifier,
+        template: null,
+        bounds: this.nodeBounds(this.element.firstChild),
+        children: [],
+      },
+      {
+        type: 'component',
+        name: 'HelloWorld',
+        args: { positional: [], named: {} },
+        instance: (instance: any) => instance !== undefined,
+        template: '(unknown template module)',
+        bounds: this.nodeBounds(this.element.firstChild!.firstChild),
+        children: [],
+      },
+    ]);
+  }
+
   @test 'getDebugCustomRenderTree works'() {
     let bucket1 = {};
     let instance1 = {};
diff --git a/packages/@glimmer/interfaces/lib/managers/internal/modifier.d.ts b/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
diff --git 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,13 @@ 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' | 'keyword';
+export type RenderNodeType =
+  | 'outlet'
+  | 'engine'
+  | 'route-template'
+  | 'component'
+  | 'modifier'
+  | 'keyword';
 
 export interface RenderNode {
   type: RenderNodeType;
diff --git a/packages/@glimmer/manager/lib/public/modifier.ts b/packages/@glimmer/manager/lib/public/modifier.ts
@@ -112,17 +112,21 @@ export class CustomModifierManager<O extends Owner, ModifierInstance>
       modifier: instance,
     };
 
-    if (import.meta.env.DEV) {
-      state.debugName = typeof definition === 'function' ? definition.name : definition.toString();
-    }
-
     registerDestructor(state, () => delegate.destroyModifier(instance, args));
 
     return state;
   }
 
-  getDebugName({ debugName }: CustomModifierState<ModifierInstance>) {
-    return debugName!;
+  getDebugName(definition: object) {
+    if (typeof definition === 'function') {
+      return definition.name || definition.toString();
+    } else {
+      return '<unknown>';
+    }
+  }
+
+  getDebugInstance({ modifier }: CustomModifierState<ModifierInstance>) {
+    return modifier;
   }
 
   getTag({ tag }: CustomModifierState<ModifierInstance>) {
diff --git a/packages/@glimmer/manager/test/managers-test.ts b/packages/@glimmer/manager/test/managers-test.ts
@@ -289,6 +289,10 @@ module('Managers', () => {
           return 'internal';
         }
 
+        getDebugInstance() {
+          return null;
+        }
+
         getDestroyable() {
           return null;
         }
diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts
@@ -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';
@@ -489,8 +490,46 @@ export class ComponentElementOperations implements ElementOperations {
     this.attributes[name] = deferred;
   }
 
-  addModifier(modifier: ModifierInstance): void {
+  addModifier(vm: InternalVM, modifier: ModifierInstance, capturedArgs: CapturedArguments): void {
     this.modifiers.push(modifier);
+
+    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;
+      }
+
+      let { element, constructing } = vm.elements();
+      let name = manager.getDebugName(definition.state);
+      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: capturedArgs,
+        instance,
+      });
+
+      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);
+      });
+    }
   }
 
   flush(vm: InternalVM): ModifierInstance[] {
@@ -645,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);
       });
diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts
diff --git a/packages/@glimmer/runtime/lib/modifiers/on.ts b/packages/@glimmer/runtime/lib/modifiers/on.ts

Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,11 @@ class OnModifierManager implements InternalModifierManager<OnModifierState, obje`
`29`	`29`	`}`
`30`	`30`
`31`	`31`	`getDebugName() {`
`32`		`- return 'on-modifier';`
	`32`	`+ return 'on';`
	`33`	`+ }`
	`34`	`+`
	`35`	`+ getDebugInstance() {`
	`36`	`+ return null;`
`33`	`37`	`}`
`34`	`38`
`35`	`39`	`install(state: OnModifierState) {`
Original file line number	Diff line number	Diff line change
`@@ -44,8 +44,12 @@ export class TestModifierManager`
`44`	`44`	`return tag;`
`45`	`45`	`}`
`46`	`46`
`47`		`- getDebugName() {`
`48`		`- return '<unknown>';`
	`47`	`+ getDebugName({ Klass }: TestModifierDefinitionState) {`
	`48`	`+ return Klass?.name \|\| '<unknown>';`
	`49`	`+ }`
	`50`	`+`
	`51`	`+ getDebugInstance({ instance }: TestModifier) {`
	`52`	`+ return instance;`
`49`	`53`	`}`
`50`	`54`
`51`	`55`	`install({ element, args, instance }: TestModifier) {`
Original file line number	Diff line number	Diff line change
`@@ -289,6 +289,10 @@ module('Managers', () => {`
`289`	`289`	`return 'internal';`
`290`	`290`	`}`
`291`	`291`
	`292`	`+ getDebugInstance() {`
	`293`	`+ return null;`
	`294`	`+ }`
	`295`	`+`
`292`	`296`	`getDestroyable() {`
`293`	`297`	`return null;`
`294`	`298`	`}`