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

Audit peerDependencies #899

Closed
gilest opened this issue Jan 21, 2023 · 4 comments · Fixed by #917
Closed

Audit peerDependencies #899

gilest opened this issue Jan 21, 2023 · 4 comments · Fixed by #917
Labels

Comments

@gilest
Copy link
Collaborator

gilest commented Jan 21, 2023

Pulled out of #894 where devDependencies have been added to @babel/runtime and @ember/string.

As suggested by @jelhan

... may need to go into peerDependency to avoid a deprecation in Ember 4.10+. Adding as peer dependency would be a breaking change. Maybe better done in separate PR.

When doing a new major for that, we should also revisit other dependencies and see if they should be peer dependencies instead. E.g. @glimmer/tracking and @glimmer/component.

@jelhan
Copy link
Collaborator

jelhan commented Jan 22, 2023

My comment was only meant for @ember/string. I don't know enough about @babel/runtime to understand how that one should be handled.

@jelhan
Copy link
Collaborator

jelhan commented Jan 22, 2023

I'm not an expert on that topic but we should investigate if the following dependencies and dev dependencies should go into peer dependencies:

  • @ember/string
  • @ember/test-helpers
  • @ember/test-waiters
  • @glimmer/component
  • @glimmer/tracking
  • ember-modifier
  • ember-source
  • tracked-built-ins

Most of them are already part of the apps blueprint. For ember-modifier and tracked-built-ins RFCs exists to add them.

@gilest
Copy link
Collaborator Author

gilest commented Jan 26, 2023

Regarding the ember-cli-babel v8 upgrade notes:

Upgrade Path for Addons
Since (v1) addons bring in their own version of ember-cli-babel, they should now also bring in their own version of @babel/core. This means that, addons should add @babel/core under dependencies in their package.json file. This makes the dependency on @babel/core more explicit while also avoiding addons having to cut a breaking release to update ember-cli-babel to v8.

@bertdeblock can we safely ignore this in a v2 addon?

@bertdeblock
Copy link
Contributor

Indeed, v2 addons don't use ember-cli-babel if I'm not mistaken, so those notes are only relevant for v1 addons.

gilest added a commit that referenced this issue Apr 7, 2023
It's a required peer dependency since ember-source 4.10.0. The deprecation is now a removal in canary v5 which is causing a CI failure.

Same change would have been made in the ember-cil 4.12 blueprint but hoping to get the CI green with a minimal change first.

Related to #899 as I'm not sure we need to declare a peerDependency from the addon itself. Reason for this is don't think the addon uses @ember/string.
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 a pull request may close this issue.

3 participants