PostCSS doesn't update SourceMaps properly when an input file is in a subdirectory about postcss

[lydell]

I ran into the same issue when I developed my fork of Autoprefixer with source maps support. Here is how I solved the problem and the relevant test. If any of that helps ;) (I must admit that is not the most well-written code I've produced -.-)

I wonder if the source-map module could be smarter about this by default, though ...

[ai]

@lydell thanks, it will be very helpful :).

[ai]

Maybe we should commit to source-map? After few days I will start to fix this issue and I try to find how it will be difficult.

[lydell]

I think it has to do with that the .applySourcemap() method form the source-map module only works correctly if items of the sources array of source maps are exactly equal: If the in-sourcemap has sources: ["folder/foo.css"] and the out-sourcemap has sources: ["foo.css"] the applySourcemap() method does not understand that it's actually the same foo.css. So I used a lot of path.relative() and such to normalize the sources arrays first. Hopefully the source-map could do this, that would be the best for everyone of course.

[ai]

There is problem, that path.relative() only in Node.js :(.

But we can use some smart autodetect and didn’t use this method in browsers.

[nDmitry]

@ai what about browserify shim?

[ai]

You are cool guys :). Thanks, that is what I need.

Hopy, that mozilla team would like to add new dependencie :).

[nDmitry]

Проблему с обновлением mappings в grunt-autoprefixer обошел переходом в папку from, чтобы input (from) путь состоял только из имени файла — https://github.com/nDmitry/grunt-autoprefixer/blob/master/tasks/autoprefixer.js#L66:L69

Короче, в переводе на консольный интерфейс из этого: autoprefixer -m in/a.css -o out/b.css я сделал это: cd in && autoprefixer -m a.css -o ../out/b.css.

В этом случае mappings при имеющейся входящей карте обновляются корректно, хотя в file и sources тогда пути всё равно неправильные, поэтому их тоже пришлось указывать вручную: https://github.com/nDmitry/grunt-autoprefixer/blob/master/tasks/autoprefixer.js#L47:L55

[lydell]

Perhaps the second argument of .applySourcemap() should be used?

[lydell]

I tried changing the .applySourceMap() line to map.applySourceMap(prev, opts.from) and that partially solved the problem.

$ cat b.css.map 
{"version":3,"file":"b.css","sources":["a.sass"],"names":[],"mappings":"..."}

$ cat out/b.css.map 
{"version":3,"file":"out/b.css","sources":["a.sass"],"names":[],"mappings":"..."}

As you can see, the source is now changed to "a.sass", but the path to "a.sass" should be relative to the source map (as @nDmitry’s expected output shows).

This is how I understand the problem:

1. Autoprefixer takes a source file as input. In this case, "in/a.css".
2. Autoprefixer puts the source file in the sources array of the source map it creates: "sources": ["in/a.css"].
3. Autoprefixer finds that "in/a.css" has a source map. It is a generated file, and therefore we don’t want to map to it, but to it’s source.
4. Autoprefixer applies the source map for "in/a.css" ("in/a.css.map"), by using the .applySourcemap() method. That method works like this: "Hey, source map [the source map generated by Autoprefixer]. You contain a source that itself has a source map. For that source, map to the locations of that source map instead!". So the .applySourcemap() methods needs two things: The name of the source that itself has a source map (the second parameter), and the source map for that source (the first parameter). However, the method allows the second parameter to be omitted.

   • If the second parameter is omitted, the method tries to guess which source the source map to be applied belongs to, by looking at the file property. In my opinion, that’s not a good guess. The file property is optional, so it may be missing. Moreover, the spec just says that it is "a name of the generated code that this source map is associated with." It doesn’t say that it is the file name, or, more importantly, a path to the file. And as far as I know, browsers ignore this property.

     Still, the file property is used in this case. "in/a.css.map" contains "file": "a.css". So the method looks in Autoprefixer’s source map’s sources property for a file called "a.css". However, it only contains "in/a.css", so it cannot find anything. So, mission complete. There’s nothing for the method to do.

   • If the second parameter is not omitted, that parameter tells the method which source file the source map to be applied belongs to. If we send in "in/a.css" (opts.from) it works better. Now the method looks in Autoprefixer’s source map’s sources property for a file called "in/a.css". This time it find such a source. It rewrites the mappings for that source, and then replace "in/a.css" with "a.sass". However, it should have rewritten the path to "a.sass" to be relative to Autoprefixer’s source map, but unfortunately it didn’t. So I think this is a bug in the source-map module. When I think about it, it is not so strange. The .applySourcemap() hasn’t got all the information it needs. It also needs a path to the source map to be applied, relative to the master source map. In our case, it needs the path to "in/a.css.map" relative to Autoprefixer’s source map. In the first case: "in/a.css.map". In the second case: "../in/a.css.map".

