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

Option to strip out sourcemaps after sentry upload #1

Closed
t3dotgg opened this issue Jun 3, 2021 · 14 comments
Closed

Option to strip out sourcemaps after sentry upload #1

t3dotgg opened this issue Jun 3, 2021 · 14 comments
Assignees

Comments

@t3dotgg
Copy link

t3dotgg commented Jun 3, 2021

Heyo! This package has been a lifesaver :)

Seems like the current setup requires build: {sourcemaps: true} , which results in sourcemaps being bundled into the public build

Would love to strip those out after the sentry upload. Happy to file a PR if you think this is a good idea

Thanks!
-Theo

@O-Hooman
Copy link

Heyo! This package has been a lifesaver :)

Seems like the current setup requires build: {sourcemaps: true} , which results in sourcemaps being bundled into the public build

Would love to strip those out after the sentry upload. Happy to file a PR if you think this is a good idea

Thanks!
-Theo

I think you can use rimraf to delete the map after the build/upload before you serve it. So it would be like this in the build script at package.json I think

vite build && rimraf dist/**/*.js.map

@ikenfin
Copy link
Owner

ikenfin commented Jul 6, 2021

Hi @theobr !

Yeah, i've thinking about this feature while developing the package, but i like the unix-way when one program is created for one job.

This approach make it possible to combine it with other tools (for example there is plugin to upload sourcemaps to bugsnag - https://github.com/ElMassimo/vite-plugin-bugsnag)

While personally i mostly use Jenkins to build and deploy (just skipping .map files from sync), you can also use rimraf script, suggested by @O-Hooman.

@filiptammergard
Copy link
Contributor

filiptammergard commented Apr 6, 2023

This is such a common need that I personally think it's worth considering supporting. Manually deleting source maps after build feels strange, since it's relying on implementation details of the plugin to work. How can we be sure the source map has been uploaded before it's removed? Isn't that timing an implementation detail that could change anytime, which would then break Sentry source maps if we do something like vite build && rimraf dist/**/*.js.map?

Happy to help adding an option for this if it's something you'd like to support!

@ikenfin
Copy link
Owner

ikenfin commented Apr 6, 2023

@filiptammergard ah, i see now what is confusing about this conditionals.

It supposed that when we getting error from sentry, then the plugin must report crash and stop build process, so vite would return exit code 1, and that will prevent calling next script (rimraf for example).

In general, this && rimraf way is totally ok, but we have little bug in sentry error handling - instead of throwing error and stop buildprocess, plugin just report this as warning. So yes, rimraf in current implementation will be runned even when sentry crashed, but anyway when you got that crash - you will want to re-run build.

I will fix it by adding emergency exit, so that chained scripts usage would be safe enough.

@ikenfin
Copy link
Owner

ikenfin commented Apr 6, 2023

Fixed in 1.2.0 - now default behavior is crash on catching sentry errors. Old behavior can be returned using legacyErrorHandlingMode: true option (https://github.com/ikenfin/vite-plugin-sentry/wiki/What-is-legacyErrorHandlingMode)

@filiptammergard
Copy link
Contributor

Cool @ikenfin, looks like a good change!

It was not what I actually meant though—I was referring to what @t3dotgg said about adding an option for removing source maps after upload automatically.

When you don't want to include source maps in hosted bundle, you enable it anyway just for this plugin to send the source maps to Sentry. Because of that I think it would make sense to also make it possible to let the plugin automatically remove the files directly after sending them—to not have to worry about removing them in the wrong way or anything similar.

What do you think? 🙂

@ikenfin
Copy link
Owner

ikenfin commented Apr 11, 2023

@filiptammergard yeah, i see what you mean. While technicaly there are no problems to make this change, there are some of my points against including options like this:

  1. Sourcemaps generating is enabled by asking Vite bundler itself (build: { sourcemap: true }), so it would be better to be implemented in Vite itself (for example option to build kind of "temporary" sourcemaps which are exists while Vite runs), they already has 'hidden' option to strip sourcemap link comments, so if there will be demand on this temporary sourcemaps - i think they can implement the feature
  2. There can be multiple plugins reliying on sourcemaps. At the moment it's not a big deal - plugins runs sequentional. But what if Vite team at some day will introduce some task parallelization (like it was with Gulp tasks) and there will no any sequential guarancy anymore?
  3. This option will require to include another dependency (rimraf or similar) and keep it updated. This can be unused or untrusted by many of users (personally i don't like any additional stuff in packages dependencies which can be replaced with simple script. Especialy fs linked stuff)

@ikenfin
Copy link
Owner

ikenfin commented Apr 11, 2023

@filiptammergard i was created feature request in Vite repository. You can track changes here - vitejs/vite#12828

Hope Vite team will introduce something like requested feature

@filiptammergard
Copy link
Contributor

@ikenfin I see your points! Thanks. Let's hope something like that is added to Vite!

@filiptammergard
Copy link
Contributor

I followed the discussion in vitejs/vite#12828 and I can see why they won't add an additional option for this since it's possible to achieve in plugin-land.

Do you still not think this is an option to support from the plugin? You could enforce it to run in the end as they showed and you wouldn't have to introduce any dependencies, rather you could do it like this: vitejs/vite#12828 (comment)

For the very rare cases where you would be using multiple plugins depending on the source maps, you could just remove the source maps the manual way instead in app-land. From the user's perspective it's pretty obvious that you can't remove source maps in one plugin if they are needed in another plugin, anyway. And in the vast majority of cases, you won't use multiple plugins depending on source maps.

WDYT?

@ikenfin
Copy link
Owner

ikenfin commented Apr 13, 2023

@filiptammergard yeah, seems they do not want to include new options.

By the way, maybe the best solution for now would be to use separate plugin, like this one - https://github.com/algesthesiah/vite-source-map-cleanup

Also, seems like vitejs/vite#12828 (comment) is working only with vite 4+. On 2 and 3 this hook does'nt see sourcemaps files. I think i will add support of this with warning on older Vite

I've used that simple plugin to test:

const makePlugin = (i) => {
  return {
    name: `plugin_${i}`,
    enforce: 'post',
    generateBundle(_, bundle) {
      console.log('start: generateBundle')
      for (const file in bundle) {
        console.log('file: ', file)
      }
      console.log('endof: generateBundle')
    }
  }
}

// ...
plugins: [ react(), makePlugin('test') ]
// ...
# Vite 2 and 3 has same output - no sourcemap files
generateBundle
file:  assets/react.35ef61ed.svg
file:  assets/index.e7ab5110.js
file:  assets/index.b9195027.css
file:  index.html
endof: generateBundle

# Vite 4 output is okay
generateBundle
file:  assets/react-35ef61ed.svg
file:  assets/index-3fce1f81.css
file:  assets/index-9d20c733.js
file:  assets/index-9d20c733.js.map
file:  index.html
endof: generateBundle

@ikenfin ikenfin reopened this Apr 13, 2023
@whispergong
Copy link

My build.outDir is dynamic, so I must get it in configResolved.And then I want to delete sourcemap in closeBundle.
However, the upload behavior of vite-plugin-sentry also occurs in closeBundle, and the enforce is post, which prevents me from using the vite plugin to delete the sourcemap after the upload behavior of vite-plugin-sentry ends

@ikenfin
Copy link
Owner

ikenfin commented Jun 15, 2023

Hi! It's been a long time of silence. I've tested delete at generateBundle stage approach, but it's not working for us. Because of generateBundle stage runs earlier than closeBundle, when we delete files from bundle - they are missing at closeBundle stage, so there are nothing to upload.

Next release version will contain cleanSourcemapsAfterUpload option to remove sourcemap files after upload complete - #328

@ikenfin ikenfin self-assigned this Jun 15, 2023
@ikenfin
Copy link
Owner

ikenfin commented Jun 15, 2023

Added cleanSourcemapsAfterUpload (v1.3.0) which must work on any vitejs versions above than 2 (tested on 2.6.0, 3.2.0 and latest 4.3.9) 👍

@ikenfin ikenfin closed this as completed Jun 15, 2023
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

5 participants