sindresorhus / gulp-autoprefixer

Prefix CSS
MIT License
692 stars 50 forks source link

Update sourcemap paths to be relative to `file.base` #92

Closed johanblumenberg closed 6 years ago

johanblumenberg commented 6 years ago

From https://www.npmjs.com/package/gulp-sourcemaps

Fixes #55

sindresorhus commented 6 years ago

This should be fixed in vinyl-sourcemaps-apply. It receives the file, so it can do the changes. That way all Gulp plugins benefit instead of just this one.

johanblumenberg commented 6 years ago

@sindresorhus No, I'm not sure it can be fixed in vinyl-sourcemaps-apply.

The problem is that gulp-autoprefixer receives file abc/source.css, but outputs a source map containing source.css. vinyl-sourcemaps-apply cannot guess that the source file is actually abc/source.css. Also the documentation in gulp-sourcemaps explicitly mentions the fact that all files must be relative to file.base, and marks this as important (see link in description).

sindresorhus commented 6 years ago

applySourceMap(file, map); receives the file and the map, so it has exactly the same information we do and it can use file.relative.

johanblumenberg commented 6 years ago

Maybe if map.file is missing, it could add that. But for sources it is impossible to know where they come from using the information in file.

Consider for example if the sass css compiler would do this:

applySourceMap(
  { path: "/Users/x/project/src/styles/style.css", relative: "src/styles/style.css" },
  { file: undefined, sources: [ "style.scss", "theme.scss", "mixins.scss" ] })

Now, it would be possible to fill in the map.file property, assuming it is the same as the given file. But it would be impossible to figure out where the sources come from.

You are assuming that every plugin that uses vinyl-sourcemaps-apply work in the same way, they have exactly one input which is the same as the output.

sindresorhus commented 6 years ago

I'm not saying it should replace the files in the source map with file. I'm saying by having file it already has the information it needs to make paths relative if they are not. vinyl-sourcemaps-apply already fixes the paths for Windows: https://github.com/gulp-sourcemaps/vinyl-sourcemaps-apply/blob/30320c97c112f44ccba02dd73ce5bed1ad4361de/index.js#L20-L23 It could make them relative too if needed using file.cwd/file.base.

johanblumenberg commented 6 years ago

Changing the path separator is not at all the same thing as changing what the path points to.

In my example above, how would applySourceMap() be able to figure out that the source file "mixins.scss" is not supposed to be "mixins.scss", but it should be "src/styles/mixins/mixins.scss"?

And how would applySourceMap() be able to figure out that in the case of gulp-autoprefixer it should change the path, but when any other plugin passes a source of "themes.scss", it should actually be "themes.scss"?

johanblumenberg commented 6 years ago

If all the sources were absolute paths it would be possible, but they are not. And I'm not sure they are supposed to be, either, since the sourcemap is passed to the browser for it to be able to display sources correctly.

johanblumenberg commented 6 years ago

@sindresorhus I rebased on latest master

johanblumenberg commented 6 years ago

I believe this fixes https://github.com/sindresorhus/gulp-autoprefixer/issues/55