webpack-contrib / postcss-loader

PostCSS loader for webpack
MIT License
2.86k stars 211 forks source link

LESS - Sourcemaps #15

Closed jantimon closed 9 years ago

jantimon commented 9 years ago

Is it possible to consume a sourcemap like the css-loader does?

Without postcss-loader I have to following:

{ test: /\.less$/, loader: ExtractTextPlugin.extract('style-loader', 'css?sourceMap!less?sourceMap) },

Now when I add the postcss-loader it looks like:

{ test: /\.less$/, loader: ExtractTextPlugin.extract('style-loader', 'css?sourceMap!postcss?sourceMap!less?sourceMap) },

Now the source map is broken and doesn't follow @import rules anymore.

ai commented 9 years ago

There is no need to use ?sourceMap, source mas should load automatically.

jantimon commented 9 years ago

Thanks I tried that however the bug is still there.

a.less

@import 'b.less';

b.less

body {background: red}

When inspecting the body element I get different results:

Result without postcss-loader: b.less
Result with postcss-loader: a.less (which is obviously wrong)

ai commented 9 years ago

@sokra maybe we can move postcss-loader to webpack support? I didn’t use it and it is very difficult to fix complicated issues :(.

ai commented 9 years ago

@jantimon can you create some small playground with this issue?

sokra commented 9 years ago

Maybe @markdalgleish like to maintain it...

jantimon commented 9 years ago

@ai here you go: http://we.tl/niZkeyeQrl

npm i
npm run dev

Inspect the h1 - you will see that the mapping for the color attribute does not work but the mapping for the font weight does.

ai commented 9 years ago

@markdalgleish =^_^=

ai commented 9 years ago

I test your case, PostCSS correctly receives previous map, so there is no need for ?sourceMap parameter.

ai commented 9 years ago

Hm. Seems like as strange case of Less/webpack/PostCSS conflict. Right not it will not be a important issue :).

jantimon commented 9 years ago

So postcss won't work with less?

jantimon commented 9 years ago

Just compared the two source maps post css receives:

Less (as described before the example did not work with less)

{
  "sources" : [
    "b.less"
  ],
  "mappings" : "AAAA;EACE,aAAA",
  "sourcesContent" : [
    "h2 {\n  color: orange;\n}"
  ],
  "names" : [

  ],
  "version" : 3
}

Scss (the same example works with scss)

{
  "sources" : [
    "example\/with-postcss\/a.scss",
    "example\/with-postcss\/b.scss"
  ],
  "sourcesContent" : [

  ],
  "mappings" : "ACAA,EAAE,CAAC;EACD,KAAK,EAAE,MAAO,GADZ",
  "names" : [

  ],
  "file" : "example\/with-postcss\/a.scss",
  "version" : 3
}

Seems like postcss needs more information - is this a post css or less bug?

ai commented 9 years ago

If Less send a source with only one file it is definitely Less bug.

jantimon commented 9 years ago

But the css-loader can handle it correctly

jantimon commented 9 years ago

Just took a look at what post-css does with the less map.

{
  "sources" : [
    "b.less"
  ],
  "mappings" : "AAAA;EACE,aAAA",
  "sourcesContent" : [
    "h2 {\n  color: orange;\n}"
  ],
  "names" : [],
  "version" : 3
}

becomes

 { version: 3,
  sources: [ 'a.less' ],
  names: [],
  mappings: 'AAAA;EACE,cAAc;CACf',
  file: 'a.less',
  sourcesContent: [ 'h2 {\n  color: orange;\n}\n' ] 
}

As you can see postcss changes the sources from 'b.less' to 'a.less'

ai commented 9 years ago

What file was main and what was imported?

ai commented 9 years ago

BTW, very strange, that Less file doesn't have file field.

jantimon commented 9 years ago

a.less is the file which imports b.less.
As post-css throws away the information about b.less the result is wrong.

ai commented 9 years ago

Can you add a file field to Less map and see will it fix a problem?

jantimon commented 9 years ago

Maybe I mixed it up with other debug tests but it seems like the following change would fix the issue:

if (typeof map === 'string') {
  map = JSON.parse(map);
}
jantimon commented 9 years ago

Hm actually that really seems to fix the issue - do you want me to file a merge request?

I guess the reason is this line: https://github.com/postcss/postcss-loader/blob/master/index.js#L17

It won't work if the map is a string.

ai commented 9 years ago

@jantimon awesome! Let’s my find out why string maps doesn’t work in PostCSS core. Maybe PR to core is a better solution.

jantimon commented 9 years ago

Only a guess but it's probably a loader bug:

from https://github.com/postcss/postcss-loader/blob/master/index.js#L17

if ( map && map.mappings ) opts.map.prev = map;

Will be false if map is a string.

ai commented 9 years ago

Oh, exactly

ai commented 9 years ago

Fix released in 0.5.1.

Thanks for doing all job by this task :D.

jantimon commented 9 years ago

Cool thanks :)

markdalgleish commented 9 years ago

@ai Did we still want to move this project to the webpack org?

ai commented 9 years ago

@markdalgleish yeap :) I am bad maintainer for webpack projects =^_^=