search

adopted-ember-addons / ember-metrics

Send data to multiple analytics integrations without re-implementing new API
MIT License
367 stars 160 forks source link

Support Ember v5 by addin @ember/string to peerDependencies #547

Closed jelhan closed 4 days ago

jelhan commented 2 weeks ago

This is a breaking change as it is adding @ember/string as an additional peer dependency.

It included #550 which should be merged first.

Windvis commented 2 weeks ago

This is a breaking change as it is adding @ember/string as an additional peer dependency.

It's just a devDep at the moment, no? Also, @ember/string is a v2 addon so ember-auto-import v2 needs to be a dep I think?

jelhan commented 2 weeks ago

This is a breaking change as it is adding @ember/string as an additional peer dependency.

It's just a devDep at the moment, no? Also, @ember/string is a v2 addon so ember-auto-import v2 needs to be a dep I think?

You are right. It should have gone into peer dependencies. But somehow I missed it. No idea why the tests are passing anyways here but no in my app. Thanks a lot for careful review! Will fix it.

jelhan commented 1 week ago

I learned that there is a 2.0.0-beta.2 release, which supports Ember v5. The code is not in this repository. But it is available on NPM.

jelhan commented 1 week ago

The code for 2.0.0-beta.2 is in this repository. This is the tag.

Also 2.0.0-beta.2 is not compatible with Ember v5. It fails due to missing peer dependency on @ember/string.

jelhan commented 1 week ago

It should work. Also the added test scenario for Ember v5.12 passes. But I haven't tested in a real app yet.

jelhan commented 4 days ago

It seems that some outdated dependencies weren't compatible with Ember v5 causing an error. After I merged in #550 it works fine. So let's land that upgrade first and than add the support for Ember v5 in the next step.

SergeAstapov commented 4 days ago

@jelhan mind to rebase this PR as https://github.com/adopted-ember-addons/ember-metrics/pull/550 landed