Fix class merging and attribute precedence

lib/ast-transform.js

@@ -180,6 +180,48 @@ class AngleBracketPolyfill {
       }
     }
 
+    function attrsAsHash(attrs) {
+      if (attrs.length > 0) {
+        return b.sexpr(
+          'hash',
+          [],
+          b.hash(
+            attrs.map(attr => b.pair(attr.name, expressionForAttributeValue(attr.value), attr.loc))
+          )
+        );
+      }
+    }
+
+    function angleAttrsExpression(attributes) {
+      let preSplatAttributes = [];
+      let postSplatAttributes = [];
+      let foundSplat = false;
+
+      for (let i = 0; i < attributes.length; i++) {
+        let attr = attributes[i];
+        if (attr.name === '...attributes') {
+          foundSplat = true;
+        } else {
+          if (foundSplat) {
+            postSplatAttributes.push(attr);
+          } else {
+            preSplatAttributes.push(attr);
+          }
+        }
+      }
+
+      let attrGroups = [
+        attrsAsHash(preSplatAttributes),
+        foundSplat && b.path('__ANGLE_ATTRS__'),
+        attrsAsHash(postSplatAttributes),
+      ].filter(Boolean);
+      if (attrGroups.length > 1) {
+        return b.sexpr('-merge-attrs', attrGroups);
+      } else {
+        return attrGroups[0];
+      }
+    }
+
     let locals = [];
 
     let visitor = {
@@ -199,18 +241,16 @@ class AngleBracketPolyfill {
 
         if (invocation.kind === 'Element') {
           if (invocation.hasAttrSplat) {
-            node.attributes = node.attributes.filter(n => n.name !== '...attributes');
-            node.modifiers.push(b.elementModifier('_splattributes', [b.path('__ANGLE_ATTRS__')]));
+            let angle = angleAttrsExpression(node.attributes);
+            node.attributes = [];
+            node.modifiers.push(b.elementModifier('_splattributes', [angle]));
           }
-
           return;
         }
 
         let { children, blockParams } = node;
 
-        let attributes = node.attributes.filter(
-          node => node.name[0] !== '@' && node.name !== '...attributes'
-        );
+        let attributes = node.attributes.filter(node => node.name[0] !== '@');
         let args = node.attributes.filter(
           node => node.name[0] === '@' && node.name !== '...attributes'
         );
@@ -221,32 +261,8 @@ class AngleBracketPolyfill {
           )
         );
 
-        if (invocation.hasAttrSplat || attributes.length > 0) {
-          let attributesHash = b.sexpr(
-            '-merge-refs',
-            [],
-            b.hash(
-              [
-                attributes.length > 0 &&
-                  b.pair(
-                    'invocation',
-                    b.sexpr(
-                      'hash',
-                      [],
-                      b.hash(
-                        attributes.map(attr =>
-                          b.pair(attr.name, expressionForAttributeValue(attr.value), attr.loc)
-                        )
-                      )
-                    )
-                  ),
-
-                invocation.hasAttrSplat && b.pair('splat', b.path('__ANGLE_ATTRS__')),
-              ].filter(Boolean)
-            )
-          );
-
-          hash.pairs.push(b.pair('__ANGLE_ATTRS__', attributesHash));
+        if (attributes.length > 0) {
+          hash.pairs.push(b.pair('__ANGLE_ATTRS__', angleAttrsExpression(attributes)));
         }
 
         if (invocation.kind === 'StaticComponent') {

tests/integration/components/angle-bracket-invocation-test.js

@@ -1,11 +1,12 @@
 import { module, test } from 'qunit';
 import { setupRenderingTest } from 'ember-qunit';
-import { render } from '@ember/test-helpers';
+import { render, settled } from '@ember/test-helpers';
 import hbs from 'htmlbars-inline-precompile';
 
 import Service, { inject as injectService } from '@ember/service';
 import { helper as buildHelper } from '@ember/component/helper';
 import Component from '@ember/component';
+import hasEmberVersion from 'ember-test-helpers/has-ember-version';
 
 module('Integration | Component | angle-bracket-invocation', function(hooks) {
   setupRenderingTest(hooks);
@@ -340,6 +341,69 @@ module('Integration | Component | angle-bracket-invocation', function(hooks) {
       assert.dom('span[data-test-my-thing]').hasText('hi martin!');
     });
 
+    test('merge class names on element', async function(assert) {
+      this.owner.register(
+        'template:components/foo-bar',
+        hbs`<span class={{@inside}} ...attributes></span>`
+      );
+
+      this.outside = 'new';
+      this.inside = 'original';
+      await render(hbs`<FooBar @inside={{this.inside}} class={{this.outside}} />`);
+
+      assert.dom('span').hasClass('original');
+      assert.dom('span').hasClass('new');
+
+      this.set('outside', undefined);
+      await settled();
+      assert.dom('span').hasClass('original');
+      assert.dom('span').doesNotHaveClass('new');
+
+      this.set('outside', 'OUT');
+      this.set('inside', undefined);
+      await settled();
+      assert.dom('span').hasClass('OUT');
+      assert.dom('span').doesNotHaveClass('new');
+      assert.dom('span').doesNotHaveClass('original');
+
+      this.set('inside', 'IN');
+      await settled();
+      assert.dom('span').hasClass('OUT');
+      assert.dom('span').hasClass('IN');
+    });
+
+    test('merge class names on component', async function(assert) {
+      this.owner.register('template:components/span', hbs`<span ...attributes></span>`);
+      this.owner.register(
+        'template:components/foo-bar',
+        hbs`<Span class={{@inside}} ...attributes></Span>`
+      );
+
+      this.outside = 'new';
+      this.inside = 'original';
+      await render(hbs`<FooBar @inside={{this.inside}} class={{this.outside}} />`);
+
+      assert.dom('span').hasClass('original');
+      assert.dom('span').hasClass('new');
+
+      this.set('outside', undefined);
+      await settled();
+      assert.dom('span').hasClass('original');
+      assert.dom('span').doesNotHaveClass('new');
+
+      this.set('outside', 'OUT');
+      this.set('inside', undefined);
+      await settled();
+      assert.dom('span').hasClass('OUT');
+      assert.dom('span').doesNotHaveClass('new');
+      assert.dom('span').doesNotHaveClass('original');
+
+      this.set('inside', 'IN');
+      await settled();
+      assert.dom('span').hasClass('OUT');
+      assert.dom('span').hasClass('IN');
+    });
+
     test('passing into element - unused', async function(assert) {
       this.owner.register(
         'template:components/foo-bar',
@@ -350,5 +414,32 @@ module('Integration | Component | angle-bracket-invocation', function(hooks) {
 
       assert.dom('span').hasText('hi martin!');
     });
+
+    test('passing into element - unused with attributes present', async function(assert) {
+      this.owner.register(
+        'template:components/foo-bar',
+        hbs`<span class="hello" ...attributes>hi martin!</span>`
+      );
+
+      await render(hbs`<FooBar />`);
+
+      assert.dom('span').hasText('hi martin!');
+    });
+
+    // This is broken in actual Ember, see
+    // https://github.com/emberjs/ember.js/pull/17533. So we only test in
+    // versions where our polyfill is active.
+    if (!hasEmberVersion(3, 4)) {
+      test('merges attributes in correct priority', async function(assert) {
+        this.owner.register(
+          'template:components/foo-bar',
+          hbs`<span data-left="left-inner" ...attributes data-right="right-inner"></span>`
+        );
+        await render(hbs`<FooBar data-left="left-outer" data-right="right-outer" />`);
+
+        assert.dom('span').hasAttribute('data-left', 'left-outer');
+        assert.dom('span').hasAttribute('data-right', 'right-inner');
+      });
+    }
   });
 });