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

Weird source map paths when used together with sass-loader #652

Closed
woldner opened this issue Jan 2, 2018 · 44 comments · Fixed by #901
Closed

Weird source map paths when used together with sass-loader #652

woldner opened this issue Jan 2, 2018 · 44 comments · Fixed by #901

Comments

@woldner
Copy link

woldner commented Jan 2, 2018

Do you want to request a feature or report a bug?
Bug report

What is the current behavior?
Wrong source map/resource paths using css-loader together with sass-loader as shown below:

Examples of source map paths using the plugin
The actual app.scss path is ./src/styles/app.scss

ex1

However the source map path for the file becomes ./src/styles/src/styles/app.scss and the absolute resource path: C:/Users/m51n/source/repos/source-map-path/src/styles/src/styles/app.scss (does not exist)

ex2

If the current behavior is a bug, please provide the steps to reproduce.
I've created a sample project here-- using the source map example from the sass-loader docs and code from the extract-text-webpack-plugin docs here

With the sample project above I used devtool: "inline-source-map".

What is the expected behavior?
Expected source map path returned by the plugin in this case would be ./src/styles/app.scss and absolute resource path as it exist on disk: C:/Users/m51n/source/repos/source-map-path/src/styles/app.scss

Related issues for reference
webpack-contrib/sass-loader#484

Please mention other relevant information such as your webpack version, Node.js version and Operating System.

webpack v3.10.0
node v9.2.0
os: windows 10

@dcatoday
Copy link

dcatoday commented Jan 3, 2018

Hi I am seeing an issue using ~paths. The path is returned without any deliminating /'s in the url.

@Songworks
Copy link

Came upon the same today with having duplicate-ish paths in sourcemaps.
Here's the relevant loaders in my config (no ExtractTextPlugin and devtool as source-map):

  [
    {
      loader: 'style-loader',
    },
    {
      loader: 'css-loader',
      options: {
        sourceMap: true,
      },
    },
    {
      loader: 'sass-loader',
      options: {
        sourceMap: true,
      },
    },
  ];

