postcss / gulp-postcss

Pipe CSS through PostCSS processors with a single parse
MIT License
769 stars 65 forks source link

Allow dirname of source path referenced by sourcemap to be customized #155

Open mojavelinux opened 5 years ago

mojavelinux commented 5 years ago

By default, the dirname of the source paths referenced by the sourcemap mirrors the src glob. For example, if the source CSS file is css/site.css, then the source paths for each @import when sourcemaps are enabled become css/base.css, etc. However, it's not clear what "css" refers to here, especially if the output folder for the CSS is already "css". Then, you get "css/css/base.css" in the developer tools.

It would be ideal if the base directory were to be named after the file in which the imports were resolved. For example, the sourcemap would instead be site.css-src/base.css.

The line of code I'm looking to be able to customize is this one:

return path.join(path.dirname(file.relative), source)

I'm looking for something like:

return path.join(file.basename + '-src', source)

or even:

return path.join(file.basename + '-' + source)

I'm open to any configuration that would give me control over that line, even if it's a callback function.

stof commented 4 years ago

gulp.src already allows to customise that thanks to its base option. In my own project, I'm using gulp.src('assets/css/*.css', { 'base': '.' }), which makes paths be relative to the root of the project rather than to the static prefix of the glob pattern.

The behavior of gulp-postcss is to respect what gulp manages as file.relative, so there is nothing to change in it IMO.

w0rm commented 3 years ago

Yeah, I think setting the base option for the glob pattern should work!

mojavelinux commented 3 years ago

I'm afraid you've misunderstood my point. I assert that the behavior of postcss is still incorrect. I will elaborate.

Let's assume I have the following structure:

src/
  css/
    site.css
    base.css
    body.css

And site.css looks like this:

@import "base.css";
@import "body.css";

Now assume I compile the CSS files as follows:

vfs
  .src('css/site.css', { base: 'src', cwd: 'src', sourcemaps: true })
  .pipe(postcss(postcssPlugins)),

The sourcemap file for css/site.css (css/site.css.map) will contain:

"sources":["css/base.css","css/body.css"]

That's suggesting that the files are located at css/css/base.css and css/css/body.css, respectively. That is incorrect. They should be "base.css" and "body.css" (as they are in the source).

Unless I'm mistaken, there's no way to control this using the base option. By manipulating base, the site.css file just ends up in the wrong directly. Please enlighten me if there is a way.

You can see an example of this scenario in the following project. https://github.com/asciidoctor/asciidoctor-docs-ui

stof commented 3 years ago

@mojavelinux if you pass a relative pass to base, it is relative to cwd. So that's your own config being wrong. Either use absolute paths, or use the right relative paths.

stof commented 3 years ago

Btw, I don't understand your point. If you tell that the base is src, css/base.css is not wrong at all. that's indeed the relative path of the base.css file compared to the base.

mojavelinux commented 3 years ago

gulp.src already allows to customise that thanks to its base option. In my own project, I'm using gulp.src('assets/css/*.css', { 'base': '.' }), which makes paths be relative to the root of the project rather than to the static prefix of the glob pattern.

I don't understand how this helps at all. If I do this, then I get 'src/css/site.css' in my output folder.

if you pass a relative pass to base, it is relative to cwd. So that's your own config being wrong. Either use absolute paths, or use the right relative paths.

What you are suggesting simply doesn't work. If I don't set base and cwd as I do, either it cannot find the files or it creates the wrong paths in the output folder.

mojavelinux commented 3 years ago

css/base.css is not wrong at all. that's indeed the relative path of the base.css file compared to the base.

No, because the sourcemap is providing relative paths to the aggregate file. So for it to reference 'css/base.css' is incorrect. The relative path from site.css is base.css, not css/base.css.

mojavelinux commented 3 years ago

My point is not that site.css is not being processed. (It is being processed). That is not the problem. The problem is that the path in the sourcemap is wrong.

mojavelinux commented 3 years ago

No matter what I set base to, the path in the sourcemap is still wrong. The path should honor the path of the import. That is what I'm arguing for.

mojavelinux commented 3 years ago

Btw, if I set base: '.', and use 'src/css/site.css' in the glob, here's what I get in my output folder:

src/
  css/
    site.css

I don't want that. I want this:

css/
  site.css

The point here is not to change the structure vinyl-fs is creating. The structure I have is the structure I want. I'm focused on the contents of the sourcemap file, specifically the paths of the imported files.

stof commented 3 years ago

@mojavelinux why would you want to see base.css but css/site.css ? If you want to see only paths relative to the css/ folder and not the src/ folder, it means that your base should not be the src/ folder but the css folder.

All paths in a sourcemap are meant to be relative to the same base folder. They are not relative to the place using them (as the sourcemap does not have a concept of where the base.css file is used from)

mojavelinux commented 3 years ago

All paths in a sourcemap are meant to be relative to the same base folder.

That's not how the browser sees it. When I load the page, Firefox reports the sourcemap file as:

css/css/base.css

You can think that the sourcemap file is relative to the base, but the browser doesn't.

mojavelinux commented 3 years ago

as the sourcemap does not have a concept of where the base.css file is used from

That reinforces my original point that there needs to be a way to configure this (independent of the base option, which is controlling a different aspect of processing).

stof commented 3 years ago

@mojavelinux well, if you dump your sourcemaps inside the css/ folder in your output, then your basis should also be the css/ folder, not the src/ folder. But that's not only about base.css. You will have the same issue for site.css (except you have no rules in this file directly to see the issue). You should configure a base folder consistent with the place where you dump the sourcemap (or use more advanced features of sourcemaps to configure the sourcemap root, but I don't how whether/how gulp exposes these features when dumping sourcemaps as I never tried to use them)

mojavelinux commented 3 years ago

Here's how it looks in Chrome btw:

sourcemap-hierarchy

mojavelinux commented 3 years ago

if you dump your sourcemaps inside the css/ folder in your output

it's gulp-postcss that's putting the sourcemaps there, not me.

You should configure a base folder consistent with the place where you dump the sourcemap

You're suggesting an action which is simply not practical. If I manipulate base, then the actual site.css file ends up in the wrong place. I don't want to move site.css. I want the sourcemap to report the correct paths.

stof commented 3 years ago

gulp-postcss does not put the sourcemaps anywhere. It attaches it to the gulp File object. Writing the sourcemap is handled by other parts of your gulp config (not shown in your issue description).

mojavelinux commented 3 years ago

Writing the sourcemap is handled by other parts of your gulp config (not shown in your issue description).

My mistake. They are written using the path .. That keeps them adjacent to the file they are mapping, which seems to me to be correct.

mojavelinux commented 3 years ago

My takeaway from this is that gulp ecosystem just doesn't know how to construct the sourcemap paths for bundled sources and they are basically just pseudo-paths relative to a base the browser cannot see. It's weird, but not important enough to battle over. (For what it's worth, gulp concat has the same problem for js files). I'll just assume that's the way it is. I don't see anyway I can make them right.

Manipulating file.relative causes site.css to end up in the wrong location, so that's not an option. And I don't see how I can get it to move the sourcemap file up a directory. I'm not going to be hassled into thinking I'm using gulp incorrectly. I was just trying to propose an improvement. But maybe there isn't one.

w0rm commented 3 years ago

I’ll keep this open, but I’d rather not add any extra configuration option for us to support, because it might just make things even more confusing. I spent a lot of time getting sourcemaps right in the past, but have forgotten how it worked.

Sourcemaps are supposed to be working the same way through a lot of plugins, according to how they are designed in Gulp. If some cases are weird, this plugin might not be the best place to fix it. Also, if one plugin does something weird, this might affect functionality of other plugins down the pipe, and I believe there were a bunch of issues like that in the past.

Perhaps we can document the existing behavior and the logic behind sourcemaps, including how the options in the glob pattern affect the file names.

mojavelinux commented 3 years ago

Also, if one plugin does something weird, this might affect functionality of other plugins down the pipe

Since we see this same problem with bundling of JS files, I think you have a strong point that it cannot simply be changed here. Either it has to happen somewhere upstream, or it has to be coordinated.

Perhaps we can document the existing behavior and the logic behind sourcemaps

That would certainly give us a path forward. Because right now, I don't see one. I have spend so many hours with this gulp build and just getting it to do what it does now took a huge investment. To have the sourcemap sources be incorrect throws a wrench into things if I have to change it, if I even knew what it is a had to change. So some sort of guidance would go a very long way. It's frustrating to have the ability to change the build, but to not know what change I have to apply to meet this theoretical "sourcemaps sources are supposed to be working" target. (Keep in mind, the sourcemap does work, it just gives the developer the wrong impression about where the file is located).

mojavelinux commented 3 years ago

Btw, I do see one path through this, which is to use another pipe function to manipulate the value of the sourceMap.sources array after the file has been combined. It's not elegant, but it at least gives me the control I want.