-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Update bundler plugin snippets to use new Debug ID process by default #6653
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
By including this option, I think people will assume that it's related to releases because of the introduction text
sourcemaps: {
// Specify the directory containing build artifacts
assets: "./dist/**",
},From my perspective, here are some requirements for this PR
|
Good point. I removed the references to releases.
Hm, I don't think these are requirements - 1 and 3 definitely aren't, so I wouldn't add them - just added text bloat. https://develop.sentry.dev/sdk/philosophy/#prioritize-customer-convenience-over-correctness As for 2. I am just gonna ship this and see what the consequences are. We can add a note later. |
|
Heres a suggestion from me Vite (example)In this guide, you'll learn how to successfully upload source maps using our vite bundler plugin. IntroductionYou can use two methods of uploading source maps:
InstallationConfigurationLearn more about configuring the plugin in our Sentry Vite Plugin documentation. Example: import { defineConfig, loadEnv } from "vite";
import { sentryVitePlugin } from "@sentry/vite-plugin";
export default defineConfig(({ command, mode }) => {
// Load env file based on `mode` in the current working directory.
// Set the third parameter to '' to load all env regardless of the `VITE_` prefix.
const env = loadEnv(mode, process.cwd(), "");
return {
build: {
sourcemap: true, // Source map generation must be turned on
},
plugins: [
// Put the Sentry vite plugin after all other plugins
sentryVitePlugin({
org: "___ORG_SLUG___",
project: "___PROJECT_SLUG___",
// Auth tokens can be obtained from https://sentry.io/settings/account/api/auth-tokens/
// and need `project:releases` and `org:read` scopes
authToken: env.SENTRY_AUTH_TOKEN,
sourcemaps: {
// Specify the directory containing build artifacts
assets: "./dist/**",
},
// Optionally uncomment the line below to override automatic release name detection
// release: env.RELEASE,
}),
],
};
});
|
|
Thanks for the suggestion! I think this is still waaaay too confusing. As a naive user, I don't know what an artifact bundle is nor what a release bundle is. This is information they should just not care about. |
|
@lforst I understand your point if the happy path of configuration works flawlessly. However, we know this flow isn't airtight. I'm concerned by providing this option without an explanation why it exists, people might assume their release bundle setup is broken because they've incorrectly configured this specific option. "Why are my source maps being uploaded?" "aha its because of sourcemaps.assets!" |
|
i added a comment to the snippet what to do if theyre on an old version |
|
You and your line comments... lol no one will see it! I don't think a comment is sufficient if the default option is not going to work for 99.9% of users. We're also loosing an opportunity to 'upsell' them on upgrading their SDK. |
|
I just want to get this out the door. Nothing is lost, nothing is permanent. |
|
So if the goal is to literally update the snippets, then of course, the job is done. I just don't see the harm in expanding the scope of this a little to potentially clarify any unknowns for the users. Am I over thinking this @AbhiPrasad @Lms24 |
|
The reason I think this is fine is because the code snippet is literally the only thing on the page users are guaranteed to look at. If they're not looking at the snippet, they didn't achieve anything at all. To me, the text surrounding the code snippet is just noise. I know, that as a developer I would ignore everything besides the snippet. That's why I acutally think it is the most prominent part of the page. |
|
I'm shipping this because it is out in the bundler plugins and this PR doesn't hurt. Feel free to open a follow-up PR if necessary. |
|
I'm in favour of releasing the docs sooner rather than later so that we get as much adoption for debug ids as possible. We can keep iterating on the specific language though, I'm in favour of highlighting the version requirements like you mentioned @Jesse-Box - the comment is not clear enough in my opinion, but this can come in follow up PRs! |
Waiting for the release of getsentry/sentry-javascript-bundler-plugins#204
Resolves getsentry/sentry#47230