To me it seems like sass-loader uses the style file path relative to the current working directory and css-loader uses that same path as just a filename (rather than considering it a proper path?) and prepends the absolute path of the directory where the style file is imported from (regardless if it's by another style file or javascript)?

As a quick example, two files:
/cwd/src/styles/core.scss
/cwd/src/components/bla/styles.scss

Both will be imported by another file within /cwd/src/components/bla.

Sourcemap for core.scss will have the path:
/cwd/src/components/bla/src/styles/core.scss
Sourcemap for styles.scss will have the path:
/cwd/src/components/bla/src/components/bla/styles.scss

@woldner
Copy link
Author

woldner commented Jan 10, 2018

@Songworks see webpack-contrib/sass-loader#529
@evilebottnawi promised that he would have a look in the near future 🔎

@alexander-akait
Copy link
Member

Just for information - reproducible test repo https://github.com/jwldnr/extract-text-issue

@mattmewton
Copy link

I'm having this issue as well. Any update as to when it will be resolved?

@woldner
Copy link
Author

woldner commented Feb 12, 2018

ping @evilebottnawi

@alexander-akait
Copy link
Member

@jwldnr sorry for delay, don't have time for this right now, PR welcome 👍

@alexander-akait
Copy link
Member

Feel free to PR if somebody have time

@woldner
Copy link
Author

woldner commented Feb 15, 2018

Understood 👍🏼

@vlad-ivanov-d
Copy link

Hey guys, do you have any progress/plans on this issue?

@npetruzzelli
Copy link

I haven't been able to fix it, but I've done some digging and will share what I have found in hopes of it being useful to someone who has more time to put into it.

In comment webpack-contrib/sass-loader#484 (comment)

A deleted user claimed to have fixed the issue by editing what was line number 198 at the time:
https://github.com/webpack-contrib/css-loader/blob/v0.28.4/lib/processCss.js#L198

So all that was done was the prefix was removed.

As of this writing, the line has changed to 204:
https://github.com/webpack-contrib/css-loader/blob/v0.28.11/lib/processCss.js#L204

/*202*/	pipeline.process(inputSource, {
/*203*/		// we need a prefix to avoid path rewriting of PostCSS
/*204*/		from: "/css-loader!" + options.from,
/*205*/		to: options.to,
/*206*/		map: options.sourceMap ? {
/*207*/			prev: inputMap,
/*208*/			sourcesContent: true,
/*209*/			inline: false,
/*210*/			annotation: false
/*211*/		} : null
/*212*/	}).then(function(result) {

This code first appears in commit:
09bd206

as posted by @sokra to fix issue #154

Before this commit, sources looked something like this:
image

This occured in version 0.19.x so the first release that saw this go into effect was 0.20.0:
https://github.com/webpack-contrib/css-loader/blob/v0.20.0/lib/processCss.js#L167

It looks like if the from attribute may need more work, then the rest of it may fall into place.

@npetruzzelli
Copy link

npetruzzelli commented Apr 20, 2018

I'm not sure about what kind of file and folder structure @jhnns was originally running into that caused the screenshot he first posted but this is what I see if I remove the prefix, and it appears to be correct:

image

Yes, my code is in a directory called src-0, which is a child of the current working directory. At the time I was trying to rule something out and having an odd name helped.

const extractSass = new ExtractTextPlugin({
    filename: "[name].css"
});

// ...

// ENTRY
    entry: "./src-0/index.js",

// ...

// RULES
                        {
                            loader: "css-loader",
                            options: {
                                sourceMap: true
                            }
                        },
                        {
                            loader: "sass-loader",
                            options: {
                                sourceMap: true,
                                sourceMapContents: false
                            }
                        }
// ...

// PLUGINS
        extractSass,
        new SourceMapDevToolPlugin({
            filename: "[file].map[query]",
            moduleFilenameTemplate: "[resource-path]",
            fallbackModuleFilenameTemplate: "[resource-path]?[hash]",
            sourceRoot: "/",
            noSources: true
        })

Setting sourceRoot: "webpack:///" doesn't hurt the sources attribute in the map, but it does prevent the browser from being able to find the file. I've tried to find information on the convention around using the sourceroot this way, but I've come up empty. Searching documentation and code repos alike hasn't explained why I've seen some boilerplates and plugins use webpack://.

I could open a PR, but all I would be doing is:

-		// we need a prefix to avoid path rewriting of PostCSS
-		from: "/css-loader!" + options.from,
+		from: options.from,

I'm going to guess it needs to be looked into a bit deeper than that.

@npetruzzelli
Copy link

I've been doing some additional digging, and it looks like the problem may be spread out between sass-loader, postcss-loader, and css-loader.

css-loader, might still be contributing to it, but not as much as initially thought. I won't know until I can fix it locally and open up pull requests for each repository.

@alexander-akait
Copy link
Member

alexander-akait commented May 24, 2018

@npetruzzelli sass-loader just output source maps from node-sass (maybe bug in node-sass), postcss don't change source maps, looks problem with css-loader or node-sass

@npetruzzelli
Copy link

@evilebottnawi I must respectfully disagree. As I will outline below, both postcss-loader and sass-loader alter the contents of the source map fields, so we can't say it "just outputs" (or otherwise passes through) anything.

I'm still looking, but so far the only place I've found css-loader to be interacting with source maps is the following:

css-loader/lib/loader.js

Lines 118 to 130 in 8897d44

if(sourceMap && result.map) {
// add a SourceMap
map = result.map;
if(map.sources) {
map.sources = map.sources.map(function(source) {
return source.split("!").pop().replace(/\\/g, '/');
}, this);
map.sourceRoot = "";
}
map.file = map.file.split("!").pop().replace(/\\/g, '/');
map = JSON.stringify(map);
moduleJs = "exports.push([module.id, " + cssAsString + ", \"\", " + map + "]);";
} else {

It looks like it is trying to repair fields that have incorrectly been assigned file system paths instead of URLs. It is doing more than that, but I haven't quite figured it out yet, though I think it may be connected to something the style-loader does.

The postcss-loader does change maps. Specifically it changes both sourceRoot and sources from URLs to file system paths. While they have similar syntax (especially if you only consider relative paths) they are not the same thing. See: https://github.com/postcss/postcss-loader/blob/44167910d13fea32208989ffb31406f9577baf28/lib/index.js#L155-L157

sass-loader does the same: https://github.com/webpack-contrib/sass-loader/blob/2529c0716b1bca321c22d16636b1385682b1c730/lib/loader.js#L85-L94

node-sass is not responsible for the source maps, as it simply consumes LibSass.

I've gone as far as to confirm that the comment in the code is incorrect with the LibSass project: sass/libsass#2651

The loader, at the very least, has access to the destination this.options.output and if the server is running, that is available through this.options.devServer. Between the entry point, the resourcePath, the context, and the server configuration, it is probably possibly to correctly determine the URLs without trying to convert them to file system paths. I'm working on a proof of concept, but don't know how long it will take.

I am paying special attention to a note in node-sass's documentation:

https://github.com/sass/node-sass/blob/8040cb7a0ab72a7c34120cc05be9e3e518f9ba34/README.md

Special behaviours

  • In the case that both file and data options are set, node-sass will give precedence to data and use file to calculate paths in sourcemaps.

If you'd like to reference the Source Map specification to confirm that the fields mentioned are not file system paths, you can find that here: Source Map Revision 3 Proposal

Is there something I am missing that would invalidate any of the above? I will be happy to acknowledge I'm wrong if I'm wrong, so please help me understand it. I'm more interested in progress than being right.

Kind regards.

@Volune
Copy link

Volune commented May 30, 2018

I met this issue too, and did some more investigation. Hopefully that can help.


First, things I've read from the doc

  • Webpack says the loaders should return sourcemaps compatible with mozilla/source-map module, see here
  • source-map says the sources must be URL

In fact, I've identified one function in source-map util.join that is not compatible at all with absolute Windows path.
A POSIX path is mostly compatibly with a URI path
When we convert a Windows path path:

  • replace \ by /
  • absolute Windows path should also be prefixed with /

For example

  • path\file.css -> path/file.css
  • Z:\path\file.css -> /Z:/path/file.css

Because they don't add the leading / when converting an absolute POSIX path to a Windows path, most libraries end up not working well with source-map on Windows.


From what I've seen from the loaders on Windows:

  • sass-loader returns a Windows relative path, which is not compliant with latest webpack doc, but seems to work most of the time
  • post-css returns a Windows absolute path, which is not consumable by any following loader
  • css-loader does that strange thing with "/css-loader!"

@alexander-akait
Copy link
Member

sass-loader returns a Windows relative path, which is not compliant with latest webpack doc, but seems to work most of the time

PR welcome!

post-css returns a Windows absolute path, which is not consumable by any following loader

PR welcome!

css-loader does that strange thing with "/css-loader!"

It is require for internal works, we can fix it in next major release.

@terremoth
Copy link

terremoth commented Jul 5, 2018

I am experiencing the same thing!

friendly ping to @jwldnr and @evilebottnawi

I am using Windows and Wamp server, and all the things and options tried in
"https://webpack.js.org/configuration/output/#output-devtoolmodulefilenametemplate"
returned a wrong path, even modifying "file//" to "file///"
It duplicates part of the path in the URI...

Using, for example, "[absolute-resource-path]" returns this:

http://mywebsite.how/assets/C:/wamp64/www/mywebsite/resources/assets/css/css/sass/resources/assets/css/css/sass/tablet/_footer.scss

(part in bold is a weird duplicated part returned)

what should return:
C:/wamp64/www/mywebsite/resources/assets/css/css/sass/tablet/_footer.scss

My infos:

1 - We are trying to automate the work, so we are trying to edit the file via Chrome Sources in DevTool
2 - Windows 10, with WAMP server
3 - Chrome 67.0.3396.99
4- Using Laravel Mix with the following config:

mix.sourceMaps().webpackConfig({
    devtool: 'source-map',
    output: {
        devtoolModuleFilenameTemplate: "file:///[ANY-OPTION]",
        devtoolFallbackModuleFilenameTemplate: "file:///[ANY-OPTION]?[hash]"
    }
});

I think solving this problem will help a lot of people.

@gcangussu
Copy link

gcangussu commented Jul 9, 2018

I'm experiencing the same problem on MacOS, the path to the original file on the source map is
/Users/gabriel/Repos/myProject/app/styles/app/styles/_styles.scss

But the respective file is really at
/Users/gabriel/Repos/myProject/app/styles/_styles.scss

Note that there is a repeated app/styles/ on the source map path.

@ignitenet-martynas
Copy link

I have found that removing the following line in node_modules/css-loader/lib/loader.js (line number 28 in css-loader version 1.0.0) fixes the problem for me:

map.sourceRoot = '';

@Nettsentrisk
Copy link

Still not resolved? Any work-arounds? I'm having this problem with css-loader + postcss-loader + sass-loader.

@magnusriga
Copy link

I'm still having the same issue. With my limited knowledge of these loaders it is hard to tell whether it is a problem at all. Will it break sourcemap support? Won't debugging work correctly? My debug points seem to be hit correctly.

@alexander-akait
Copy link
Member

Please create minimum reproducible test repo

@woldner
Copy link
Author

woldner commented Nov 7, 2018

Please create minimum reproducible test repo

https://github.com/jwldnr/extract-text-issue

@alexander-akait
Copy link
Member

@jwldnr don't use extract-text-webpack-plugin, it is break source map, use https://github.com/webpack-contrib/mini-css-extract-plugin, also please describe in readme what you have and what you expected

@magnusriga
Copy link

@evilebottnawi Here is a MWE: https://github.com/magnusriga/css-loader

If you take a look at the source map paths in bundle.js within /dist, you will see the issue. The paths are wrong (multiple repeated /src paths, etc.).

If you remove sass-loader from the loader chain, the problem disappears, for some reason.

@npetruzzelli
Copy link

npetruzzelli commented Nov 7, 2018

@jwldnr - to further support evil's comment, you can find additional documentation on the mini-css-extract-plugin in the repo or in Webpack's documentation:
https://webpack.js.org/plugins/mini-css-extract-plugin/

The extract text plugin calls this out:

⚠️ Since webpack v4 the extract-text-webpack-plugin should not be used for css. Use mini-css-extract-plugin instead.

⚠️ For webpack v1, see the README in the webpack-1 branch.

More can be read in webpack-contrib/extract-text-webpack-plugin#749

@magnusriga - I have a PR open to work with evil on the issue, but he has other priorities he needs to work on before he can give my PR the time it needs for feedback. Please follow #753 and its counter part webpack-contrib/sass-loader#602 if you are interested, but be aware it will take some time before further action can be taken.

@alexander-akait
Copy link
Member

@npetruzzelli sass-loader/less-loader/postcss-loader ready for fixes, you can safely send PRs there

@magnusriga
Copy link

@npetruzzelli Ok, thanks. Just as a heads up, my minimum working example does not include any extract loaders or plugins not needed to demonstrate the issue. It should be directly to the point.

Quick question though: Will these paths cause any actual issues? i.e. won't source maps work properly?

Thank you

@alexander-akait
Copy link
Member

We are working on linux and source maps for CSS works great, i think problem only for windows so I ask to create minimum reproducible test repo

@npetruzzelli
Copy link

Thanks @evilebottnawi - Since I haven't touched this since August, it will take me some time to get caught up / merge in changes that have come in since then. I will start with sass-loader per my prior comment. I may be a bit slow on the update as there is a lot going on personally and professionally right now, but I will try to have an update for you by the end of the month.

@alexander-akait
Copy link
Member

@npetruzzelli thanks for any helping 👍

@Nettsentrisk
Copy link

extract-text-webpack-plugin is irrelevant for my case, since it's happening in develop mode with HMR, where style-loader and no extract plugins are being used (I use mini-css-extract-plugin anyway).

I think this problem only affects Windows users, which might be why it's flown under the radar for so long.

@tenebrius
Copy link

Any progress or workaround guys?

@alexander-akait
Copy link
Member

@tenebrius Pr welcome

@npetruzzelli
Copy link

@evilebottnawi - A lot of the hectic stuff has passed. I plan to resume work on it shortly. I am sorry for the delay.

@alexander-akait
Copy link
Member

@npetruzzelli maybe i can help if you provide minimum reproducible test repo, i am already working on 2 version (you can look progress in master branch)

@npetruzzelli
Copy link

@evilebottnawi - I'll continue with comments in the PR: #753

There, I've included a link to the fork and am in the process of merging in updates to the master branch. Once I have tests working, I will create a minimal, reproducible test repo (separate from the fork).

Master still has version 1.x according to it's package.json file. If you intended for me to look at progress for 2.x in the next branch, I do not have permission to view that. I'm going to guess you want me to work from master, can you confirm that?

@alexander-akait
Copy link
Member

@npetruzzelli use master branch 👍

@Chris-Kim
Copy link

For me, this issue is resolved by upgrading css-loader to 2.x.x

@alexander-akait
Copy link
Member

Other example https://github.com/kyle-ssg/sass-loader-bug

@alexander-akait
Copy link
Member

Find solution and problem, will be solved and released today

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

Successfully merging a pull request may close this issue.