source-map-inline-sources uses Windows paths on Windows

[XhmikosR]

I was running npm run dist on Bootstrap v4-dev branch and I notice that the map files have the paths with Windows separator. I don't think that's right.

{"version":3,"sources":["../../scss/bootstrap.scss","../../scss/_print.scss","dist/css/bootstrap.css","../../scss/_reboot.scss"

vs this on Windows

{"version":3,"sources":["..\\..\\scss\\bootstrap.scss","..\\..\\scss\\_print.scss","dist\\css\\bootstrap.css","..\\..\\scss\\_reboot.scss"

/CC @jakubpawlowicz

Using v4.1.7.

[XhmikosR]

OK, this might not be an issue, but is it really needed to use Windows separator?

[jakubpawlowicz]

You built it on Windows, right? I wonder how such source map is interpreted by a non-Windows browser, e.g. Chrome on Mac.

[XhmikosR]

Yeah.

That is why I think always using the nix separator is better.

[jakubpawlowicz]

I think we already force UNIX path separator to URLs.

[XhmikosR]

Then it would make sense to do the same here too? I don't seem to have any issues with the unix separator on Windows for sourcemaps.

[jakubpawlowicz]

Worth making it simpler. Scheduling for 4.2.

[XhmikosR]

@jakubpawlowicz: so any news about this? It makes collaboration with people on different OS'es than I am hard and messy. :/

[jakubpawlowicz]

For backwards compatibility I'd rather add it as an option defaulting to sth like { lineBreak: 'system' } where user can override it with 'unix', 'windows', or 'lf', 'crlf', also see #238

[XhmikosR]

I don't think the current behavior is right but I haven't read the spec. If the specs allow Windows paths then what you suggest makes sense, otherwise this should be considered a bug and just fixed.

#238 makes sense, totally. Usually I don't hit this because I try to set LF on the repo side plus the CSS are usually one line so no line ending :P

[XhmikosR]

@jakubpawlowicz: can you please take care of this? I'm pretty sure the current behavior is non-standard even if it works.

[jakubpawlowicz]

Sure, we can have a switch in 4.2 as described above. Then default it to unix from v5. Does it sound right?

[XhmikosR]

Sounds good, although, I don't see why one would even need the paths to be Windows-like, when clearly, using unix works everywhere...

[jakubpawlowicz]

Me neither but I work with *nix systems only so can't tell if there's a use case for CRLF.

[XhmikosR]

I use Windows machines, and I see no use case for this. Browsers work just fine with unix separators.

[XhmikosR]

I mean, it's like with URLs; both might work, but what do we use?

I can't find anything in the specs so I guess this should be handled like URLs.

Thus, this isn't a feature request, rather a bug report and no feature should be added; the original behavior should be changed to use unix paths.

[XhmikosR]

@jakubpawlowicz: any news?

[mvelosop]

Hi @jakubpawlowicz, just to add one more upvote to this!

And I've also found that on Windows the minification adds a CRLF in the packed line:

but the comment lines are ok:

[XhmikosR]

@jakubpawlowicz: ping

[XhmikosR]

@jakubpawlowicz: this issue was about the path separator though. Is this addressed in 4.2.0 too?

How can I specify the format option to enforce \n via the CLI?

Thanks!

[jakubpawlowicz]

Ooops, I somehow focused on the breaksWith. Will add an option to configure that too.

Re CLI you can use cleancss --format breaksWith=lf or cleancss --format breaksWith=unix. Just added it to CLI readme.

[XhmikosR]

Thanks for the help!

Yeah, the path separator is still an issue with 4.2.0.

[XhmikosR]

@jakubpawlowicz: the line endings issue isn't completely fixed for me. I still get a CRLF before the sourceMappingURL line.

[jakubpawlowicz]

Do you use CLI or some sort of 3rd party library on top of clean-css?

[XhmikosR]

@jakubpawlowicz: nope. https://github.com/twbs/bootstrap/blob/v4-dev/package.json#L36

[jakubpawlowicz]

It's in our CLI so it has to be fixed there - that's the line: https://github.com/jakubpawlowicz/clean-css-cli/blob/master/index.js#L249

[XhmikosR]

Yeah, but I'm still seeing one CRLF line just before the sourceMappingURL locally on Windows if I just do npm run css-minify

[XhmikosR]

@jakubpawlowicz: yeah that line is wrong. It shouldn't be OS dependent. It should take into consideration the breaksWith option.

[jakubpawlowicz]

^ is fixed in CLI 4.2.1

[XhmikosR]

@jakubpawlowicz: definitely not fixed.

[XhmikosR]

I made an issue in the CLI repo about the line endings issue clean-css/clean-css-cli#26

[XhmikosR]

@jakubpawlowicz: can you please tackle this issue along with clean-css/clean-css-cli#26? Without these 2 fixed I can't make a dist commit on Bootstrap.

And I don't want to switch to something else, well, because I like clean-css and as you know I'm using it since the beginning. :)

[XhmikosR]

@Johann-S: any chance you could have a look? It doesn't seem there's enough interest to fix this and it definitely affects us.

[seowalex]

Would love some news on this as well.

[XhmikosR]

@jakubpawlowicz so about this is issue, which is still present on master. I believe you should always use Linux path separators in sourcemaps. If this issue is solved, then only the linebreak issue is left to fix (clean-css/clean-css-cli#26 but probably here too) and we should be able to get reproducible builds regardless of OS.

[jakubpawlowicz]

Won't fix as clean-css is going into maintenance mode.

[XhmikosR]

That's a pity because this is a real bug.

[jakubpawlowicz]

PRs are welcomed!

[XhmikosR]

Well, if you recall I tried to solve the issue but never managed to do it properly: #1070

It's the same issue here.