So, here is what I think needs to be done:

1. Postcss should provide the second parameter to the .applySourcemap() method (as I tested).
2. The .applySourcemap() method should take a third parameter (as I described above). Moreover, I think that it should deprecate the use of just one or two parameters. Three should always be provided.
3. Postcss should provide the third parameter to the .applySourcemap() method.

/cc @fitzgen

[ai]

@lydell you are awesome :D thanks for good investigation.

[lydell]

Here is the test for .applySourceMap(). It does not cover sub-directories and the usage of the second parameter. That should be looked into as well.

[fitzgen]

Assuming that there's tests, docs, and nice code, I'll accept a PR that adds the extra parameter to applySourceMap :)

[ai]

@lydell you tell about take file from previus map (and of cource this idea is wrong, becuase it is option file), but why whe can’t change file option in previus map?

Can you please check my fix: a2e2652

It is seems to simple to me, maybe there are some side effects?

[ai]

/cc @nDmitry @fitzgen

[lydell]

That looks exactly like @map.applySourceMap(prev, @opts.from) to me. I'll look more at it asap hopefully tomorrow.

[ai]

@lydell second argument for applySourceMap don’t solve all problems, because file and sources in generated map will not be relative from map file?

[lydell]

As far as I understand, your fix and using the second argument are equivalent. They both partially solve the problem: The .applySourceMap() method then correctly understand which source the source map to apply. belongs to. But it cannot know how to rewrite any sources from the source map to be applied relative to the new source map, so their paths will be wrong. (I could be wrong, though, because I couldn't test it when I wrote this).

Last Friday I started working on improving the .applySourceMap() method. However, I hadn't time to finish it and send a pull request. Hopefully I can do that tomorrow.

[ai]

@lydell does I understand correct, that next problem is that we use apsolute path in map, but must use paths relative from map file?

I try to fix it in MapGenerator hack.

[lydell]

Source maps are free to use both absolute and relative paths. If they are relative, they should be relative to the source map itself. That's the problem. When we apply the previous map, the sources paths of that map will be put in our new map. But they will still be relative to the previous map. That's a bug in the .applySourceMap() method. Which I'm working on. No hack should be needed.

[nDmitry]

@ai just do what I did in grunt-autoprefixer or wait for the applySourceMap() fix.

1. AFAIK, file property should be a name of a generated file, not a path. Check this out. Mozilla's lib doesn't work either without the property or if it's incorrect, so we should provide it if there isn't one in an input map. I use my function ensureFile() twice—on an input map to ensure that the property exists and it is correct, and once more when I get an output map (because source-map puts there a path).
2. As lydell said, sources is a mess. I just take paths from an input map and make them relative to the output dir.

Also I don't want to look at all that CoffeeScript (sorry, it makes me sick =)), but make sure that your tests actually test mappings.

[ai]

@nDmitry don’t be so cruel with CoffeeScript ;). There are a lot of execulent software on Ruby and Python with same code style :).

@lydell I think to take now path hacks from grunt-autoprefixer and clean them when source-map will be fixed right. Users too wait for 1.1 release and now I prefer hotfix :).

[lydell]

Sounds sensible 👍

[ai]

@nDmitry @lydell I think I have done with relative paths :). Can you please check last 2 tests.

[lydell]

Looks good!

Good idea to hack fix this for the time being. I did get time to work on my patch yesterday, but I ran into several unexpected problems.

[lydell]

mozilla/source-map#93