Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong SourceMap genrated #434

Closed
zq1997 opened this issue Jul 22, 2020 · 22 comments
Closed

Wrong SourceMap genrated #434

zq1997 opened this issue Jul 22, 2020 · 22 comments

Comments

@zq1997
Copy link

zq1997 commented Jul 22, 2020

Details

Wrong SourceMap genrated

Reproduction (Code)

// webpack.config.js
...
entry : {
  test: './src/test.css',
}
plugins: [
  new webpack.SourceMapDevToolPlugin({
    filename: '[file].map',
    noSources: true,
    moduleFilenameTemplate: '[resource-path]'
  }
],
module: {
  rules: [
    {
        test: /\.css$/,
        use: [
          MiniCssExtractPlugin.loader,
          {
            loader: 'css-loader',
            options: {
              sourceMap: true
            }
          },
          Conf.cases(Conf.prod, {
            loader: 'postcss-loader',
            options: {
              plugins: [
                autoprefixer,
                cssnano
              ],
              sourceMap: true
            }
          })
        ]
    }
  ]
}
...
body {
    margin: 0;
}

generated sourcemap has wrong path test.css instead of ./src/test.css

{"version":3,"sources":["test.css"],"names":[],"mappings":"AAAA,KACI,QACJ,C","file":"test.css","sourceRoot":""}

Environment

OS node npm/yarn package
Ubuntu20.04.1 LTS 13.14.0 6.14.4 3.0.0
@alexander-akait
Copy link
Member

Bug on css-loader side and mini-css-extract-plugin, already in WIP, we need postcss@8 release to fix it

@zq1997
Copy link
Author

zq1997 commented Jul 22, 2020

But if I disable postcss loader,the problem goes away.
And if I append sass-loader after postcss, also problem free.
A little confuseing.

@alexander-akait
Copy link
Member

Oh, yep, I was misleading, it is bug in postcss, because postcss, don't know how generate absolute sources, let's keep open

@alexander-akait
Copy link
Member

@zq1997 maybe you can create minimum reproducible test repo, just want to add tests on this case?

@zq1997
Copy link
Author

zq1997 commented Jul 22, 2020

Just three reproducible files, (too lazy to create a repo)
package.json

{
  "name": "test",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "start": "webpack -c webpack.config.js"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "css-loader": "^3.6.0",
    "mini-css-extract-plugin": "^0.9.0",
    "postcss-loader": "^3.0.0",
    "sass": "^1.26.10",
    "sass-loader": "^9.0.2",
    "webpack": "^4.43.0",
    "webpack-cli": "^3.3.12"
  }
}

webpack.config.js

const webpack = require('webpack');
const MiniCssExtractPlugin = require('mini-css-extract-plugin');

module.exports = {
  mode: "development",
  entry : {
    test: './src/test.css',
  },
  devtool: 'source-map',
  plugins: [
    new MiniCssExtractPlugin(),
  ],
  module: {
    rules: [
      {
        test: /\.css$/,
        use: [
          MiniCssExtractPlugin.loader,
          {
            loader: 'css-loader',
            options: {
              sourceMap: true
            }
          },
          {
            loader: 'postcss-loader',
            options: {
              plugins: [],
              sourceMap: true
            }
          },
          // enable sass-loader will make source-map work properly.
          // "sass-loader"
        ]
      }
    ]
  }
}

src/test.css

body {
    margin: 0;
}

Then

npm start
cat dist/test.css.map

GOT

{"version":3,"sources":["webpack:///test.css"],"names":[],"mappings":"AAAA;IACI,SAAS;AACb","file":"test.css","sourcesContent":["body {\n    margin: 0;\n}\n"],"sourceRoot":""}

with sass-loader

{"version":3,"sources":["webpack:///./src/test.css","webpack:///test.css"],"names":[],"mappings":"AAAA;EACI;ACCJ,C","file":"test.css","sourcesContent":["body {\n    margin: 0;\n}\n","body {\n  margin: 0;\n}"],"sourceRoot":""}

the differnces is directory name ./src

@zq1997
Copy link
Author

zq1997 commented Jul 22, 2020

Ops, these files cannot reproduce But if I disable postcss loader,the problem goes away., sorry but my origin repository is too complex...
maybe we can focus on And if I append sass-loader after postcss, also problem free. this problem, its reproduced here.

@alexander-akait
Copy link
Member

Thanks!

@zq1997
Copy link
Author

zq1997 commented Jul 24, 2020

I wrote an no-op loader to view the sourcemap changes

module.exports = function(source, map, meta) {
  console.log(map);
  this.callback(null,source,map,meta);
}

apply the loader

          path.resolve('./nop-loader.js'),
          {
            loader: 'postcss-loader',
            options: {
              plugins: [],
              sourceMap: true
            }
          },
          path.resolve('./nop-loader.js'),
          // enable sass-loader will make source-map work properly.
          "sass-loader"

