search

kybishop / ember-popper

Popper.js for Ember
MIT License
39 stars 35 forks source link

Update ember-ref-modifier to 1.0 #117

Closed simonihmig closed 4 years ago

simonihmig commented 4 years ago

Supersedes #114

simonihmig commented 4 years ago

@jrjohnson I used your previous work here, and got (most) failing tests fixed. The issue seemed to be that ref is setting asynchronously (see https://github.com/lifeart/ember-ref-modifier/blob/master/addon/modifiers/ref.js#L72), so a few settled() calls in tests were able to fix this.

However tests for Ember 2.18 and 3.4 are still failing. Seems ember-ref-modifier does not support these, and also seems you know about it! 😝https://github.com/lifeart/ember-ref-modifier/pull/199

Any specific reasons to not support the older versions anymore? Not sure we can drop support here already!? 🤔

jrjohnson commented 4 years ago

Nice! Passing tests always better.

If I'm understanding the list correctly then 3.12 is the only currently supported LTS. Anyone still using an older version of ember can just stay on an earlier version of this addon.

ref-modifier probably runs on 3.12 just fine, but it can't be tested against it because there is no polyfill for setComponentTemplate so the glimmer component tests in the addon fail. That's why I bumped the min version to 3.13. No tests / no guarantees.

simonihmig commented 4 years ago

ref-modifier probably runs on 3.12 just fine

Indeed, also tests for 3.8 were passing. Thanks for the explanation! I was a bit hesitant to drop support for 3.12, as we should have at least the latest LTS supported. But as it seems to work fine, we can keep it. Here's a PR for dropping the older versions: #119