-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat(astro): Add bundleSizeOptimizations
vite options to integration
#13250
Conversation
size-limit report 📦
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for splitting up the PRs! LGTM!
@@ -48,6 +48,9 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => { | |||
sourcemaps: { | |||
assets: uploadOptions.assets ?? [getSourcemapsAssetsGlob(config)], | |||
}, | |||
bundleSizeOptimizations: { | |||
excludePerformanceMonitoring: options.bundleSizeOptimizations?.excludeTracing || false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
m: What about passing all the bundle size optimizations through (or more specifically, defining all of them)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me, @Lms24 what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good, let's do it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for updating! LGTM!
(let's make sure to update the PR title to reflect that we now include all options)
}, | ||
debug: options.debug ?? false, | ||
}), | ||
bundleSizeOptimizations: options.bundleSizeOptimizations, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
h: sorry almost missed this but the vite plugin doesn't yet support excludeTracing
, right?
I think we need to remap this key specifically to excludePerformanceMonitoring
bundleSizeOptimizations.excludeTracing
bundleSizeOptimizations
vite options to integration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adjusting! Now we can
The bundler-plugins still refer to the option as
excludePerformanceMonitoring
(here), but this is going to be renamed toexcludeTracing
, so I already used the new naming as discussed with @Lms24 and @mydea.part of #13013