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

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

Closed
3 tasks done
zip-fa opened this issue Oct 26, 2023 · 8 comments · Fixed by #9412
Closed
3 tasks done

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

zip-fa opened this issue Oct 26, 2023 · 8 comments · Fixed by #9412
Assignees
Labels
Package: angular Issues related to the Sentry Angular SDK Type: Bug

Comments

@zip-fa
Copy link

zip-fa commented Oct 26, 2023

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/angular-ivy

SDK Version

7.75.1

Framework Version

17.0.0-rc.1

Link to Sentry event

No response

SDK Setup

No response

Steps to Reproduce

ng serve on esbuild-based builder in angular

Expected Result

Working angular project

Actual Result

Not working angular project :))

Logs, explanation: angular/angular-cli#26135

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 Oct 26, 2023
@github-actions github-actions bot added the Package: angular Issues related to the Sentry Angular SDK label Oct 26, 2023
@Lms24
Copy link
Member

Lms24 commented Oct 27, 2023

Hi @zip-fa thank you for reporting! Haven't looked into Angular 17 yet but I'll try to reproduce this. Are you using some kind of SSR setup or does this happen out of the box?

@zip-fa
Copy link
Author

zip-fa commented Oct 27, 2023

@Lms24 here is the repro: https://github.com/zip-fa/ng17-sentry-bug

don't forget to run npm install with --force option :)

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 Oct 27, 2023
@Lms24
Copy link
Member

Lms24 commented Oct 27, 2023

Awesome, thank you!

don't forget to run npm install with --force option :)

I know, I'll bump the peer deps as soon as we figured this out ;)

@Lms24
Copy link
Member

Lms24 commented Oct 27, 2023

Workaround (only required for SSR)

Okay, so I tried a couple of things and I think this can be narrowed down to our SDK being imported into server code:

  • app.config.ts imports Sentry and registers the Sentry ErrorHandler (+ Sentry TraceService in a full SKD setup)
  • app.config.server.ts imports appConfig and merges it with its serverConfig object.

If I manually declare appConfig in app.config.server.ts without adding Sentry to it but keep Sentry providers in app.config.ts, ng serve works without an error message:

// app.config.server.ts

import { mergeApplicationConfig, ApplicationConfig } from '@angular/core';
import { provideServerRendering } from '@angular/platform-server';
import { provideRouter } from '@angular/router';
import { routes } from './app.routes';
import { provideClientHydration } from '@angular/platform-browser';
import { appConfig } from './app.config';

//import { appConfig } from './app.config';

const appConfigWithoutSentry: ApplicationConfig = {
  providers: [provideRouter(routes), provideClientHydration()],
};

const serverConfig: ApplicationConfig = {
  providers: [provideServerRendering()],
};

export const config = mergeApplicationConfig(
  // appConfig,
  appConfigWithoutSentry,
  serverConfig
);

What's important to know is that our SDK for Angular only works in the browser environment. It's not meant to be imported by the server (e.g. for SSR). We might expand angular SSR support in the future but we haven't looked into this a lot and additionally have a lot of other things going on.

Some other observations:

  • Seems like everything works fine for Angular 17 apps without SSR, so I'm gonna bump peer deps soon.
  • ng build also works for Angular 17 with SSR without the need for the workaround above

@zip-fa
Copy link
Author

zip-fa commented Oct 27, 2023

In angular v17, when ssr option is enabled, vite starts to serve angular in ssr mode under the hood with ng serve command. So every library must support it from v17 internally. You can do some checks to skip logic inside your library so it can work

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 Oct 27, 2023
@Lms24
Copy link
Member

Lms24 commented Oct 27, 2023

You can do some checks to skip logic inside your library so it can work

@zip-fa do you have some pointers for what kind of checks?

To me, it seems to me like the require statements in the UMD bundle of the Angular v12 package format (APF) are not compatible with Vite-based ng serve. Not sure how to work around this with runtime checks 🤔

@zip-fa
Copy link
Author

zip-fa commented Oct 27, 2023

There was simple environmental check, which is currently broken in serve mode:

const isServer = isPlatformServer(inject(PLATFORM_ID))

angular/angular-cli#26148

So i don't really know. Maybe you should consult with angular's core team

@zip-fa
Copy link
Author

zip-fa commented Oct 27, 2023

You can do some checks to skip logic inside your library so it can work

@zip-fa do you have some pointers for what kind of checks?

To me, it seems to me like the require statements in the UMD bundle of the Angular v12 package format (APF) are not compatible with Vite-based ng serve. Not sure how to work around this with runtime checks 🤔

I digged deeper today. I have already structure as you described, sentry got included only in app.config.browser.ts, but it executes under-the-hood in vite with application builder (which will be new default soon).
Also, even if you get this working without issues with your "hack", you will stuck with the same error after this simple import:

import { setUser } from '@sentry/angular-ivy';

setUser(someId);

So there must be some kind of fix for this, or maybe new version of package which will support new reality (vite and esbuild).
The faster you guys fix this the less issues you get soon :)

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 Oct 27, 2023
@getsantry getsantry bot removed the status in GitHub Issues with 👀 Oct 31, 2023
Lms24 added a commit that referenced this issue Oct 31, 2023
…ith SSR config (#9412)

Adjust the entry points of `@sentry/angular-ivy`'s
`package.json` to point directly to FESM2015 bundles (= bundled ESM2015
code) instead of the UMD bundles. This fixes an error when the old (no
longer supported) UMD bundles were picked up by Vite in Angular apps
with SSR config (#9376).

A proper long term fix to this is to bump to Angular 15 in this package
which we can only do in a new major.
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
2 participants