From 4b1265082ab62f1c0f401214886d8daabbcd3f61 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Mon, 4 Jun 2018 15:31:16 -0400 Subject: [PATCH] Fixup issues with dynamic angle bracket invocation. This fixes two different issues identified with dynamic invocation via angle brackets: * The Angle Bracket Invocation RFC specifically forbids implicit `this` lookups from being considered. Prior to the changes here `` would have worked the same as ``. These changes bring the implementation in line with the RFC. * The prior implementation did not support using a path off of a local, these changes fix that. --- .../compiler/lib/template-compiler.ts | 10 ++-- .../test-helpers/lib/suites/components.ts | 60 +++++++++++++++++++ 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/packages/@glimmer/compiler/lib/template-compiler.ts b/packages/@glimmer/compiler/lib/template-compiler.ts index 6a933b72eb..fec1fbc1c7 100644 --- a/packages/@glimmer/compiler/lib/template-compiler.ts +++ b/packages/@glimmer/compiler/lib/template-compiler.ts @@ -464,19 +464,21 @@ function isArg(path: AST.PathExpression): boolean { function isDynamicComponent(element: AST.ElementNode): boolean { let open = element.tag.charAt(0); + let [maybeLocal] = element.tag.split('.'); let isNamedArgument = open === '@'; - let isLocal = element['symbols'].has(element.tag); - let isPath = element.tag.indexOf('.') > -1; + let isLocal = element['symbols'].has(maybeLocal); + let isThisPath = element.tag.indexOf('this.') === 0; - return isLocal || isNamedArgument || isPath; + return isLocal || isNamedArgument || isThisPath; } function isComponent(element: AST.ElementNode): boolean { let open = element.tag.charAt(0); + let isPath = element.tag.indexOf('.') > -1; let isUpperCase = open === open.toUpperCase() && open !== open.toLowerCase(); - return isUpperCase || isDynamicComponent(element); + return (isUpperCase && !isPath) || isDynamicComponent(element); } function assertIsSimplePath(path: AST.PathExpression, loc: AST.SourceLocation, context: string) { diff --git a/packages/@glimmer/test-helpers/lib/suites/components.ts b/packages/@glimmer/test-helpers/lib/suites/components.ts index e1bb039bb3..7db5fe8f7a 100644 --- a/packages/@glimmer/test-helpers/lib/suites/components.ts +++ b/packages/@glimmer/test-helpers/lib/suites/components.ts @@ -181,6 +181,23 @@ export class BasicComponents extends RenderTest { this.assertStableRerender(); } + @test({ + kind: 'glimmer', + }) + 'invoking dynamic component (named arg path) via angle brackets'() { + this.registerHelper('hash', (_positional, named) => named); + this.registerComponent('Glimmer', 'Foo', 'hello world!'); + this.render({ + layout: '<@stuff.Foo />', + args: { + stuff: 'hash Foo=(component "Foo")', + }, + }); + + this.assertHTML(`
hello world!
`); + this.assertStableRerender(); + } + @test({ kind: 'glimmer', }) @@ -297,6 +314,18 @@ export class BasicComponents extends RenderTest { this.assertStableRerender(); } + @test({ + kind: 'glimmer', + }) + 'invoking dynamic component (local path) via angle brackets'() { + this.registerHelper('hash', (_positional, named) => named); + this.registerComponent('Glimmer', 'Foo', 'hello world!'); + this.render(`{{#with (hash Foo=(component 'Foo')) as |Other|}}{{/with}}`); + + this.assertHTML(`hello world!`); + this.assertStableRerender(); + } + @test({ kind: 'glimmer', }) @@ -412,6 +441,37 @@ export class BasicComponents extends RenderTest { this.assertStableRerender(); } + @test({ + kind: 'glimmer', + }) + 'invoking dynamic component (path) via angle brackets does not support implicit `this` fallback'() { + class TestHarness extends EmberishGlimmerComponent { + public stuff: any; + + constructor(args: any) { + super(); + this.stuff = { + Foo: args.attrs.Foo, + }; + } + } + this.registerComponent('Glimmer', 'TestHarness', '', TestHarness); + this.registerComponent( + 'Glimmer', + 'Foo', + 'hello world!', + class extends EmberishGlimmerComponent { + constructor() { + super(...arguments); + throw new Error('Should not have instantiated Foo component.'); + } + } + ); + + this.render(''); + this.assertStableRerender(); + } + @test({ kind: 'glimmer', })