Skip to content

Commit bfeebae

Browse files
chancancodechancancode
committed
Cleanup modifier debug render tree implementation
1 parent a5c2af3 commit bfeebae

File tree

8 files changed

+118
-124
lines changed

8 files changed

+118
-124
lines changed

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

+5-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,11 @@ class OnModifierManager implements InternalModifierManager<OnModifierState, obje
2929
}
3030

3131
getDebugName() {
32-
return 'on-modifier';
32+
return 'on';
33+
}
34+
35+
getDebugInstance() {
36+
return null;
3337
}
3438

3539
install(state: OnModifierState) {

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

+6-2
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,12 @@ export class TestModifierManager
4444
return tag;
4545
}
4646

47-
getDebugName() {
48-
return '<unknown>';
47+
getDebugName({ Klass }: TestModifierDefinitionState) {
48+
return Klass?.name || '<unknown>';
49+
}
50+
51+
getDebugInstance({ instance }: TestModifier) {
52+
return instance;
4953
}
5054

5155
install({ element, args, instance }: TestModifier) {

‎packages/@glimmer-workspace/integration-tests/test/debug-render-tree-test.ts

+52-79
Original file line numberDiff line numberDiff line change
@@ -67,21 +67,6 @@ class DebugRenderTreeDelegate extends JitRenderDelegate {
6767

6868
this.registry.register('component', name, definition);
6969
}
70-
71-
registerCustomModifier(name: string) {
72-
const r = setModifierManager(
73-
() => ({
74-
capabilities: modifierCapabilities('3.22'),
75-
createModifier() {},
76-
installModifier() {},
77-
updateModifier() {},
78-
destroyModifier() {},
79-
}),
80-
class DidInsertModifier {}
81-
);
82-
this.registry.register('modifier', name, r);
83-
return r;
84-
}
8570
}
8671

8772
class DebugRenderTreeTest extends RenderTest {
@@ -342,71 +327,81 @@ class DebugRenderTreeTest extends RenderTest {
342327
],
343328
},
344329
],
345-
}
330+
},
346331
]);
347332
}
348333

