-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Convert to V2 addon #166
Convert to V2 addon #166
Conversation
We are running in this issue: embroider-build/ember-auto-import#588 (instanceof is not working correctly in tests) An alternative is to hold class in an global context since this bug is fixed... let me know if this workaround is okay for now, or if e need to remove it |
We have a Atm i have removed it... let me know if we need this... |
@esbanarango @RobbieTheWagner CI is now green 🚀... please read my previous comments and let me know whats the plan |
@mkszepp thanks for your work on this! Can you elaborate on if there are actual breaking changes in this? If we think the initializer was not needed, then is this actually breaking? Are we dropping node or Ember versions or changing anything major here? I want to make sure we label the PR correctly. |
@RobbieTheWagner by merging this changes we are breaking, because we have dropped the app Nodejs version is not anymore necessary to document, because v2 addons are statically... for this reason its also not braking. Ember Version support: |
We are also losing the ability blueprints with this PR. I recall reading that blueprints aren't even compatible with V2 addons, but maybe something to mention in the release |
Ups... blueprint we ca readd... the convert script has maybe not implemented to copy into right place.. |
blueprints were readded... they are working like before... no braking changes |
blueprints are now in TS... when consumer app has |
one thing what we should do (let me know if we want do it here or later in an other PR)...
So i think, we should make blueprint changes braking and allow only ember-can/blueprints/ability-test/index.js Lines 6 to 21 in d075697
|
@mkszepp I agree we should only support |
@RobbieTheWagner no its braking, because we have removed |
But I thought you said the initializer was doing nothing? If it was not needed before, I wouldn't consider that breaking. |
Yes it looks so... |
Okay, I marked as enhancement and merged. Thanks for the PR! |
Now, as we have a modern app, its time to move to an addon V2 (RFC)
Changes:
instanceof
check (caused by ember-auto-import bug see below)When this changes are landed we should do:
computed.js
(after that, the addon is typed)close #159