Skip to content
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

Make ember-auto-import@2 a dependency #1292

Merged
merged 1 commit into from
Dec 16, 2022

Conversation

SergeAstapov
Copy link
Contributor

This is breaking change but would allow us to convert the addon to v2 format in a minor release

Copy link
Contributor

@chriskrycho chriskrycho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💙

@chriskrycho chriskrycho merged commit 1c637d9 into emberjs:master Dec 16, 2022
@SergeAstapov SergeAstapov deleted the eai-v2 branch December 16, 2022 03:37
@bertdeblock
Copy link
Member

I'm unsure if this is correct. AFAIK, it's the consuming app that needs Embroider or ember-auto-import v2 to use a v2 addon. That's what's also mentioned in the v2 addon blueprint:
https://github.com/embroider-build/addon-blueprint/blob/main/files/README.md#compatibility.

@SergeAstapov
Copy link
Contributor Author

SergeAstapov commented Dec 17, 2022

@bertdeblock idea behind this is that v2 addon requires consuming app to either use Embroider or have eai v2, as you said.
Any addon consuming v2 addon or addon with eai v2 also needs to be either v2 adon or have eai v2 (which probably not an issue for this addon).

however, any addon having eai v2 as dependency, also requires the app to have eai v2.
If addon didn’t have it before or used eai v1 - this is breaking change now to require eai v2.

idea of this PR is to make a breaking change now.

and once someone has a bandwidth, now we can convert to v2 addon fromat without breaking change.

@bertdeblock
Copy link
Member

@ember/test-helpers doesn't use any v2 addons and doesn't import any normal npm packages as far as I can tell, so technically it doesn't need ember-auto-import in dependencies in the first place I think?
I guess I don't understand why it's important to be able to do the v2 conversion in a minor release.

@SergeAstapov
Copy link
Contributor Author

SergeAstapov commented Dec 17, 2022

I guess I don't understand why it's important to be able to do the v2 conversion in a minor release.

Only to avoid another major release, no other real reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants