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

Impossible to generate an absolute source map "sources" #1352

Closed
alexander-akait opened this issue Apr 23, 2020 · 12 comments
Closed

Impossible to generate an absolute source map "sources" #1352

alexander-akait opened this issue Apr 23, 2020 · 12 comments
Labels
Milestone

Comments

@alexander-akait
Copy link

alexander-akait commented Apr 23, 2020

We are currently experiencing problems with generating absolute "source" in source maps. And there is no solution. We use postcss as a parser for css in webpack (css-loader/postcss-loader/other loaders use postcss).

Why we need absolute source maps?

  1. Custom source map generation

Each loader should return absolute sources (just note - babel-loader/ts-loader generate absolute urls in sources by default). This allows us to allow developers to customize source maps, for example:

new webpack.SourceMapDevToolPlugin({
  // Keep source maps in separate directory
  // Very useful for projects which tracks errors using `sentry`
  // Allowed to request files from `sourcemaps` directory only with allowed IPs
  // It is simple case
  filename: 'sourcemaps/[file].map',
  publicPath: 'https://example.com/project/',
  fileContext: 'public'
});
  1. Hard to merge source maps between other css like loaders.

Looks the same problem - #1348.

For example when you use sass-loader/less-loader and postcss-loader, it really makes it almost impossible to merge source maps from other loaders/tools. We have to rely on cwd, but it is wrong, monorepos suffer from this - webpack-contrib/css-loader#1064.

  1. We need a prefix.

Since the original maps may intersect with the existing source map (not from webpack), sources may intersect, for example part of css I generated uses postcss, other part generated using css-loader/postcss-loader, to avoiding this we uses webpack:// prefix for sources in source maps from webpack. But without an absolute path in sources, we cannot do it right, we don't know sourceRoot.

It is worth saying that perhaps we could fix this without global changes (breaking changes) if postcss output sourceRoot in source maps.

Recently, we get quite a lot of problems with this and it would be nice not assign it to 8 version and fix it in 7 version. Yep, I can send a PR 👍

If required, I can provide a reproducible examples, but I think the ideas/problems here are pretty simple.

Ideally if postcss@8 generated absolute sources by default if you don't provide to option.


I noticed another strange behavior, I would consider it as a bug:

postcss(plugins)
    .process(content, {
      from: this.resourcePath,
      to: '/'
      map: {  
        prev: map,
        inline: false,
        annotation: false,
      },
    })

Generate sources like (no / before sources and it is expected):

{
  sources: [ 'home/name/projects/css-loader/test/fixtures/source-map/nested/nested.postcss.css' ],
}

But:

postcss(plugins)
    .process(content, {
      from: this.resourcePath,
      //  Empty string
      to: ''
      map: {  
        prev: map,
        inline: false,
        annotation: false,
      },
    })

Generate sources like (it is not absolute sources and it is unexpected):

{
  sources: [ 'nested.postcss.css' ],
}

Looks we doesn't respect '' (empty string) and consider it as falsy value.

And yes, sources are urls, but some tools (including postcss) generate \ slashes on windows, but it is invalid, I think this may be a problem when you will merge sources on windows from other tools which always generate / slashes, but we can add tests for it.

/cc @ai I would like to hear your opinion on this, we are preparing a big update for CSS pipelines in webpack, and broken source map is blocker for us. It is also worth saying that such problems are experiencing with other bundlers like rollup and parcel.

@ai
Copy link
Member

ai commented Apr 23, 2020

  1. Are you talking about absolute paths in sources?
  2. What do you think about URL in sources? Source map "sources" array should be treated as URLs, not paths #1347

@alexander-akait
Copy link
Author

Are you talking about absolute paths in sources?

Yes, at the moment it is impossible to get them

What do you think about URL in sources? #1347

We should support them, but there is a problem here that some packages still generate not URLs, so we should take attention on this.

Possible fixes for v7:

  • to: '' should consider as not falsy value and generate absolute URLs (very simple solution)
  • always return source maps with sourceRoot, will allow us to transform relative sources to absolute sources (also consider sourceRoot from other tools like sass/less` when we merge source maps)
  • implement a new options for source maps for generate absolute sources (but the less options are better, I don’t really like this solution)

@ai
Copy link
Member

ai commented Apr 23, 2020

Yeap, we can’t generate absolute paths by default, but it will be nice to have option

@ai ai added the API label Apr 23, 2020
@ai ai added this to the 8.0 milestone Apr 23, 2020
@alexander-akait
Copy link
Author

@ai How about fixing this for v7, this is a real pain, especial for monorepos

@alexander-akait
Copy link
Author

alexander-akait commented Apr 23, 2020

It may take a long time when v8 was released, but problems exist right now.

@ai
Copy link
Member

ai commented Apr 23, 2020

What will be the smallest fix for the webpack team?

Adding sourceRoot to the map file?

@alexander-akait
Copy link
Author

alexander-akait commented Apr 24, 2020

@ai Yes, adding sourceRoot will allow to us to implement workaround

What do you think generate absolute sources by default for v8 (as I said early, babel/ts do it by default)


I would like to add tests to make sure that postcss merges source maps fine when other source map contains sourceRoot too, I can send a PR

@ai
Copy link
Member

ai commented Apr 24, 2020

I would like to add tests to make sure that postcss merges source maps fine when other source map contains sourceRoot too, I can send a PR

It will be very helpful

What do you think generate absolute sources by default for v8 (as I said early, babel/ts do it by default)

I do not like that after generation, you will not be able to move the files. Files generated at one client will not work on another machine.

@alexander-akait
Copy link
Author

alexander-akait commented Apr 24, 2020

@ai What about d small breaking change for v8 and:

  • to: '' should consider as not falsy value and generate absolute URLs (very simple solution)

PostCSS already works fine with to: '/' value, so it will be easy to generate absolute sources without new options and big changes

@ai
Copy link
Member

ai commented Apr 24, 2020

PostCSS already works fine with to: '/' value, so it will be easy to generate absolute sources without new options and big changes

But we use a relative URL in the generated file.

What do you think about the extra option to use an absolute path?

@alexander-akait
Copy link
Author

But we use a relative URL in the generated file.
What do you think about the extra option to use an absolute path?

I think it is real unnecessary, the to option already allow to you define where you want to generate files and when you using to: '/' you mean I want to generate sources relative to root filesystem directly. Why do not support to: ''? New options are adding extra support and extra complexity.

@ai
Copy link
Member

ai commented Jul 20, 2020

Fixed 76ac72e

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

No branches or pull requests

2 participants