The result make me a little confused, why there is two sources? and relative path became absolute

{
  version: 3,
  sourceRoot: '/home/zq/workspace/try',
  sources: [ 'src/test.css' ],
  names: [],
  mappings: 'AAAA;EACI',
  sourcesContent: [ 'body {\n    margin: 0;\n}\n' ]
}
You did not set any plugins, parser, or stringifier. Right now, PostCSS does nothing. Pick plugins for your case on https://www.postcss.parts/ and use them in postcss.config.js.
{
  version: 3,
  sources: [
    '/home/zq/workspace/try/src/test.css',
    '/home/zq/workspace/try/src/test.css'
  ],
  names: [],
  mappings: 'AAAA;EACI,SAAA;ACCJ',
  file: '/home/zq/workspace/try/src/test.css',
  sourcesContent: [ 'body {\n    margin: 0;\n}\n', 'body {\n  margin: 0;\n}' ]
}

@zq1997
Copy link
Author

zq1997 commented Jul 24, 2020

My original project is writen in Vue, I also print the sourcemap result of ./src/app.vue

============= before postcss-loader (Without sass-loader)
{
  version: 3,
  sources: [ 'app.vue' ],
  names: [],
  mappings: ';;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;AAuDA;AACA;AACA',
  file: 'app.vue',
  sourceRoot: 'src',
  sourcesContent: [ ... ]
} undefined
============= after postcss-loader
{
  version: 3,
  sources: [ '/home/zq/workspace/TryVuetifySSR/src/src/app.vue' ],
  names: [],
  mappings: 'AAuDA,WACA,QACA',
  file: '/home/zq/workspace/TryVuetifySSR/src/app.vue',
  sourcesContent: [ ... ]
}

the absolute path is wrong becuase .../src/src/... is not any directory, seems to be a wrong path concatation

@alexander-akait
Copy link
Member

alexander-akait commented Jul 24, 2020

Bug here, it is invalid use path.resolve, because we don't know context https://github.com/postcss/postcss-loader/blob/master/src/index.js#L157

@zq1997
Copy link
Author

zq1997 commented Jul 24, 2020

My original project is writen in Vue, I also print the sourcemap result of ./src/app.vue

============= before postcss-loader (Without sass-loader)
{
  version: 3,
  sources: [ 'app.vue' ],
  names: [],
  mappings: ';;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;AAuDA;AACA;AACA',
  file: 'app.vue',
  sourceRoot: 'src',
  sourcesContent: [ ... ]
} undefined
============= after postcss-loader
{
  version: 3,
  sources: [ '/home/zq/workspace/TryVuetifySSR/src/src/app.vue' ],
  names: [],
  mappings: 'AAuDA,WACA,QACA',
  file: '/home/zq/workspace/TryVuetifySSR/src/app.vue',
  sourcesContent: [ ... ]
}

the absolute path is wrong becuase .../src/src/... is not any directory, seems to be a wrong path concatation

And this problem is caused by https://github.com/postcss/postcss-loader/blob/master/src/index.js#L141 line. postcss return source map with src/src path

@alexander-akait
Copy link
Member

Yes, we need fix it

@alexander-akait
Copy link
Member

alexander-akait commented Aug 14, 2020

Let's close in favor #390, we need postcss@8 release to fix it, title of the issue was updated

@rjgotten
Copy link

rjgotten commented Sep 7, 2020

@evilebottnawi
Oh, yep, I was misleading, it is bug in postcss, because postcss, don't know how generate absolute sources, let's keep open

The source maps specification only supports relative or absolute URLs in the sources collection:

Resolving Sources

If the sources are not absolute URLs after prepending of the “sourceRoot”, the sources are resolved relative to the SourceMap (like resolving script src in a html document).

-- https://sourcemaps.info/spec.html#h.75yo6yoyk7x5

An absolute URL means an actual fully qualified http(s)://<domain>/<path>. PostCSS can never know how to generate an absolute URL because it's called with a file from disk and has no knowledge of where that file fits in the grand scheme of things, or where it will be deployed. Generating a URI that is relative to the original input file is the correct solution.

It should be Webpack's responsibility to transform that properly.
Webpack has everything available to do so:

  • It has a given root context carrying the disk path where the webpack compilation is rooted.
  • It has the current asset context, carrying the disk path where the asset was originally located.
  • It can compute the relative disk path between the two.
  • It can interpret that as a relative URI.
  • It can combine that with the generated source map's relative URI(s) to obtain final sources that are relative URIs to the compilation root.
  • It can apply those final relative URI as the path for the webpack:// protocol, or whatever else the user has set in their configuration.

And done.