349334
@test modifiers() {
350335
this.registerComponent('Glimmer', 'HelloWorld', 'Hello World');
351336
const didInsert = () => null;
352-
this.registerModifier(
353-
'did-insert',
354-
class {
355-
element?: SimpleElement;
356-
didInsertElement() {}
357-
didUpdate() {}
358-
willDestroyElement() {}
359-
}
337+
338+
class DidInsertModifier {
339+
element?: SimpleElement;
340+
didInsertElement() {}
341+
didUpdate() {}
342+
willDestroyElement() {}
343+
}
344+
345+
this.registerModifier('did-insert', DidInsertModifier);
346+
347+
class MyCustomModifier {}
348+
349+
setModifierManager(
350+
() => ({
351+
capabilities: modifierCapabilities('3.22'),
352+
createModifier() {
353+
return new MyCustomModifier();
354+
},
355+
installModifier() {},
356+
updateModifier() {},
357+
destroyModifier() {},
358+
}),
359+
MyCustomModifier
360360
);
361-
const modifier = this.defineModifier('did-update');
361+
362+
const foo = Symbol('foo');
363+
const bar = Symbol('bar');
362364

363365
this.render(
364-
`<div {{on 'click' this.didInsert}} {{did-insert this.didInsert}} {{did-update this.didInsert}} {{this.modifier this.didInsert}}
366+
`<div {{on 'click' this.didInsert}} {{did-insert this.foo bar=this.bar}} {{this.modifier this.bar foo=this.foo}}
365367
><HelloWorld />
366368
{{~#if this.more~}}
367-
<div {{on 'click' this.didInsert}}></div>
369+
<div {{on 'click' this.didInsert passive=true}}></div>
368370
{{~/if~}}
369371
</div>`,
370372
{
371373
didInsert: didInsert,
372-
modifier: modifier,
374+
modifier: MyCustomModifier,
375+
foo,
376+
bar,
373377
more: false,
374378
}
375379
);
376380

377381
this.assertRenderTree([
378-
{
379-
type: 'modifier',
380-
name: 'did-update',
381-
args: { positional: [didInsert], named: {} },
382-
instance: (instance: any) => typeof instance.installModifier === 'function',
383-
template: null,
384-
bounds: this.nodeBounds(this.element.firstChild),
385-
children: [],
386-
},
387382
{
388383
type: 'modifier',
389384
name: 'on',
390385
args: { positional: ['click', didInsert], named: {} },
391-
instance: (instance: any) => typeof instance === 'object',
386+
instance: null,
392387
template: null,
393388
bounds: this.nodeBounds(this.element.firstChild),
394389
children: [],
395390
},
396391
{
397392
type: 'modifier',
398393
name: 'DidInsertModifier',
399-
args: { positional: [didInsert], named: {} },
400-
instance: (instance: any) => typeof instance.installModifier === 'function',
394+
args: { positional: [foo], named: { bar } },
395+
instance: (instance: unknown) => instance instanceof DidInsertModifier,
401396
template: null,
402397
bounds: this.nodeBounds(this.element.firstChild),
403398
children: [],
404399
},
405400
{
406401
type: 'modifier',
407-
name: 'did-insert',
408-
args: { positional: [didInsert], named: {} },
409-
instance: (instance: any) => typeof instance.install === 'function',
402+
name: 'MyCustomModifier',
403+
args: { positional: [bar], named: { foo } },
404+
instance: (instance: unknown) => instance instanceof MyCustomModifier,
410405
template: null,
411406
bounds: this.nodeBounds(this.element.firstChild),
412407
children: [],
@@ -431,34 +426,25 @@ class DebugRenderTreeTest extends RenderTest {
431426
type: 'modifier',
432427
name: 'on',
433428
args: { positional: ['click', didInsert], named: {} },
434-
instance: (instance: any) => typeof instance === 'object',
435-
template: null,
436-
bounds: this.nodeBounds(this.element.firstChild),
437-
children: [],
438-
},
439-
{
440-
type: 'modifier',
441-
name: 'did-update',
442-
args: { positional: [didInsert], named: {} },
443-
instance: (instance: any) => typeof instance.installModifier === 'function',
429+
instance: null,
444430
template: null,
445431
bounds: this.nodeBounds(this.element.firstChild),
446432
children: [],
447433
},
448434
{
449435
type: 'modifier',
450436
name: 'DidInsertModifier',
451-
args: { positional: [didInsert], named: {} },
452-
instance: (instance: any) => typeof instance.installModifier === 'function',
437+
args: { positional: [foo], named: { bar } },
438+
instance: (instance: unknown) => instance instanceof DidInsertModifier,
453439
template: null,
454440
bounds: this.nodeBounds(this.element.firstChild),
455441
children: [],
456442
},
457443
{
458444
type: 'modifier',
459-
name: 'did-insert',
460-
args: { positional: [didInsert], named: {} },
461-
instance: (instance: any) => typeof instance.install === 'function',
445+
name: 'MyCustomModifier',
446+
args: { positional: [bar], named: { foo } },
447+
instance: (instance: unknown) => instance instanceof MyCustomModifier,
462448
template: null,
463449
bounds: this.nodeBounds(this.element.firstChild),
464450
children: [],
@@ -475,8 +461,8 @@ class DebugRenderTreeTest extends RenderTest {
475461
{
476462
type: 'modifier',
477463
name: 'on',
478-
args: { positional: ['click', didInsert], named: {} },
479-
instance: (instance: any) => typeof instance === 'object',
464+
args: { positional: ['click', didInsert], named: { passive: true } },
465+
instance: null,
480466
template: null,
481467
bounds: this.nodeBounds(this.element.firstChild!.lastChild),
482468
children: [],
@@ -488,38 +474,29 @@ class DebugRenderTreeTest extends RenderTest {
488474
});
489475

490476
this.assertRenderTree([
491-
{
492-
type: 'modifier',
493-
name: 'did-update',
494-
args: { positional: [didInsert], named: {} },
495-
instance: (instance: any) => typeof instance.installModifier === 'function',
496-
template: null,
497-
bounds: this.nodeBounds(this.element.firstChild),
498-
children: [],
499-
},
500477
{
501478
type: 'modifier',
502479
name: 'on',
503480
args: { positional: ['click', didInsert], named: {} },
504-
instance: (instance: any) => typeof instance === 'object',
481+
instance: null,
505482
template: null,
506483
bounds: this.nodeBounds(this.element.firstChild),
507484
children: [],
508485
},
509486
{
510487
type: 'modifier',
511488
name: 'DidInsertModifier',
512-
args: { positional: [didInsert], named: {} },
513-
instance: (instance: any) => typeof instance.installModifier === 'function',
489+
args: { positional: [foo], named: { bar } },
490+
instance: (instance: unknown) => instance instanceof DidInsertModifier,
514491
template: null,
515492
bounds: this.nodeBounds(this.element.firstChild),
516493
children: [],
517494
},
518495
{
519496
type: 'modifier',
520-
name: 'did-insert',
521-
args: { positional: [didInsert], named: {} },
522-
instance: (instance: any) => typeof instance.install === 'function',
497+
name: 'MyCustomModifier',
498+
args: { positional: [bar], named: { foo } },
499+
instance: (instance: unknown) => instance instanceof MyCustomModifier,
523500
template: null,
524501
bounds: this.nodeBounds(this.element.firstChild),
525502
children: [],
@@ -724,10 +701,6 @@ class DebugRenderTreeTest extends RenderTest {
724701
assert.deepEqual(this.delegate.getCapturedRenderTree(), [], 'there was no output');
725702
}
726703

727-
defineModifier(name: string) {
728-
return this.delegate.registerCustomModifier(name);
729-
}
730-
731704
nodeBounds(_node: SimpleNode | null): CapturedBounds {
732705
let node = expect(_node, 'BUG: Expected node');
733706

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

+1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export interface InternalModifierManager<
2323
getTag(modifier: TModifierInstanceState): UpdatableTag | null;
2424

2525
getDebugName(Modifier: TModifierDefinitionState): string;
26+
getDebugInstance(Modifier: TModifierInstanceState): unknown;
2627

2728
// At initial render, the modifier gets a chance to install itself on the
2829
// element it is managing. It can also return a bucket of state that

‎packages/@glimmer/interfaces/lib/runtime/debug-render-tree.d.ts

+7-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,13 @@ import type { SimpleElement, SimpleNode } from '@simple-dom/interface';
33
import type { Bounds } from '../dom/bounds.js';
44
import type { Arguments, CapturedArguments } from './arguments.js';
55

6-
export type RenderNodeType = 'outlet' | 'engine' | 'route-template' | 'component' | 'modifier' | 'keyword';
6+
export type RenderNodeType =
7+
| 'outlet'
8+
| 'engine'
9+
| 'route-template'
10+
| 'component'
11+
| 'modifier'
12+
| 'keyword';
713

814
export interface RenderNode {
915
type: RenderNodeType;

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

+10-6
Original file line numberDiff line numberDiff line change
@@ -112,17 +112,21 @@ export class CustomModifierManager<O extends Owner, ModifierInstance>
112112
modifier: instance,
113113
};
114114

115-
if (import.meta.env.DEV) {
116-
state.debugName = typeof definition === 'function' ? definition.name : definition.toString();
117-
}
118-
119115
registerDestructor(state, () => delegate.destroyModifier(instance, args));
120116

121117
return state;
122118
}
123119

124-
getDebugName({ debugName }: CustomModifierState<ModifierInstance>) {
125-
return debugName!;
120+
getDebugName(definition: object) {
121+
if (typeof definition === 'function') {
122+
return definition.name || definition.toString();
123+
} else {
124+
return '<unknown>';
125+
}
126+
}
127+
128+
getDebugInstance({ modifier }: CustomModifierState<ModifierInstance>) {
129+
return modifier;
126130
}
127131

128132
getTag({ tag }: CustomModifierState<ModifierInstance>) {

0 commit comments

Comments
 (0)