webpack-contrib / mini-css-extract-plugin

Lightweight CSS extraction plugin
MIT License
4.66k stars 389 forks source link

Adding arbitrary bits of string breaks sourcemaps for `source-map-js` #1075

Closed romainmenke closed 6 months ago

romainmenke commented 8 months ago

https://github.com/webpack-contrib/mini-css-extract-plugin/blob/8480bce915fbb9482823397bff296817db7924d6/src/index.js#L1459

see : https://github.com/postcss/postcss/issues/1914#issuecomment-1915679704


PostCSS uses source-map-js and this package errors when the original line and column are missing in sourcemaps.

see : https://github.com/7rulnik/source-map-js/issues/18


mini-css-extract-plugin at least always inserts a single newline without a mapping to the original source.

So any CSS produced with mini-css-extract-plugin will contain a sourcemap that might error in PostCSS.

Not every user is affected because the error only happens when the user has a noop config for PostCSS which will result in a NoWorkResult there.

With recent changes in PostCSS it became more likely to have a NoWorkResult.


I don't know enough about sourcemaps to be able to tell if this is an issue that should be fixed here or if it is purely a bug in source-map-js.

alexander-akait commented 8 months ago

@7rulnik can you look at this, I think the bug in source-map-js

@romainmenke Can you create reproducible test repo? As you can see all bundlers affected, so I think the problem is not in webpack

7rulnik commented 8 months ago

@alexander-akait I will try to look into it next week. As I understand it happens with source-map too, right?

ehoogeveen-medweb commented 8 months ago

I don't know how hard it would be to check, but I wonder if https://github.com/jridgewell/source-map (being an independent implementation) errors on this too.

alexander-akait commented 7 months ago

Someone can create a reproducible example?

silverwind commented 7 months ago

In theory, it should reproduce with the CSS from https://github.com/swagger-api/swagger-ui.

romainmenke commented 7 months ago

Minimal repro focussed on mini-css-extract-plugin : https://github.com/romainmenke/mini-css-extract-plugin-repro

See what I did in the last commit : https://github.com/romainmenke/mini-css-extract-plugin-repro/commit/4d7f943da6388f2e18ca5d82a693015913d35206

If I comment out source.add("\n"); the projects builds and the resulting sourcemap can be consumed by source-map-js.

If I leave the code in the project still builds but the result sourcemap can not be consumed by source-map-js.

That does not mean that there is a problem in mini-css-extract-plugin. As I said before, I don't know enough about sourcemaps to be able to tell what is fine and what is not.

I also noticed that you need quite a lot of moving parts in webpack before it goes wrong. You need :

That I found a line of code that, when removed, makes the error go away doesn't tell us what is actually wrong.

romainmenke commented 7 months ago

I am also not that interested/invested in this issue :D

I use webpack, postcss, ... and help to maintain a bunch of the things involved, but I don't use these tools in this exact way and as far as I can tell the issue definitely isn't in a project that I help to maintain.

Is there a volunteer who wants to take over this issue and help maintainers with providing feedback, reproductions, ...

alexander-akait commented 7 months ago

@romainmenke Bug in postcss and cssnano, there are no problems in mini-css-extract-plugin, postcss doesn't generate required meta infromation in source maps for lines

alexander-akait commented 7 months ago

I can remove the line with extra \n. but it doesn't solve the problem it all, you will faced with it every time when other tool will add a new line

romainmenke commented 7 months ago

That makes sense, thank you for taking a look at this 🙇

I am not sure what the next steps are. Do we need to refactor how source maps are generated in PostCSS?


You can close this issue, thank you again :)

ai commented 7 months ago

@alexander-akait any idea what is wrong in PostCSS source map processing?

We found the issue when we started to use 1-to-1 source map https://github.com/postcss/postcss/blob/main/lib/no-work-result.js#L31C19-L39

romainmenke commented 7 months ago

@ai, this issue existed even before those changes. They were just much rarer because NoWorkResult could be easily avoided. When the bool checks were changed it became much more common to have a NoWorkResult.

romainmenke commented 6 months ago

Closing as a change was made for this in source-map-js

Thank you all for the feedback and insights

ai commented 6 months ago

The fix require updating PostCSS to 8.4.36 to use new option from source-map-js