Split modifier and component to resolved variants

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

@@ -116,6 +116,7 @@ class DebugRenderTreeTest extends RenderTest {
     const HelloWorld = defComponent('<p ...attributes>{{@arg}}</p>');
     const noopFn = () => {};
     const noop = defineSimpleModifier(noopFn);
+
     const Root = defComponent(
       `<HelloWorld {{noop}} @arg="first"/>{{#if state.showSecond}}<HelloWorld @arg="second"/>{{/if}}`,
       { scope: { HelloWorld, state, noop }, emit: { moduleName: 'root.hbs' } }
@@ -144,7 +145,7 @@ class DebugRenderTreeTest extends RenderTest {
             children: [
               {
                 type: 'modifier',
-                name: 'noop',
+                name: 'noopFn',
                 args: { positional: [], named: {} },
                 instance: (modifier: unknown) => modifier && Reflect.get(modifier, 'fn') === noopFn,
                 template: null,

packages/@glimmer-workspace/integration-tests/test/strict-mode-test.ts

@@ -586,7 +586,7 @@ class StaticStrictModeTest extends RenderTest {
 
     this.assert.throws(() => {
       this.renderComponent(Bar);
-    }, /Attempted to load a modifier, but there wasn't a modifier manager associated with the definition. The definition was:/u);
+    }, /Expected a dynamic modifier definition, but received an object or function that did not have a modifier manager associated with it. The dynamic invocation was `\{\{false\}\}`/u);
   }
 }
 

packages/@glimmer/compiler/lib/builder/builder.ts

@@ -232,16 +232,14 @@ export function buildNormalizedStatements(
 export function buildAppend(
   trusted: boolean,
   expr: Expressions.Expression
-): WireFormat.Statement[] {
+): WireFormat.Statements.SomeAppend {
   if (Array.isArray(expr) && expr[0] === Op.GetFreeAsComponentOrHelperHead) {
-    return [
-      trusted
-        ? [Op.UnknownTrustingAppend, expr as Expressions.GetUnknownAppend]
-        : [Op.UnknownAppend, expr as Expressions.GetUnknownAppend],
-    ];
+    return trusted
+      ? [Op.UnknownTrustingAppend, expr as Expressions.GetUnknownAppend]
+      : [Op.UnknownAppend, expr as Expressions.GetUnknownAppend];
   }
 
-  return [[trusted ? Op.TrustingAppend : Op.Append, expr]];
+  return [trusted ? Op.TrustingAppend : Op.Append, expr];
 }
 
 export function buildStatement(
@@ -250,14 +248,16 @@ export function buildStatement(
 ): WireFormat.Statement[] {
   switch (normalized.kind) {
     case APPEND_PATH_HEAD: {
-      return buildAppend(normalized.trusted, buildGetPath(normalized.path, symbols));
+      return [buildAppend(normalized.trusted, buildGetPath(normalized.path, symbols))];
     }
 
     case APPEND_EXPR_HEAD: {
-      return buildAppend(
-        normalized.trusted,
-        buildExpression(normalized.expr, normalized.trusted ? 'TrustedAppend' : 'Append', symbols)
-      );
+      return [
+        buildAppend(
+          normalized.trusted,
+          buildExpression(normalized.expr, normalized.trusted ? 'TrustedAppend' : 'Append', symbols)
+        ),
+      ];
     }
 
     case CALL_HEAD: {
@@ -818,20 +818,21 @@ export function invokeType(
 }
 
 export function callType(expr: Expressions.Expression): CallLexicalOpcode | CallResolvedOpcode {
-  if (!Array.isArray(expr)) throw Error('Something is suspicious @fixme');
+  if (!Array.isArray(expr)) {
+    throw Error('Something is suspicious @fixme');
+  }
 
   let type = expr[0];
 
   switch (type) {
-    case Op.GetLexicalSymbol:
-    case Op.GetSymbol:
-      return Op.CallLexical;
     case Op.GetFreeAsHelperHead:
+    case Op.GetFreeAsModifierHead:
+    case Op.GetFreeAsComponentHead:
     case Op.GetFreeAsComponentOrHelperHead:
     case Op.GetStrictKeyword:
       return Op.CallResolved;
 
     default:
-      throw Error(`Something is suspicious (unexpected ${type} opcode in call) @fixme`);
+      return Op.CallLexical;
   }
 }

packages/@glimmer/compiler/lib/passes/2-encoding/content.ts

@@ -17,7 +17,7 @@ import { SexpOpcodes } from '@glimmer/wire-format';
 import type { OptionalList } from '../../shared/list';
 import type * as mir from './mir';
 
-import { buildAppend } from '../../builder/builder';
+import { buildAppend, callType } from '../../builder/builder';
 import { deflateAttrName, deflateTagName } from '../../utils';
 import { EXPR } from './expressions';
 
@@ -119,10 +119,8 @@ export class ContentEncoder {
     return [SexpOpcodes.TrustingAppend, EXPR.expr(html)];
   }
 
-  AppendTextNode({
-    text,
-  }: mir.AppendTextNode): WireFormat.Statements.Append | WireFormat.Statements.UnknownAppend {
-    return buildAppend(false, EXPR.expr(text))[0];
+  AppendTextNode({ text }: mir.AppendTextNode): WireFormat.Statements.SomeAppend {
+    return buildAppend(false, EXPR.expr(text));
   }
 
   AppendComment({ value }: mir.AppendComment): WireFormat.Statements.Comment {
@@ -168,8 +166,16 @@ export class ContentEncoder {
         return [dynamicAttrOp(param.kind), ...dynamicAttr(param)];
       case 'StaticAttr':
         return [staticAttrOp(param.kind), ...staticAttr(param)];
-      case 'Modifier':
-        return [SexpOpcodes.Modifier, EXPR.expr(param.callee), ...EXPR.Args(param.args)];
+      case 'Modifier': {
+        const expr = EXPR.expr(param.callee);
+        return [
+          callType(expr) === SexpOpcodes.CallLexical
+            ? SexpOpcodes.LexicalModifier
+            : SexpOpcodes.ResolvedModifier,
+          EXPR.expr(param.callee),
+          ...EXPR.Args(param.args),
+        ];
+      }
     }
   }
 
@@ -226,10 +232,14 @@ export class ContentEncoder {
     definition,
     args,
     blocks,
-  }: mir.InvokeComponent): WireFormat.Statements.InvokeComponent {
+  }: mir.InvokeComponent): WireFormat.Statements.SomeInvokeComponent {
+    const expr = EXPR.expr(definition);
+
     return [
-      SexpOpcodes.InvokeComponent,
-      EXPR.expr(definition),
+      typeof expr === 'string' || callType(expr) === SexpOpcodes.CallLexical
+        ? SexpOpcodes.InvokeLexicalComponent
+        : SexpOpcodes.InvokeResolvedComponent,
+      expr,
       EXPR.Positional(args.positional),
       EXPR.NamedArguments(args.named),
       blocks ? CONTENT.NamedBlocks(blocks) : null,

packages/@glimmer/compiler/lib/wire-format-debug.ts

@@ -124,9 +124,10 @@ export default class WireFormatDebugger {
         case Op.Comment:
           return ['comment', opcode[1]];
 
-        case Op.Modifier:
+        case Op.LexicalModifier:
+        case Op.ResolvedModifier:
           return [
-            'modifier',
+            opcode[0] === Op.ResolvedModifier ? 'modifier:resolved' : 'modifier',
             this.formatOpcode(opcode[1]),
             this.formatParams(opcode[2]),
             this.formatHash(opcode[3]),
@@ -233,9 +234,10 @@ export default class WireFormatDebugger {
         case Op.GetDynamicVar:
           return ['-get-dynamic-vars', this.formatOpcode(opcode[1])];
 
-        case Op.InvokeComponent:
+        case Op.InvokeLexicalComponent:
+        case Op.InvokeResolvedComponent:
           return [
-            'component',
+            opcode[0] === Op.InvokeLexicalComponent ? 'component' : 'component:resolved',
             this.formatOpcode(opcode[1]),
             this.formatParams(opcode[2]),
             this.formatHash(opcode[3]),

packages/@glimmer/interfaces/lib/compile/wire-format/api.ts

@@ -33,13 +33,15 @@ import type {
   IfInlineOpcode,
   IfOpcode,
   InElementOpcode,
-  InvokeComponentOpcode,
+  InvokeLexicalComponentOpcode,
+  InvokeResolvedComponentOpcode,
   LetOpcode,
+  LexicalModifierOpcode,
   LogOpcode,
-  ModifierOpcode,
   NotOpcode,
   OpenElementOpcode,
   OpenElementWithSplatOpcode,
+  ResolvedModifierOpcode,
   StaticArgOpcode,
   StaticAttrOpcode,
   StaticComponentAttrOpcode,
@@ -220,12 +222,17 @@ export namespace Statements {
   export type Blocks = Core.Blocks;
   export type Path = Core.Path;
 
+  export type SomeAppend = Append | TrustingAppend | UnknownAppend | UnknownTrustingAppend;
+  export type SomeModifier = LexicalModifier | ResolvedModifier;
+  export type SomeInvokeComponent = InvokeLexicalComponent | InvokeResolvedComponent;
+
   export type UnknownAppend = [UnknownAppendOpcode, Expressions.GetUnknownAppend];
   export type UnknownTrustingAppend = [UnknownTrustingAppendOpcode, Expressions.GetUnknownAppend];
   export type Append = [AppendOpcode, Expression];
   export type TrustingAppend = [TrustingAppendOpcode, Expression];
   export type Comment = [CommentOpcode, string];
-  export type Modifier = [ModifierOpcode, Expression, Params, Hash];
+  export type LexicalModifier = [LexicalModifierOpcode, Expression, Params, Hash];
+  export type ResolvedModifier = [ResolvedModifierOpcode, Expression, Params, Hash];
   export type Block = [BlockOpcode, Expression, Params, Hash, Blocks];
   export type Component = [
     op: ComponentOpcode,
@@ -304,8 +311,16 @@ export namespace Statements {
     block: SerializedInlineBlock,
   ];
 
-  export type InvokeComponent = [
-    op: InvokeComponentOpcode,
+  export type InvokeLexicalComponent = [
+    op: InvokeLexicalComponentOpcode,
+    definition: Expression,
+    positional: Core.Params,
+    named: Core.Hash,
+    blocks: Blocks | null,
+  ];
+
+  export type InvokeResolvedComponent = [
+    op: InvokeResolvedComponentOpcode,
     definition: Expression,
     positional: Core.Params,
     named: Core.Hash,
@@ -316,12 +331,9 @@ export namespace Statements {
    * A Handlebars statement
    */
   export type Statement =
-    | Append
-    | TrustingAppend
-    | UnknownAppend
-    | UnknownTrustingAppend
+    | SomeAppend
+    | SomeModifier
     | Comment
-    | Modifier
     | Block
     | Component
     | OpenElement
@@ -339,7 +351,7 @@ export namespace Statements {
     | Each
     | Let
     | WithDynamicVars
-    | InvokeComponent;
+    | SomeInvokeComponent;
 
   export type Attribute =
     | StaticAttr
@@ -349,7 +361,7 @@ export namespace Statements {
     | ComponentAttr
     | TrustingComponentAttr;
 
-  export type ComponentFeature = Modifier | AttrSplat;
+  export type ComponentFeature = SomeModifier | AttrSplat;
   export type Argument = StaticArg | DynamicArg;
 
   export type ElementParameter = Attribute | Argument | ComponentFeature;

packages/@glimmer/interfaces/lib/compile/wire-format/opcodes.d.ts

@@ -4,7 +4,8 @@ export type UnknownAppendOpcode = 2;
 export type UnknownTrustingAppendOpcode = 3;
 export type TrustingAppendOpcode = 4;
 export type CommentOpcode = 5;
-export type ModifierOpcode = 6;
+export type LexicalModifierOpcode = 6;
+export type ResolvedModifierOpcode = 55;
 export type StrictModifierOpcode = 7;
 export type BlockOpcode = 8;
 export type StrictBlockOpcode = 9;
@@ -57,7 +58,8 @@ export type IfOpcode = 41;
 export type EachOpcode = 42;
 export type LetOpcode = 44;
 export type WithDynamicVarsOpcode = 45;
-export type InvokeComponentOpcode = 46;
+export type InvokeLexicalComponentOpcode = 46;
+export type InvokeResolvedComponentOpcode = 47;
 
 // Keyword Expressions
 export type HasBlockOpcode = 48;

packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/resolution.ts

@@ -80,7 +80,10 @@ export function resolveComponent(
   expr: Expressions.Expression,
   then: (component: CompileTimeComponent) => void
 ): void {
-  localAssert(isGetFreeComponent(expr), 'Attempted to resolve a component with incorrect opcode');
+  localAssert(
+    isGetFreeComponent(expr),
+    `Attempted to resolve a component with incorrect opcode (${JSON.stringify(expr)})`
+  );
 
   let type = expr[0];
 

packages/@glimmer/opcode-compiler/lib/syntax/statements.ts

@@ -54,7 +54,7 @@ import {
 } from '../opcode-builder/helpers/components';
 import { Replayable, ReplayableIf, SwitchCases } from '../opcode-builder/helpers/conditional';
 import { expr } from '../opcode-builder/helpers/expr';
-import { isGetFreeComponent, isGetFreeModifier } from '../opcode-builder/helpers/resolution';
+import { isGetFreeComponent } from '../opcode-builder/helpers/resolution';
 import { CompilePositional, SimpleArgs } from '../opcode-builder/helpers/shared';
 import {
   Call,
@@ -91,22 +91,22 @@ STATEMENTS.add(SexpOpcodes.Comment, (op, [, comment]) => Comment(op, comment));
 STATEMENTS.add(SexpOpcodes.CloseElement, (op) => CloseElement(op));
 STATEMENTS.add(SexpOpcodes.FlushElement, (op) => FlushElement(op));
 
-STATEMENTS.add(SexpOpcodes.Modifier, (encode, [, expression, positional, named]) => {
-  if (isGetFreeModifier(expression)) {
-    encode.modifier(expression, (handle: number) => {
-      encode.op(VM_PUSH_FRAME_OP);
-      SimpleArgs(encode, positional, named, false);
-      encode.op(VM_MODIFIER_OP, handle);
-      encode.op(VM_POP_FRAME_OP);
-    });
-  } else {
-    expr(encode, expression);
+STATEMENTS.add(SexpOpcodes.ResolvedModifier, (encode, [, expression, positional, named]) => {
+  encode.modifier(expression, (handle: number) => {
     encode.op(VM_PUSH_FRAME_OP);
     SimpleArgs(encode, positional, named, false);
-    encode.op(VM_DUP_OP, $fp, 1);
-    encode.op(VM_DYNAMIC_MODIFIER_OP);
+    encode.op(VM_MODIFIER_OP, handle);
     encode.op(VM_POP_FRAME_OP);
-  }
+  });
+});
+
+STATEMENTS.add(SexpOpcodes.LexicalModifier, (encode, [, expression, positional, named]) => {
+  expr(encode, expression);
+  encode.op(VM_PUSH_FRAME_OP);
+  SimpleArgs(encode, positional, named, false);
+  encode.op(VM_DUP_OP, $fp, 1);
+  encode.op(VM_DYNAMIC_MODIFIER_OP);
+  encode.op(VM_POP_FRAME_OP);
 });
 
 STATEMENTS.add(SexpOpcodes.StaticAttr, (encode, [, name, value, namespace]) => {
@@ -408,15 +408,21 @@ STATEMENTS.add(SexpOpcodes.WithDynamicVars, (encode, [, named, block]) => {
   }
 });
 
-STATEMENTS.add(SexpOpcodes.InvokeComponent, (encode, [, expr, positional, named, blocks]) => {
-  if (isGetFreeComponent(expr)) {
+STATEMENTS.add(
+  SexpOpcodes.InvokeLexicalComponent,
+  (encode, [, expr, positional, named, blocks]) => {
+    InvokeDynamicComponent(encode, expr, null, positional, named, blocks, false, false);
+  }
+);
+
+STATEMENTS.add(
+  SexpOpcodes.InvokeResolvedComponent,
+  (encode, [, expr, positional, named, blocks]) => {
     encode.component(expr, (component: CompileTimeComponent) => {
       InvokeComponent(encode, component, null, positional, hashToArgs(named), blocks);
     });
-  } else {
-    InvokeDynamicComponent(encode, expr, null, positional, named, blocks, false, false);
   }
-});
+);
 
 function hashToArgs(hash: WireFormat.Core.Hash | null): WireFormat.Core.Hash | null {
   if (hash === null) return null;

packages/@glimmer/wire-format/index.ts

@@ -25,7 +25,8 @@ export function isAttribute(val: Statement): val is Statements.Attribute {
     val[0] === opcodes.StaticComponentAttr ||
     val[0] === opcodes.TrustingComponentAttr ||
     val[0] === opcodes.AttrSplat ||
-    val[0] === opcodes.Modifier
+    val[0] === opcodes.LexicalModifier ||
+    val[0] === opcodes.ResolvedModifier
   );
 }