Convert `@sentry/ember` addon to V2 addon format

[johanrd]

Problem Statement

Modern Ember build tools (@embroider/vite and @embroider/webpack) benefit greatly from static, pre-built addons when bundling the app.

The current @sentry/ember package is currently on the old ember addon-format. It works, but it is not optimized for the modern ember build tooling, as it does currently needs be processed/rewritten by @embroider on every build.

Also, it is currently sorted with warnings in the group of other legacy V1 addons when running e.g. npx ember-addon-v2-scanner@latest:

NPM Name	Version	Current version	On V2 Addon	V2 Addon available
'@sentry/ember'	'9.9.0'	'9.9.0'	❌	❌

(related to https://github.com/getsentry/sentry-javascript/issues/12336, but this is more specific)

Solution Brainstorm

Here is a guide for porting an addon to the V2 format .

[mydea]

Hey, thanks for opening this.

We are currently pretty busy, so I can't really say when the team will find time to pick this up. We'd be happy to review a PR for this, though! A challenge would be that we have to remain compatible with the old format too - not sure if/how it is possible for us to publish both variants at the same time? If this is not possible in the same package, we can probably only make this change in the next major (v10), but work could be prepared for it already in advance.

[johanrd]

@mydea ok, good. Yes, it would maybe considered a breaking change in some cases (but also maybe not for a simple addon like this?). The V2 addon format with @embroider/addon-shim provides backwards compatible support for classic builds.

[mydea]

Basically, if current E2E tests using the Ember SDK pass, any change is fine 😅 (I believe). As long as compatibility remains given, we can always update! This means it has to support:

Node 18+ Ember 4+

Which is our current support policy.

[johanrd]

Ok, good. Backwards compatibility not be a problem then, as I believe the V2 addon format is supported by ember 3.28+.

[RobbieTheWagner]

Is anyone currently working on this? If not, I might be able to help out.

[andreiborza]

@RobbieTheWagner not at the moment, we're all swamped with other stuff at the moment 😅. Contributions are highly appreciated tho!

[RobbieTheWagner]

@andreiborza gotcha, I can definitely take on the work. I think one thing that will make it harder though is that the repo is still using yarn v1. Do you know if anyone is working on newer yarn versions or better yet switching to pnpm?

[s1gr1d]

Why would using yarn v1 make it harder? Just interested in the reason because it helps us prioritize.

We were talking about eventually upgrading to yarn v4 and we are gradually working towards it (starting with this PR). We currently don't have plans to switch to pnpm.

[RobbieTheWagner]

Why would using yarn v1 make it harder? Just interested in the reason because it helps us prioritize.

We were talking about eventually upgrading to yarn v4 and we are gradually working towards it (starting with this PR). We currently don't have plans to switch to pnpm.

@s1gr1d it is hard for me to fully explain why, but mostly around issues with yarn v1 hoisting all dependencies and handling peerDependencies incorrectly. I believe yarn v4 and pnpm enforce that you must install anything you import, and are better at handling multiple versions of the same package, since they do not hoist.

v2 Ember addons are very strict, so they can be compatible with vite, so using a tool that aligns with that like yarn v4 or pnpm is helpful to ensure all the things are working as intended.

[s1gr1d]

You can use whatever package manager in your Ember project. I see your concerns with yarn v1 and you don't have to use it for your project when you use Ember addons v2.

However, you brought that up related to contributing to sentry-javascript. You should not run into problems here if you follow the contribution guidelines. The package manager which is used in the sentry-javascript project should not affect your Ember project since you install the dependencies yourself (with your package manager).

[RobbieTheWagner]

@s1gr1d I don't think you are understanding what I am saying. This issue is about converting @sentry/ember to a v2 addon. If we do not have a package manager that actually handles dependencies correctly, like yarn 4 or pnpm, then we cannot really convert to a v2 addon because v2 addons are strict and require that every dependency actually exist, not be auto installed or hoisted.

[s1gr1d]

I think we may have had some miscommunication here, so let me clarify my understanding and make sure we're aligned on the technical challenges.

Assuming you mean the sentry-javavascript repository with "we", I suppose you're specifically concerned about the development and testing process during the v2 addon conversion within the sentry-javascript repo itself, not downstream usage.

There are two different scopes to consider here:

For end-users of the addon: They can use whatever package manager they prefer when consuming a published v2 addon. The package manager used in Sentry's development environment doesn't affect their projects.

For developing the addon itself: When we're developing a v2 addon, we need our development environment to surface dependency issues early, during development rather than after publishing. Our E2E tests are separate and install dependencies with pnpm, so eventual dependency issues would face the latest in our E2E tests.

Does this distinction between development-time vs runtime dependency resolution make sense?

[RobbieTheWagner]

@s1gr1d I am talking only about development. The package manager a library chooses to use has no bearing on consuming packages and never has.

To convert the Sentry Ember addon to a v2 addon we need pnpm or yarn 4 to align with the strictness of v2 addons.

[s1gr1d]

I (and the rest of the JS SDK team) would argue that it should not create a problem while developing the add-on. For testing, we use pnpm and if you run into any issues, you can always ask :)

