sentry-javascript icon indicating copy to clipboard operation
sentry-javascript copied to clipboard

feat(browser): [v7] Add moduleMetadataIntegration lazy loading support

Open gilisho opened this issue 1 year ago • 3 comments

This PR fixes #13803. The corresponding PR for latest major version is #13817.

I saw the bundles generation for integration is happening in @sentry/integrations, so I added a corresponding file for modulemetadata in this package that exports that integration from @sentry/core, so that the bundle actually gets created for this integration as needed.

gilisho avatar Sep 27 '24 11:09 gilisho

I am not 100% sure if it is necessary to backport this to v7 - IMHO this is specialised enough that it appears OK to me to not have this in v7? Especially since lazyLoadIntegration is not available there at all. Do you need this in v7?

mydea avatar Oct 07 '24 08:10 mydea

Currently my platform is in v7 (we initially were in v5 so migrating to v8 is going to be impossible ATM), so I do need it in v7 as well. I want to load the integration's bundle and do:

    if (window.Sentry.moduleMetadataIntegration) {
      Sentry.addIntegration(Sentry.moduleMetadataIntegration());
    }

inside window.sentryOnLoad. Without it in v7, the micro-frontend solution would be unusable when using loader script in v7. Maybe lazy loading is an incorrect term for v7, but there's still a need for a bundle for this integration.

gilisho avatar Oct 07 '24 08:10 gilisho

OK, I see, that's fine.

Then, instead of having this in @sentry/integrations, I would generate this from the browser package:

  1. Create an entry point there for the integration in packages/browser/src/integrations/modulemetadata.ts
  2. Add an addon bundle for this in https://github.com/getsentry/sentry-javascript/blob/v7/packages/browser/rollup.bundle.config.mjs, similar to https://github.com/getsentry/sentry-javascript/blob/v7/packages/browser/rollup.bundle.config.mjs#L28

mydea avatar Oct 08 '24 09:10 mydea

Hi @mydea! Could you take a look at this PR as well? This is the correspondent PR of #13817 for v7.

gilisho avatar Nov 11 '24 09:11 gilisho