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

[vitest] require() of ES Module @angular/core/fesm2022/core.mjs not supported #9376 #10511

Closed
3 tasks done
andreialecu opened this issue Feb 6, 2024 · 7 comments · Fixed by #10569
Closed
3 tasks done
Labels
Package: angular Issues related to the Sentry Angular SDK Type: Bug

Comments

@andreialecu
Copy link
Contributor

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/angular

SDK Version

7.99.0

Framework Version

No response

Link to Sentry event

No response

SDK Setup

No response

Steps to Reproduce

Repro at:
https://stackblitz.com/edit/vitest-dev-vitest-b9tqx7?file=package.json,test%2Fbasic.test.ts%3AL3

Expected Result

No error.

Actual Result

I'm attempting to migrate a test suite to vitest as per: https://analogjs.org/docs/features/testing/vitest#manual-installation and @sentry/angular seems to prevent this because as soon as it is imported, Vite will complain about:

Error: require() of ES Module@angular/core/fesm2015/core.mjs not supported.
Instead change the require of @angular/core/fesm2015/core.mjs to a dynamic import() which is available in all CommonJS modules.

This was reported previously here: #9376 but in a different context

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Feb 6, 2024
@github-actions github-actions bot added the Package: angular Issues related to the Sentry Angular SDK label Feb 6, 2024
@andreialecu
Copy link
Contributor Author

andreialecu commented Feb 6, 2024

The above issue seems to happen with @sentry/angular but it appears there's a package @sentry/angular-ivy where this should be resolved according to #9412.

However, if I change the import to @sentry/angular-ivy it results in:

Error: Failed to resolve entry for package "@sentry/angular-ivy". The package may have incorrect main/module/exports specified in its package.json.

Repro at:
https://stackblitz.com/edit/vitest-dev-vitest-pcf6gd?file=package.json%3AL26-L26,test%2Fbasic.test.ts

@Lms24
Copy link
Member

Lms24 commented Feb 7, 2024

Hi @andreialecu thanks for writing in!

Interesting, I've seen this before with vitest in other SDKs of ours (specifically Svelte). First off, @sentry/angular is built with Angular 10 so it's very old by now and not recommended for more recent Angular versions. @sentry/angular-ivy is built with Angular 12 but I believe the Angular Package Format 12 is still too old for Vitest, which I guess required type: module and ESM exports declared in package.json.

The good news is that we're currently working on a new major version of our SDKs (v8), meaning we'll be able to drop support for older Angular versions. Our plan for now is to probably go to Angular 14 or 15, depending on these compatibility issues. Also, we'll merge @sentry/angular and @sentry/angular-ivy into one package again. Progress is tracked in #9830.

I'll keep this issue open for now as it represents a specific problem with the current package. However, realistically, we can't fix this on v7. Apologies for that.

@getsantry getsantry bot removed the status in GitHub Issues with 👀 2 Feb 7, 2024
@Lms24 Lms24 changed the title require() of ES Module @angular/core/fesm2022/core.mjs not supported #9376 [vitest] require() of ES Module @angular/core/fesm2022/core.mjs not supported #9376 Feb 7, 2024
@andreialecu
Copy link
Contributor Author

Thanks for looking into this. Please see my second comment here, it appears something else is wrong with angular-ivy which is probably a different issue that can be fixed.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Feb 7, 2024
@Lms24
Copy link
Member

Lms24 commented Feb 8, 2024

So it seems like the problem is that we're missing an "exports" map in package.json. I got your reproduction example to work by appending

"exports": {
    ".": {
      "es2015": "./fesm2015/sentry-angular-ivy.js",
      "esm2015": "./esm2015/sentry-angular-ivy.js",
      "fesm2015": "./fesm2015/sentry-angular-ivy.js",
      "import": "./fesm2015/sentry-angular-ivy.js",
      "require": "./bundles/sentry-angular-ivy.umd.js"
    }
}

to the package.json of @sentry/angular-ivy.

Btw, thanks for providing a reproduction! 🙏

I'll try throwing this into the package and testing with NG12 to 17 again. If this works, we can backport this to our v7 branch. Again, the actual solution here is to bump to a newer Angular version and stick to the package format of that Angular version. For context, this is the APF specification for Angular 15.

@andreialecu
Copy link
Contributor Author

andreialecu commented Feb 8, 2024

Just a note that adding exports could be a breaking change if someone, for some reason, is doing any deep imports to deep paths inside the sentry package. It's not likely, but could happen.

What is more likely, is needing to reach package.json, as some tools and libraries may enumerate packages and their versions, and should still be able to. See how angular is forwarding them here: https://yarnpkg.com/package?q=%40angular%2Fcore&name=%40angular%2Fcore&file=%2Fpackage.json

If you'd like to provide a 'catch-all', it should address both package.json and deep imports, and it should be as simple as adding: "./*": "./*"

To confirm that it works, testing if import * as packageJson from "@sentry/angular-ivy/package.json" or any other deep resolves, should be sufficient.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Feb 8, 2024
@Lms24
Copy link
Member

Lms24 commented Feb 8, 2024

@andreialecu you mean like this?

"exports": {
    ".": {
      "es2015": "./fesm2015/sentry-angular-ivy.js",
      "esm2015": "./esm2015/sentry-angular-ivy.js",
      "fesm2015": "./fesm2015/sentry-angular-ivy.js",
      "import": "./fesm2015/sentry-angular-ivy.js",
      "require": "./bundles/sentry-angular-ivy.umd.js"
    },
    "./*": "./*"
}

generally, I'd argue that anyone doing deep imports must expect things to break but if it's as simple as that, we can do it

@andreialecu
Copy link
Contributor Author

Yes, that should do the trick. That should also cover package.json, which I'd strongly recommend exposing. As mentioned, patterns like (await import("some-pkg/package.json")).version are common.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: angular Issues related to the Sentry Angular SDK Type: Bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants