Corrupt source map paths on Windows and invalid sources

[rjgotten]

Details

postcss-loader continues to produce invalid source map paths on Windows, after #224 was closed.
Said issue was pointing to css-loader as a culprit.
css-loader was fixed.
postcss-loader was not.

Reproduction (Code)

The following webpack loader sequence produces valid source map paths:

[
  MiniCssExtract.loader,
  "css-loader",
  "sass-loader"
]

The following webpack loader sequences produces INVALID paths:

[
  MiniCssExtract.loader,
  "css-loader",
  "postcss-loader",
  "sass-loader"
]

I have also written a small custom loader which passes through the content and sourcemap from the previous loader, while logging the sourcemap sources array with a simple console.log() call.

Placing said logger on top of sass-loader shows paths relative to the webpack project root. Placing said logger on top of postcss-loader shows the same type of corrupted paths as mentioned in #224.

Environment

OS	node	npm/yarn	package
Windows 10	8.9.x	6.1.x	3.0.0

[alexander-akait]

@rjgotten can you create minimum reproducible test repo?

[rjgotten]

@evilebottnawi
I'll try to work on that tomorrow.

[michael-ciniawsky]

@rjgotten Could you try with the following patch ?

https://github.com/postcss/postcss-loader/blob/master/src/index.js#L157-L159

if (map) {
-  map.file = path.isAbsolute(map.file) ? map.file : path.resolve(map.file)
+  map.file = path.isAbsolute(map.file) ? map.file : path.resolve(map.file)
-  map.sources = map.sources.map((src) => path.resolve(src))
+  map.sources = map.sources.map((src) => {
+     if (path.isAbsolute(src))
+        return src
+     }
+
+     return path.resolve(src)
+  })
}

[alexander-akait]

@michael-ciniawsky good catch! Don't think what source map can have absolute path 😕

[michael-ciniawsky]

This seems to be likely the offending line causing the issue on windows when the postcss-loader resolves the paths. I don't have access to a windows machine and therefore can't verify, but on Linux/macOS with and without the suggested patch sourcemaps seem to work correctly (besides using the style-loader but that's definitely another bug there, so try with e.g the mini-css-extract-plugin when testing)

map.json

{ 
  version: 3,
  sources: [
     '/Users/Cini/Github/webpack/__test__/client/styles/imports/import.css',
     '/Users/Cini/Github/webpack/__test__/client/styles/App.scss' 
  ],
  names: [],
  mappings: 'AAAA,yBAAA;ACAA;EACE,...',
  file: '/Users/Cini/Github/webpack/__test__/client/styles/App.scss',
  sourcesContent: [ '...' ] 
}

[alexander-akait]

@michael-ciniawsky what do you think about this https://github.com/webpack-contrib/sass-loader/blob/master/lib/loader.js#L70? Maybe we should fix it in sass-loader

[rjgotten]

@michael-ciniawsky
Could you try with the following patch ?

Sadly has no effect.

I'll start working on building a test case.

[rjgotten]

@evilebottnawi
Testcase is ready: https://github.com/NetMatch/postcss-loader-testcase

The build has a passthrough loader woven in which shows the source map sources going into and coming out of postcss loader.

What I'm seeing when running this on Windows:

Sourcemap spy - postcss-loader input:
  src\style\app.scss
  src\style\sub\a.scss
  src\style\sub\b.scss


Sourcemap spy - postcss-loader output:
  D:\github_git\postcss-loader-testcase\src\style\D:\github_git\postcss-loader-testcase\src\style\sub\a.scss
  D:\github_git\postcss-loader-testcase\src\style\D:\github_git\postcss-loader-testcase\src\style\sub\b.scss
  D:\github_git\postcss-loader-testcase\src\style\D:\github_git\postcss-loader-testcase\src\style\app.scss

[michael-ciniawsky]

Did you also try with commenting out these lines completely to see if the path.resolve in potcss-loader causes this weird concatination on windows? I wll try my best to debug it, but as mentioned before I don't have aceess to a windows machine so it is hard for me to get my hands on this :)

[rjgotten]

I don't have aceess to a windows machine

I think Microsoft offers VMs for developers, with licenses that auto-expire after a set period.
Might be a bit of a help in that department, if you're dealing with this type of thing.

Did you also try with commenting out these lines completely

Hmm.. I should've remembered to try that. Ok. I've made a note to try commenting out the path.resolve lines completely. Will get to that after the weekend.

[mutoo]

I once found there is a key function (util.join()) in source-map lib could probably cause this problem:
mozilla/source-map#355

but after discussing, I realized that the paths in the sources field should be URLs but the os paths, that why util.join() doesn't treat windows device path as an absolute path. so it generated the paths like this.

I read the code of the build pipeline: [css-loader, postcss-loader, sass-loader], then I got many bad idea in them:

sass-loader: normalize all the relative path in the sources field to os path, bad! since it should be URLs, should always use POSIX path separator (/) rather than os path separator (\ on windows).
postcss-loader: convert relative paths to os absolute paths (\), bad! see below.
css-loader: use whatever paths from the previous loader, but convert to POSIX path separator (/), so-so. should be responsible to convert the paths to relative paths, or better, the webpack-internal path.

Don't insert absolute paths into the module code as they break hashing when the root for the project is moved.
https://webpack.js.org/contribute/writing-a-loader/#absolute-paths

There will be much effort to fix the bug since it's related to 3 loaders & 1 lib, so I wrote a small loader to fix the sourcemap temporary in my projects. Which fix and convert the os absolute path to webpack-internal:// path
https://gist.github.com/mutoo/ad6dd6101535e82a44bcc2a051d7cb22

[rjgotten]

@mutoo

Imho all loaders should be producing source maps that are based on relative URIs from generated source to original source. Any loader that doesn't generate source maps in that way is - quite frankly - "doing it wrong" and should be fixed.

It should be Webpack's responsibility to remap those relative paths to the webpack://./ rooted form once it bundles everything into an entry-point or chunk output file.

[michael-ciniawsky]

I think it does that already, but later (after loaders) within a webpack (core) plugin (SourceMapDevToolPlugin) and this plugin needs absolute file paths to work correctly and it's seems to be also working on UNIX platforms (see #390 (comment)). The problem we are having are malformed paths on windows caused by some loader. Likely because the sass-loader is normalizing the file paths incorrectly atm. Once I get access to a windows machine I will debug it myself, but it may take a few days...

[rjgotten]

this plugin needs absolute file paths

And here's the likely root cause of all these loaders having bad implementations: the fact that Webpack requires intentionally malformed source maps to work 'correctly' to begin with.

If it all winds back to Webpack itself, then they need a good boot up the ass to fix this.

The fact that it "seems to be also working on UNIX platforms" is probably why this problem came to be so wide-spread and persistent in the first place. Odds are someone half-assed the core source mapping functionality and never bothered looking at it any further, because "works on my machine". i.e. it works under their *nix OS, where OS paths happily are close enough to URIs to be compatible. Not so with Windows, ofcourse.

Lord knows why Webpack's core source maps plugin requires absolute file paths. It has access to Webpack's own output paths as the generated source paths, which it should already be able to combine with relative paths in the source map just fine. And that should give it the absolute paths it needs to produce its project-relative webpack://./ source map entries.

[michael-ciniawsky]

https://github.com/webpack/webpack/blob/master/lib/SourceMapDevToolPlugin.js#L174-L223

I don't know the exact why behind it, webpack currently supports 12 different 'types' of sourcemaps flavours, some the world never heard about before :). Anyways this won't likely change in the near future and we should focus on finding the bug that causes the malformed paths instead :)

[rjgotten]

@michael-ciniawsky
Those 12 different types are various ways of outputting maps, and variations on including intermediate compilation/bundling stages into the maps or not.

Should have very little, if anything, to do with the types it can take in. Which should just be spec-adherent.

[alexander-akait]

Somebody can create minimum reproducible test repo?

[rjgotten]

Somebody can create minimum reproducible test repo?

You missed this, maybe?
#390 (comment)

[alexander-akait]

Very thanks, we work on refactor postcss-loader/css-loader (already refactored) and style-loader, in near future i will see how we can fix it 👍

[syberon]

I got this issue with invalid source-map paths in Windows too.

I use this loader chain:
**sass-loader** => **postcss-loader** => **css-loader** => **style-loader**

In generated map file i get this source file paths:
"sources":["\\\\/src\\\\css\\\\main.scss","main.scss","\\\\/src\\\\css\\\\bootstrap.scss\...

When i disable postcss-loader, then i get source map files with normal paths:
"sources":["/src/css/main.scss","main.scss","/src/css/bootstrap.scss"...

[alexander-akait]

@syberon can you create minimum reproducible test repo?

[rjgotten]

@syberon

The problem is caused by Webpack's build pipeline being entirely based on local filesystem paths, including paths used for source maps, whereas source maps by specification require valid URIs.

The official source maps reference implementation from Mozilla holds to the specification and will not treat \ separators from Windows local filesystem paths as path separators.

Webpack itself dodges that bullet because it rolls its own source mapping implementation, which plays fast-and-loose with the specification on this point. But PostCSS and everything that makes use of it -- that includes css- loader btw. as iirc it leverages it for some internal transformations -- are built on Mozilla's reference implementation and they will give you corrupt paths like the ones you are seeing.

The architecturally sound solution here is for Webpack to fix their broken non-compliant implementation, but they've been pathologically in denial about it being an issue for 2+ years.

So you're left with each individual loader that (even transitively) makes use of the reference implementation from the source-maps NPM package, having to convert the incoming sourcemap from absolute filesystem paths to relative URIs for its source-map manipulations to work as expected, and then convert the outgoing sourcemap back from relative URIs to absolute filesystem paths for Webpack to continue to be able to continue to do its job afterwards.

---

You can handle this transformation yourself externally, via a few custom loaders that manipulate the source maps. For your specific case:

1. Put a custom loader to execute before sass-loader which transforms webpack's incoming sourcemap to relative URIs. You have access to the current resource's absolute filesystem path as options.context so you can easily use path.relative to turn absolute source map paths into relative file system paths and from there it's a short skip to turn them into relative URI paths.
   (Swapping \ for / is most of it.)
2. Put another loader to execute directly after css-loader and before anything like MiniCssExtractPlugin or similar solutions to extract CSS back out. This loader will need to transform relative URI source map sources back to absolute local file system paths. Basically the reverse of the first loader. This step is somewhat complicated by the fact that css-loader outputs a JS module which embeds both the compiled CSS as well as its source map. So you'll need to parse the module into an AST; find the tokens that form the source map sources; transform the paths; then re-emit the AST to JS source code. esprima and escodegen packages may help there.

NOTE: You cannot transform the source map paths back before css-loader as it uses PostCSS (and thus Mozilla's reference source-maps) internally. You must transform it back afterwards which sadly requires sourcecode rewriting.

---

<rant>
And really; I would recommend you do handle it yourself, as bug reports regarding this issue go nowhere. Case in point: this issue has been open for longer than a year already...
It's not just Webpack itself that's dragging its feet. It's every other prominent official and officious loader as well. Basically; the entire toolchain is rotten from front-to-back when it comes to handling source maps on Windows. And I don't see it getting fixed anytime soon. (Or anytime ever for that matter.)
</rant>

[syberon]

@evilebottnawi
I created a test repo.
https://github.com/syberon/postcss-bug

Main problem is because of one backslash inserted in path (before src):
"sources":["\\\\/src\\\\css\\\\main.scss","main.scss","\\\\/src\\\\css\\\\bootstrap.scss\...

@rjgotten
For now as a temporary(or not?) solution i created a simple custom loader and put it in before css-loader. Loader only parses sources path list, correcting path and push it to css-loader:

const path = require('path');
module.exports = function( content, sourcemap ) {
	if ( sourcemap != null ) {
		let srcPath = path.resolve(__dirname + '/../');
		sourcemap.sources.forEach( (source, index) => {
			sourcemap.sources[index] = source.replace(srcPath, '');
		});
	}
	this.callback( null, content, sourcemap );
};

[rjgotten]

@syberon
Provided sass-loader has output comparable to less-loader, you'll want something like this inbetween sass-loader and postcss-loader :

const path = require( "path" );

module.exports = function( source, map ) {
  if ( map && map.sources ) {
    let context = this.context.replace( /\\/g, "/" );
    map.sources = map.sources.map( source => {
      if ( !path.isAbsolute( source )) return source;

      source = source.replace( /\\/g, "/" );
      return path.posix.relative( context, source );
    });
  }

  this.callback( null, source, map );
};

And then put something like the following between css-loader and style-loader:

const path = require( "path" );
const esprima = require( "esprima" );
const escodegen = require( "escodegen" );

module.exports = function( content ) {
  this.callback( null, adaptSources( content, this.context ));
};

function adaptSources( content, context ) {
  let ast = esprima.parseScript( content, { comment : true }, node => {
    if ( !isSourceMap( node )) return;

    getSourcesNodes( node ).forEach( node => {
      if ( !path.isAbsolute( node.value )) {
        node.value = path.resolve( context, node.value );
      }
    });
  });

  return escodegen.generate( ast );
}

function isSourceMap( node ) {
  if ( node.type !== "ObjectExpression" ) return false;

  let first = node.properties && node.properties[ 0 ];

  return (
       first != null
    && first.type === "Property"
    && first.key.type === "Literal"
    && first.key.value === "version"
  );
}

function getSourcesNodes( node ) {
  let prop = node.properties.find( prop => prop.key.value === "sources" );

  return prop && ( prop.value.type === "ArrayExpression" )
    ? prop.value.elements
    : [];
}

That should provide a more general purpose, robust solution workaround.

[alexander-akait]

Somebody can create reproducible test repo, we will update scss/less/css loaders in near future, so we can try to fix it

[syberon]

@evilebottnawi
I created a test repo.
https://github.com/syberon/postcss-bug

[alexander-akait]

@syberon big thanks, i will loon on this problem in near future ⭐

[Posnakidesd]

Hi. Is this issue resolved?

[alexander-akait]

Fixed in master, release will be soon - next week