From e1334aebc41ae23058587a48d4f2c85f017194d5 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Wed, 5 Sep 2018 15:44:18 -0400 Subject: [PATCH] Ensure dynamic component invocations follow splattribute rules. When the `DynamicComponent` opcode was initial introduce, it did not properly replicate the `SetComponentAttrs` toggling system that `Ops.Component` has. This has the unfortunate result of making dynamic component invocations not follow the established rules for splattributes (invocation "wins" for all attributes other than `class` and `class` is merged). In this context `DynamicComponent` is specifically referring to angle bracket invocation with either a named argument or path. --- .../@glimmer/opcode-compiler/lib/syntax.ts | 19 +++--- .../test-helpers/lib/suites/components.ts | 58 +++++++++++++++++++ 2 files changed, 69 insertions(+), 8 deletions(-) diff --git a/packages/@glimmer/opcode-compiler/lib/syntax.ts b/packages/@glimmer/opcode-compiler/lib/syntax.ts index 61b1874429..81ff65be75 100644 --- a/packages/@glimmer/opcode-compiler/lib/syntax.ts +++ b/packages/@glimmer/opcode-compiler/lib/syntax.ts @@ -106,15 +106,18 @@ export function statementCompiler(): Compilers { STATEMENTS.add(Ops.DynamicComponent, (sexp: S.DynamicComponent, builder) => { let [, definition, attrs, args, template] = sexp; - let block = builder.template(template); - let attrsBlock = - attrs.length > 0 - ? builder.inlineBlock({ - statements: attrs, - parameters: EMPTY_ARRAY, - }) - : null; + + let attrsBlock = null; + if (attrs.length > 0) { + let wrappedAttrs: WireFormat.Statement[] = [ + [Ops.ClientSideStatement, ClientSide.Ops.SetComponentAttrs, true], + ...attrs, + [Ops.ClientSideStatement, ClientSide.Ops.SetComponentAttrs, false], + ]; + + attrsBlock = builder.inlineBlock({ statements: wrappedAttrs, parameters: EMPTY_ARRAY }); + } builder.dynamicComponent(definition, attrsBlock, null, args, false, block, null); }); diff --git a/packages/@glimmer/test-helpers/lib/suites/components.ts b/packages/@glimmer/test-helpers/lib/suites/components.ts index 7db5fe8f7a..09148b4ef2 100644 --- a/packages/@glimmer/test-helpers/lib/suites/components.ts +++ b/packages/@glimmer/test-helpers/lib/suites/components.ts @@ -198,6 +198,64 @@ export class BasicComponents extends RenderTest { this.assertStableRerender(); } + @test({ + kind: 'glimmer', + }) + 'invoking curried component with attributes via angle brackets (invocation attributes clobber)'() { + this.registerHelper('hash', (_positional, named) => named); + this.registerComponent( + 'Glimmer', + 'Foo', + '

hello world!

' + ); + this.render({ + layout: '<@stuff.Foo data-foo="invocation" />', + args: { + stuff: 'hash Foo=(component "Foo")', + }, + }); + + this.assertHTML(`

hello world!

`); + this.assertStableRerender(); + } + + @test({ + kind: 'glimmer', + }) + 'invoking curried component with attributes via angle brackets (invocation classes merge)'() { + this.registerHelper('hash', (_positional, named) => named); + this.registerComponent('Glimmer', 'Foo', '

hello world!

'); + this.render({ + layout: '<@stuff.Foo class="invocation" />', + args: { + stuff: 'hash Foo=(component "Foo")', + }, + }); + + this.assertHTML(`

hello world!

`); + this.assertStableRerender(); + } + + @test({ + kind: 'glimmer', + }) + 'invoking dynamic component (named arg) via angle brackets supports attributes (invocation attributes clobber)'() { + this.registerComponent( + 'Glimmer', + 'Foo', + '
hello world!
' + ); + this.render({ + layout: '<@foo data-test="foo"/>', + args: { + foo: 'component "Foo"', + }, + }); + + this.assertHTML(`
hello world!
`); + this.assertStableRerender(); + } + @test({ kind: 'glimmer', })