getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.68k stars 1.49k forks source link

v7.112 webpack & angular warning: node_modules/@sentry/integrations/esm/offline.js depends on 'localforage'. CommonJS or AMD dependencies can cause optimization bailouts. #11817

Closed rubiesonthesky closed 6 days ago

rubiesonthesky commented 3 weeks ago

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/browser

SDK Version

7.112.0

Framework Version

No response

Link to Sentry event

No response

SDK Setup

Sentry.init({
  dsn: '',
  environment: new URL('', window.location.href).hostname,
  release: VERSION,
  autoSessionTracking: true,
  ignoreErrors: ['ResizeObserver loop limit exceeded'],
  denyUrls: [/localhost/],
  initialScope: (scope) => {
    scope.setTags({title: document.title});
    return scope;
  },
  beforeSend(event) {
    // some logic
    return null;
  },
});

Steps to Reproduce

This is Angular app, using Angular 16. (And using @sentry/browser)

  1. Update Sentry from v7.111 to v.112
  2. ng build

Expected Result

Not to log any warnings

If I want earlier functionality, do I need to set integrations: [] in init? This was not covered in the release notes.

Actual Result

Warning printed

Warning: <project>/node_modules/@sentry/integrations/esm/offline.js depends on 'localforage'. CommonJS or AMD dependencies can cause optimization bailouts.
For more info see: https://angular.io/guide/build#configuring-commonjs-dependencies
AbhiPrasad commented 3 weeks ago

To clarify this is just a warning right? It does not cause the build to fail?

I think we should not vendor the offline integration like we do with the rest because of this problem. Opening a PR!

AbhiPrasad commented 3 weeks ago

Well nvm we don't vendor offline at all, but I guess webpack complains about it. I think we have to alias the localforage import with a dynamic require, or vendor in localforage to get around this problem.

@mydea could you take a look on Monday?

rubiesonthesky commented 3 weeks ago

Yeah luckily it's just warning, so it does not block building the application. :)

I can see that in package.json, packages immediate, lie and localforage are now added when they weren't present earlier.

These are the only Sentry packages that we have installed

"@sentry/browser": "7.112.0",
"@sentry/types": "7.112.2",
AbhiPrasad commented 3 weeks ago

There's another solution here we can do which sucks, but is better than nothing to unblock us.

  1. Create a @sentry-internal/integrations package, and put all of the integrations other than offline into that package.
  2. Make @sentry/integrations depend on @sentry-internal/integrations, and then have it use offline integration
  3. Make browser sdk depend on @sentry-internal/integrations

Sorry for the trouble @rubiesonthesky - appreciate your patience while we get a fix out!

mydea commented 6 days ago

We've released v8.0.0 which dropped the offline integration overall - so updating to this should get rid of this warning! I'll close this issue, if you still run into this on the latest version of the SDK, please re-open the issue - thank you!