html-next / ember-hammertime

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

Update Ember and fix AST issues #40

Closed RobbieTheWagner closed 7 years ago

RobbieTheWagner commented 7 years ago

This should fix #38

RobbieTheWagner commented 7 years ago

@runspired @eriktrom @kybishop something was broken with .bind, so I had to store config values myself. There is probably a better way to do this, if either of you know of something. We could also use something like https://github.com/null-null-null/ember-get-config to grab the config values instead. Please let me know your thoughts and preference 😄

eriktrom commented 7 years ago

This is fine by me - I'm curious though - the code, besides the updates, and the refactors, has the same logic.

Except the bind - is it possible that upstream there is a bug with ember-cli where setupPreprocessorRegistry is being called twice, where registry.add is borking things up?

I read through the code, and based on your comment and my close reading, I still don't get what exactly broke, and why?

at any rate, if it solves the problem LGTM

RobbieTheWagner commented 7 years ago

@eriktrom I searched all of GitHub for htmlbars-ast-plugin and was unable to find anyone else passing a config in. I believe whatever scope it binds to by default was messed up by the bind and caused it to have the incorrect context when doing whatever it was doing with visitors. I would love to more fully understand what broke, but I have no idea where to find the relevant code in Ember itself or how to begin tracking it down.

eriktrom commented 7 years ago

thanks @rwwagner90 for the update - i may have searched around too had u not mentioned u tried

in the end, I like the way its written better now, nice work - could argue that .bind(Ctor, config) was a bit too magical for most anyway

RobbieTheWagner commented 7 years ago

@eriktrom I will go ahead and merge then, and if @runspired or @kybishop have any better ideas, we can always change it later.