-
-
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): Accept all vite-plugin options #15638
Conversation
Adds support for `unstable_sentryVitePluginOptions` which can be used to pass any valid vite-plugin option. Fixes getsentry#15601
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 the PR! We should address the one comment I left!
@@ -85,6 +85,7 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => { | |||
}, | |||
}, | |||
debug: options.debug ?? false, | |||
...unstable_sentryVitePluginOptions |
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.
In theory we need to spread this into all of the sub-options to be correct. Similar to how we do it here:
...sentryBuildOptions.unstable_sentryWebpackPluginOptions?.reactComponentAnnotation, |
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.
Done: 15198e3
(#15638)
Btw. I'm not sure if the nextjs package spreads into sub-options correctly. It looks to me like at the end, it overwrites them anyway:
...sentryBuildOptions.unstable_sentryWebpackPluginOptions, |
It looks like there is no test that would test a scenario with unstable_sentryWebpackPluginOptions
set 🤔
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.
Oh I think you're right. Thanks for catching that. I'll fix that in a follow-up 👍
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 a lot for contributing this!
This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #15638 Co-authored-by: Lms24 <8420481+Lms24@users.noreply.github.com>
@angelikatyborska Thanks for contributing this! :) It's for the code snippet in the |
I opened a PR for the docs: getsentry/sentry-docs#13035 |
Adds support for
unstable_sentryVitePluginOptions
which can be used to pass any valid vite-plugin option.Fixes #15601
Before submitting a pull request, please take a look at our
Contributing guidelines and verify:
yarn lint
) & (yarn test
).Hello 👋 this is my first ever contribution in this repository. I hope I understood and followed all the contribution guides correctly. Please let me know if I need to fix something, add more tests, refactor something. Or feel free to push more commits yourself - whichever working style you prefer!
I tried to base my implementation of this feature for sentry/astro on the existing implementation for sentry/sveltekit, from where I copied for example the type comment.