The issue with ALL these loaders for CSS, PostCSS, Less and SASS is not with the third party tooling.
It lies squarely with Webpack and these loaders itself, which produce and expect to consume local disk paths as sources in source maps. And that's 100% a violation of spec.

@alexander-akait
Copy link
Member

alexander-akait commented Sep 7, 2020

@rjgotten it is job of css-loader, other loaders should emit absolute path (not URLs) in sources (fixed in sass-loader/less-loader/postcss-loader, WIP in stylus-loader), please look at source code if css-loader https://github.com/webpack-contrib/css-loader/blob/master/src/utils.js#L414

@alexander-akait
Copy link
Member

And that's 100% a violation of spec.

Yes, sources should be URL, but no one tool supports merging sources maps using file:// protocol for 100% supporting spec in sources, so we use path (not URL) in sources right now

@alexander-akait
Copy link
Member

If you have a problems right now please create reproducible test repo, note - postcss-loader is not published yet with fix, so you can have corrupted sources right now

@rjgotten
Copy link

rjgotten commented Sep 7, 2020

no one tool supports merging sources maps using file:// protocol for 100% supporting spec in sources

No tool would ever need to.

According to spec, a source map's sources property can be a relative URL, in which case it's relative to the file for which it was generated. In case of a Webpack compilation, that file has a disk path which is known to Webpack. Webpack knows the disk path to the compilation root as well. So you can create a relative path from root-to-file, which can be straight-up interpreted as a relative URI. If you prefix that to the URLs in sources URIs and then publish that as the new source map, then you have relative URLs; relative to Webpack's compilation root.

Once Webpack finishes combining them, you still have relative URLs in the sourcemap and the only thing Webpack has to do is populate the final combined source map's sourceRoot field with the protocol segment that defaults to webpack://, because if a URL in sources is relative, the final computed URL has sourceRoot applied (when available) to make it absolute.

The only issue that exists with this, are pre-generated source maps that ship with absolute URLs. The answer to that property of the problem is simple: just pass them through. Let them flow through unaltered and they'll end up 'as is' in the final source map. The behavior for resolving sources is by specification such that if an entry in sources is already an absolute URL, then sourceRoot won't be applied to it.

If a pre-generated source map has an absolute URL in its sources, then that's how it was meant to be by the author(s) and the default should be to respect that. If users want to fix that part, then they can work around it with a loader which converts those offending source maps' sources into relative URLs on a case-by-case basis, choosing the paths that they want by hand.

If you have a problems right now

I have problems with you insinuating that PostCSS itself has an actual bug here which needs to be patched. When clearly it's Webpack and its loaders which have ass-backwards behavior. And on a wider scale I have issues with Webpack foregoing to fix the root of the problem and instead choosing for a litany of workarounds at the individual loader level; to convert valid source maps into the intentionally invalid source maps -- 'pseudo source maps' ? -- that Webpack expects.

If you implement support for a standard, damn well implement support for the standard. Don't cook your own idea of it.
You are not 1990s era Microsoft.

@alexander-akait
Copy link
Member

alexander-akait commented Sep 7, 2020

@rjgotten

No tool would ever need to.

You just said that sources should be URL, but sass-loader/less-loader and other loaders emit \ on windows, isn't this a violation of the specification?

I don't want to be involved in a toxic non-constructive talk.

I asked you to look at the code of css-loader. Also, if you were attentive, you would notice that babel/typescript/coffee tools emit absolute sources using Node API, so they are all fools too. Constantly rewriting relative sources with root context creates more code and reduce performance.

@alexander-akait
Copy link
Member

If you implement support for a standard, damn well implement support for the standard. Don't cook your own idea of it.
You are not 1990s era Microsoft.

Apparently since that time, many still have not learned to communicate

@alexander-akait
Copy link
Member

alexander-akait commented Sep 7, 2020

I have problems with you insinuating that PostCSS itself has an actual bug here which needs to be patched. When clearly it's Webpack and its loaders which have ass-backwards behavior. And on a wider scale I have issues with Webpack foregoing to fix the root of the problem and instead choosing for a litany of workarounds at the individual loader level; to convert valid source maps into the intentionally invalid source maps -- 'pseudo source maps' ? -- that Webpack expects.

postcss/postcss#1348
postcss/postcss#1347
postcss/postcss#1352

Need more links?

@alexander-akait
Copy link
Member

alexander-akait commented Sep 7, 2020

If a pre-generated source map has an absolute URL in its sources, then that's how it was meant to be by the author(s) and the default should be to respect that. If users want to fix that part, then they can work around it with a loader which converts those offending source maps' sources into relative URLs on a case-by-case basis, choosing the paths that they want by hand.

Can you just explain your problem? We using absolute sources between loaders (to avoid constantly rewriting this in each loader), library authors should still use relative URLs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants