-
-
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
fix(sveltekit): Avoid loading vite config to determine source maps setting #15440
Conversation
9bb759d
to
efaf8c5
Compare
// resolving filesToDeleteAfterUpload here, because we return the original deletion plugin which awaits the promise | ||
resolveFilesToDeleteAfterUpload(undefined); |
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.
this is the ugly part of this PR: We always need to resolve the promise we pass to the vite plugin. So everywhere where we early return, in addition to the expected places.
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
This PR refactors how we determine the user-provided source map generation settings in our
sentrySvelteKit
vite plugins.Previously, we'd call Vite's
loadConfigFromFile
function which sounded good but had a couple of drawbacks:vite
in a vite config creates a circular import chain (fixed via fix(sveltekit): Avoid top-levelvite
import #15371)sentrySvelteKit
loads Vite config in the wrong mode #15414, we always loaded themode: "production"
version of the config, which was also problematic.Since there's no reliably way to read the
mode
for Vite, and parsingprocess.argv
also seemed sketchy, I decided to go down the Vite-recommended route and to read the config in theconfig
hook. This required a change in our@sentry/vite-plugin
which was released in 3.2.0, so this PR also bumps the Vite plugin version.Concrete changes:
config
hook of our source maps settings sub pluginfilesToDeleteAfterUpload
promise whenever we know what to set it to (using a promise allows us to defer this decision to plugin hook runtime rather than plugin creation time)closes #15414