html-next / ember-hammertime

TouchAction (aka "fastclick") Support for Ember Applications
MIT License
56 stars 23 forks source link

You cannot use `attributeBindings` on a tag-less component #19

Closed thagul closed 8 years ago

thagul commented 8 years ago

I've just migrated my application to ember.js 2.8 and it started throwing error in the console.

Uncaught Error: Assertion Failed: You cannot use attributeBindings on a tag-less component: <yyyyyy@component:xxxxx::ember382>

Due to this error, the app doesn't load and shows an empty page.

I've created a small demo app with just ember-hammertime and a tagless component and it still throws the same error: https://github.com/thagul/hammertime-2-8 To verify just run the app and check the console.

runspired commented 8 years ago

@chancancode I think this is something you need to know about. The TL;DR here is that ember-hammertime reopens Component to add this binding here: https://github.com/runspired/ember-hammertime/blob/develop/addon/mixins/touch-action.js#L31

I'll update this to no-op on a tagless component, but is there a better pattern for this? For hammertime to properly work it needs to be able to do this for all components that define click and have a valid tagName.

ef4 commented 8 years ago

I think your existing AST transform is on the right track. This could be a compile time thing only, and for the rarer case where people want to dynamically change their touch action styling, you can give them an explicit mixin.

Once we have glimmer components and element modifier, you could just handle all the dynamic cases with an element modifier that goes right in the template.

runspired commented 8 years ago

Resolved by the release of 1.1.0

thagul commented 8 years ago

I'm still having the same error with latest v1.1.1 release.

I've updated the demo app at https://github.com/thagul/hammertime-2-8 with the latest version of ember-hammertime if it can help. Thanks

eriktrom commented 8 years ago

confirmed - regression - for reference - the functionality just went through 3 iterations

  1. https://github.com/runspired/ember-hammertime/commit/d061986d20c2e1bc34aca3c7a4373a25ff095067
  2. https://github.com/runspired/ember-hammertime/commit/cded5afcbc8bab7a0d8ca5d533eda73e8a0789bf
  3. https://github.com/runspired/ember-hammertime/commit/6275bec1678a5054b10dc4b812427bb080838b04

Haven't read the last, the 2nd tries to not replace attributeBindings, but given the file is a mixin and concatenated properties are meant to extend their parent class's array at each level of inheritance that shouldn't have been an issue (but maybe was, dunno)

Likely then its #3 that may be breaking it, but anyone is free to step in in the meantime (hopefully the info above helps a bit)

runspired commented 8 years ago

If either of you have time to dig in I can get a patch release out quick, I won't have time to do any investigating for a few days.

RobbieTheWagner commented 8 years ago

@runspired would you mind doing a patch release with this fix please? 😄

runspired commented 8 years ago

@rwwagner90 one was, it didnt fix it, we're close to having a fix in place that does.

tavosansal commented 8 years ago

Is there a fix planned for this?