[RobbieTheWagner]

@s1gr1d I am telling you it does though. I'm on the Ember Learning Core team and have converted many addons to v2 and maintain quite a large number of addons in general, so I know what I am telling you is true, it's not a guess.

I'm not sure I understand the motivation behind staying on yarn v1 for you all anyway, but if that is what you prefer, we may benefit from splitting out the Ember addon into its own package.

[s1gr1d]

Okay then you definitely have experience with that - and it sounds like a very good reason for upgrading yarn. Like I said above, concrete reasons help us prioritize the upgrade to yarn v4. We'll keep you updated on the changes.

[mydea]

So, while generally we eventually do want to move to yarn 4, this is not as easy as just bumping the yarn version. I actually made a draft to try this out here: https://github.com/getsentry/sentry-javascript/pull/16744 but it fails in many ways, esp. because it seems to not work great/out of the box with yarn & ci stuff. This needs a bunch of careful work that touches all of the SDKs (not just Ember), we do not know when we will get around to do complete this.

Apart from this: I still do not understand why this should be necessary to update the addon. What exactly should not work with the current yarn v1 approach? I understand that hoisting etc. may affect this, but this should not be a blocking concern and can very much be made to work. Importantly, the thing where this matters (acceptence/e2e tests) are handled in an isolated way anyhow - we do not run ember acceptence test in the monorepo package, but isolated in e2e-tests, which spins up a fully isolated pnpm-based app in a tmp dir, unaffected by the monorepo and yarn 1. This should be more then enough to verify that this works as expected, and unit/integration tests should work just fine with yarn 1?

[aklkv]

I can give it a try with yarn v1 setup, but the last time I tried I failed miserably to even build it locally.

[mydea]

I you have volta setup, and generally follow our contribution docs, it should work just fine to get things going. let us know if something does not work for you, then we will try to fix it! 🙏

[johanrd]

Side note: Having Sentry as a v2 addon will also for "free" remove the four separate build steps for:

https://github.com/getsentry/sentry-javascript/blob/ea20d8db38f334f01e7c1581ddc0ded16c3bbb27/packages/ember/package.json#L37-L40

Since these are direct dependencies of a v1 addon, they are all separately dragged through a build step in embroider per dependency. (which means 5 in total counting the addon itself)

Some are possible to remove completely, while others are automatically removed to test-app dependencies by v2 conversion🎉

[chargome]

@RobbieTheWagner @aklkv is anyone of you still working on this?

[aklkv]

@RobbieTheWagner @aklkv is anyone of you still working on this?

This work has been deprioritized for us for the time being, we are hoping to circle back in the next couple of weeks

[getsantry[bot]]

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!

---

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

[RobbieTheWagner]

Could we remove the auto close stuff here please? We definitely intend to work on this eventually and would like to keep the issue open.

[chargome]

Removed it 👍