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

Ember: Upgrade Embroider dependencies #4128

Merged

Conversation

alexlafroscia
Copy link
Contributor

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

  • If you've added code that should be tested, please add tests.
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

I've been trying to hunt down old versions of Embroider in my application's dependencies and get them upgraded to the last versions possible.

Two old versions were brought in by @sentry/ember

  • 0.4.x was brought in by the very specific, old version of ember-auto-import that the package depends on. It is only able to depend on 1.6.x releases, which date back to June of 2020
  • 0.37.0 was depended on specifically, but the latest is 0.47.1. Because these are pre-major releases, they will not naturally fluctuate to a newer version

This PR updates both of these dependencies to the latest versions possible without making a breaking change.

ember-auto-import version 2 is available, but upgrading to depend on that would be a breaking change to @sentry/ember as it requires that the host app also depends on ember-auto-import version 2. I don't know how you all schedule breaking changes given that all of your packages use the same version numbers, so I decided not to upgrade all the way to that.

@alexlafroscia
Copy link
Contributor Author

alexlafroscia commented Nov 5, 2021

Ah, I see that #4108 also attempts this!

I added a commit with --ignore-engines as mentioned here to see if that can resolve the issue. It seems sort of dangerous, but maybe will be "just fine"?

Copy link
Member

@k-fish k-fish left a comment

Choose a reason for hiding this comment

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

@alexlafroscia thanks for updating the other deps as well! Yeah, --ignore-engines should be fine for the interim, all the tests pass and the addon functions, I believe it should be okay since we're using a lower version of node and not a higher one.

@AbhiPrasad
Copy link
Member

Mind fixing the yarn.lock conflicts @alexlafroscia? (I ran https://github.com/atlassian/yarn-deduplicate 😅)

Will merge after that. Thanks!

@alexlafroscia
Copy link
Contributor Author

Rebased to fix the conflicts 👍 thanks for the reminder @AbhiPrasad!

@alexlafroscia
Copy link
Contributor Author

alexlafroscia commented Nov 11, 2021

@AbhiPrasad As of the latest commit here, there are @babel packages that could be de-duplicated if you want to keep things clean (I know I always prefer that, personally). Do you want me to include that in this PR?

@AbhiPrasad
Copy link
Member

As of the latest commit here, there are @babel packages that could be de-duplicated if you want to keep things clean (I know I always prefer that, personally). Do you want me to include that in this PR?

@alexlafroscia Don't worry about that, we can take care of it afterwards

You'll have to rebase again because we just cut a release, sorry for the trouble.

@alexlafroscia
Copy link
Contributor Author

No problem!

Rebased again 👍

Embroider also released a newer version yesterday, so I upgraded to that while rebasing

@AbhiPrasad AbhiPrasad enabled auto-merge (squash) November 12, 2021 18:25
@AbhiPrasad
Copy link
Member

AbhiPrasad commented Nov 12, 2021

For ember-auto-import, we can create a GH issue to track and make it part of the next major bump (which we are planning for rn). The issue might even just track everything we can do for @sentry/ember next major bump.

Thoughts @k-fish?

@AbhiPrasad AbhiPrasad merged commit cc47929 into getsentry:master Nov 12, 2021
onurtemizkan pushed a commit that referenced this pull request Dec 19, 2021
Two old versions were brought in by @sentry/ember
- `0.4.x` was brought in by the very specific, old version of ember-auto-import that the package depends on. It is only able to depend on `1.6.x` releases, which date back to June of 2020
- `0.37.0` was depended on specifically, but the latest is `0.47.1`. Because these are pre-major releases, they will not naturally fluctuate to a newer version

This PR updates both of these dependencies to the latest versions possible without making a breaking change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants