Use a more up-to-date rollup sourcemap plugin #1951
Conversation
|
Bundle size report: Size Change: 0 B
ℹ️ View DetailsNo major changes |
| "rollup-plugin-sourcemaps": "^0.6.3", | ||
| "rollup-plugin-include-sourcemaps": "^0.7.0", |
There was a problem hiding this comment.
Is the package very new? I am a bit scared of replacing a package with 600k weekly downloads with one which as 5...
https://www.npmjs.com/package/rollup-plugin-sourcemaps
https://www.npmjs.com/package/rollup-plugin-include-sourcemaps
There was a problem hiding this comment.
The "big" package hasn't had a commit for years, and the new is a fork of it made with a single commit on top of it. I did create a PR on the original package as well, but it seems that other people have attempted that before. I can change back if it gets merged eventually.
There was a problem hiding this comment.
At the end of the day, this entire plugin is just one file with 73 lines of code...
|
We need to decide if we want to create a pre-release before these changes get merged, just to be oh the safe side... |
|
I think it would be good to make a pre-release as a smoke test. We can always swap this later - my personal fork of this library has everything updated even beyond this PR, and it could help out too if we have to adjust anything. |
|
What does this plugin do anyway? Sourcemaps are already broken in CI due to an issue with https://github.com/maplibre/maplibre-gl-js/actions/runs/3724432570/jobs/6316584006#step:6:16 |
|
@rotu it's a good point - the bundling space is changing really fast currently (I'd love to replace terser), so maybe we can wait this out a bit or remove the sourcemap plugin. |
|
I still don't quite understand what this plugin supposedly does :-). Also, I expect webapp developers to do tree-shaking and minification downstream. So maybe we can just drop terser entirely. |
|
From our rollup config: plugins: [ That is the only documentation on it I can find |
|
I see that but it seems Maplibre has had defective source maps since 2.2.0-pre.4 |
|
Not everyone is using webpack etc, maplibre can be used as is with script tag in a HTML. Although not common these days... |
True. And those users are getting broken sourcemaps and stack traces today. I'm thinking that use case is better served with non-minified files or with an optimizing CDN like the excellent https://www.skypack.dev/ |
|
Looking at the diff, it looks like it is related to the json assert introduction if I need to guess: |
|
There's issue #1582 which was supposedly fixed in #1780 But I can't tell if the issue was correctly diagnosed and/or fixed. Maybe @pgoldberg knows. |
#1780 did fix #1582. Here's a gist of the bundled distribution and its source map built off of that commit: https://gist.github.com/pgoldberg/122b40f29a04d85ce3338209f6b12bed It does look like this has since regressed due to the terser plugin issue that you pointed out |
We previously used: https://github.com/maxdavidson/rollup-plugin-sourcemaps
It hasn't been updated for years and is not rollup-3 compatible due to a 'peerDependency' in @rollup/pluginutils which it relies on. This PR change to a more recent fork of it.
This PR can be tested by running 'npm i' and checking that the dependency warning about conflicting rollup versions is gone.