From 484e81ea575cdb4e17ec1045c47273dabc70ae74 Mon Sep 17 00:00:00 2001 From: Chad Hietala Date: Tue, 7 Jun 2016 16:33:40 -0700 Subject: [PATCH] [GLIMMER] Refactor attributebinding application We need to apply attribute bindings in reverse to account for concated properties. --- .../lib/syntax/curly-component.js | 25 ++++++++++++++++--- .../ember-glimmer/lib/utils/references.js | 20 +++++++++++---- .../components/attribute-bindings-test.js | 2 +- 3 files changed, 38 insertions(+), 9 deletions(-) diff --git a/packages/ember-glimmer/lib/syntax/curly-component.js b/packages/ember-glimmer/lib/syntax/curly-component.js index 8600eb14066..057a37f34de 100644 --- a/packages/ember-glimmer/lib/syntax/curly-component.js +++ b/packages/ember-glimmer/lib/syntax/curly-component.js @@ -16,6 +16,27 @@ function aliasIdToElementId(args, props) { } } +// We must traverse the attributeBindings in reverse keeping track of +// what has already been applied. This is essentially refining the concated +// properties applying right to left. +function applyAttributeBindings(attributeBindings, component, operations) { + let seen = []; + let i = attributeBindings.length - 1; + + while (i !== -1) { + let binding = attributeBindings[i]; + let parsedMicroSyntax = AttributeBindingReference.parseMicroSyntax(binding); + let [ prop ] = parsedMicroSyntax; + + if (seen.indexOf(prop) === -1) { + seen.push(prop); + AttributeBindingReference.apply(component, parsedMicroSyntax, operations); + } + + i--; + } +} + export class CurlyComponentSyntax extends StatementSyntax { constructor({ args, definition, templates }) { super(); @@ -138,9 +159,7 @@ class CurlyComponentManager { let { attributeBindings, classNames, classNameBindings } = component; if (attributeBindings) { - attributeBindings.forEach(binding => { - AttributeBindingReference.apply(component, binding, operations); - }); + applyAttributeBindings(attributeBindings, component, operations); } if (classRef) { diff --git a/packages/ember-glimmer/lib/utils/references.js b/packages/ember-glimmer/lib/utils/references.js index 7ffab90a400..5b54d9f2201 100644 --- a/packages/ember-glimmer/lib/utils/references.js +++ b/packages/ember-glimmer/lib/utils/references.js @@ -237,24 +237,34 @@ export class InternalHelperReference extends CachedReference { import { assert } from 'ember-metal/debug'; export class AttributeBindingReference extends CachedReference { - static apply(component, microsyntax, operations) { - let reference = this.parse(component, microsyntax); + static apply(component, parsedMicroSyntax, operations) { + let reference; + let prop = parsedMicroSyntax[0]; + let attr; + + if (parsedMicroSyntax.length === 1) { + reference = new this(component, prop); + } else { + attr = parsedMicroSyntax[1]; + reference = new this(component, prop, attr); + } + operations.addAttribute(reference.attributeName, reference); } - static parse(component, microsyntax) { + static parseMicroSyntax(microsyntax) { let colonIndex = microsyntax.indexOf(':'); if (colonIndex === -1) { assert('You cannot use class as an attributeBinding, use classNameBindings instead.', microsyntax !== 'class'); - return new this(component, microsyntax); + return [microsyntax]; } else { let prop = microsyntax.substring(0, colonIndex); let attr = microsyntax.substring(colonIndex + 1); assert('You cannot use class as an attributeBinding, use classNameBindings instead.', attr !== 'class'); - return new this(component, prop, attr); + return [prop, attr]; } } diff --git a/packages/ember-glimmer/tests/integration/components/attribute-bindings-test.js b/packages/ember-glimmer/tests/integration/components/attribute-bindings-test.js index 6c6ed0232a1..4f8f19008fc 100644 --- a/packages/ember-glimmer/tests/integration/components/attribute-bindings-test.js +++ b/packages/ember-glimmer/tests/integration/components/attribute-bindings-test.js @@ -315,7 +315,7 @@ moduleFor('Attribute bindings integration', class extends RenderingTest { this.assertComponentElement(this.nthChild(0), { tagName: 'div', attrs: { 'id': 'special-sauce' } }); } - ['@htmlbars attributeBindings are overwritten']() { + ['@test attributeBindings are overwritten']() { let FooBarComponent = Component.extend({ attributeBindings: ['href'], href: 'a href'