alexlafroscia / ember-cli-stencil

Automatic discovery of Stencil.js components for your Ember application
26 stars 7 forks source link

Compatibility with ember-cli 3.4 #7

Closed xcambar closed 4 years ago

xcambar commented 5 years ago

This PR removes the usage of AddonDiscovery in favour of the PackageInfo, introduced in Ember-cli 3.4

alexlafroscia commented 5 years ago

Hey, thanks for the fix!

It seems like the tests should be passing on master but the PR is failing. Can you investigate?

alexlafroscia commented 5 years ago

Since it seems like it could be flakey tests, I don't necessarily want to block this PR from being merged until that's solved... but I would like some assurance that the behavior actually does work both pre- and post-3.4.

Do you have any ideas on how we can do that? AFAIK ember-try does not allow swapping the ember-cli version but my information might be out-of-date.

alexlafroscia commented 5 years ago

Hoping that #10 fixes the issues with the flakey test

TRMW commented 4 years ago

Has anyone had a chance to investigate this further? It'd be great to be able to use this addon with newer versions of Ember!

alexlafroscia commented 4 years ago

I’m planning to check this out soon!

Likely what I’ll do is both get it working with the latest ember-cli and the latest Stencil compiler, since I’m sure there’s been changes on that end, too. At that point I’ll cut a new major version bump.

I’m not actively using this addon these days, but am between jobs and would love to devote some time to open source in the interim! I plan to check this out sometime in the next week or so. Thanks for the bump!

On Aug 6, 2019, at 5:20 PM, Matt Wright notifications@github.com wrote:

 Has anyone had a chance to investigate this further? It'd be great to be able to use this addon with newer versions of Ember!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

alexlafroscia commented 4 years ago

Okay, so I was able to determine why the tests were flakey and fixed that up. Rebasing this PR to get the tests passing again...

alexlafroscia commented 4 years ago

This PR doesn't have tests for the new behavior, but I'm going to update the add-on itself to use the latest version of the CLI and, hopefully, can validate that all still works with the changes that way. Thanks for your patience here!

TRMW commented 4 years ago

Thank you for getting this merged! Looking forward to the new version!

alexlafroscia commented 4 years ago

This was just released to npm as part of version 1.0.0 🎉

Note: I removed the Ember component wrapper behavior in favor of just using them as "real" WebComponents, as more modern versions of Ember support WCs well. I also removed the custom action stuff in favor of the new {{on}} element modifier, since that reduced a lot of what this add-on needs to do!

I also made the add-on support @stencil/core@1.0.X so, if you're using an older version of Stencil, I recommend upgrading to the latest. The change to my test components was really small but the "correct" means to import them changed a bit in the official 1.X release.

TRMW commented 4 years ago

Sorry for the delayed response, but thanks so much for this update! We're using it in our Ember app and things seem to Just Work. Thank you!

alexlafroscia commented 4 years ago

Not a problem! I love to hear that it's helping you out!