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

[v8] Add browser-runtime package #9917

Closed
mydea opened this issue Dec 19, 2023 · 4 comments
Closed

[v8] Add browser-runtime package #9917

mydea opened this issue Dec 19, 2023 · 4 comments

Comments

@mydea
Copy link
Member

mydea commented Dec 19, 2023

Lukas has identified a potential issue, if we want to bundle all dependencies of packages into a single index.js file, including all dependencies. If, as of now, e.g. @sentry/react depends on @sentry/browser and adds some more stuff on top it picks from @sentry/core, if @sentry/browser inlines @sentry/core it will not be possible to dedupe core imports in the react package.

A potential solution for this could be to have a @sentry-internal/browser-runtime package similar to the server-runtime package, which holds basically all the code from @sentry/browser, but does not bundle it all into one file. Then both @sentry/browser and the other browser packages like vue, react, ... can depend on browser-runtime too, and all of these can then bundle all their dependencies - including core - correctly into a single file.

This does not technically need to happen in v8, but is probably a requirement if we want to bundle all dependencies together.

@mydea mydea mentioned this issue Dec 19, 2023
@AbhiPrasad
Copy link
Member

if we want to bundle all dependencies of packages into a single index.js file

I would avoid inlining deps if we can, having single index.js but keeping package boundaries between core/browser/nextjs etc. seems reasonable to me.

@lforst
Copy link
Member

lforst commented Dec 19, 2023

The plan is not to bundle dependencies but to just set preserveModules: false.

@mydea
Copy link
Member Author

mydea commented Dec 19, 2023

Hmm I don't remember anymore who brought this up, but I do remember this was mentioned at some point - but I have no strong feelings on this either way. If we don't want to inline core, then this is not necessary.

@Lms24
Copy link
Member

Lms24 commented Feb 28, 2024

In the interest of streamlining (going through #9508), let's table this for v8 - wdyt? I'll close the issue for now but we can reopen if necessary

@Lms24 Lms24 closed this as not planned Won't fix, can't repro, duplicate, stale Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants