Fix file path in source maps

[cevou]

There are currently two problems regarding source maps file path with this loader:

1. The loader depends on self.options.output.path to be set. If you don't set this property the relative urls in line 281 is calculated wrong. By using the context this problem does not exist anymore.
2. node-sass uses the directory in which the map file is created as root path. All source paths within the map are relative to this path. However, when webpack processes the sourcemaps it expects all paths to be relative to the webpack execution directory. To resolve all files correctly I added the sourceRoot property to express the relative path.

In combination with the suggested fix for the css-loader (webpack-contrib/css-loader#280) this produces nice paths for source maps, that reflect the actual file structure within the project.

[jhnns]

Thx! 👍

I probably need to change this again because a loader should not access webpack options

[jhnns]

Shipped as 3.2.2

[billyross]

Just wondering if it would be possible to retain the option of using output.path if present? I'm currently seeing a webpack build fail when loading font files...
ERROR in ./~/css-loader!./~/resolve-url-loader!./~/sass-loader?sourceMap!./~/bootstrap-loader/lib/bootstrap.styles.loader.js!./~/bootstrap-loader/ no-op.js Module not found: Error: Cannot resolve 'file' or 'directory' ./fonts/vlook/6dfa3ef5-ebc5-4d23-a631-b3d746c2ad39.woff2 in C:\working\{distPath}\node_modules\bootstrap-loader @ ./~/css-loader!./~/resolve-url-loader!./~/sass-loader?sourceMap!./~/bootstrap-loader/lib/bootstrap.styles.loader.js!./~/bootstrap-loader/no-op. js 6:1131-1198

[cevou]

Do you have a sample configuration file with which the error could be reproduced?

[billyross]

Hi there, unfortunately I can't provide the exact configuration although it is essentially based on https://github.com/davezuko/react-redux-starter-kit.

With webpack.config here:
https://github.com/davezuko/react-redux-starter-kit/blob/master/build/webpack.config.js
Consts defined here:
https://github.com/davezuko/react-redux-starter-kit/blob/master/config/index.js.

I'd assume with this recent change that that project would have similar problems should custom fonts be loaded also.

[e-cloud]

yes, it should be breaking change. I am facing the same error.

[bel0v]

same here

[cevou]

After some more investigation. This change indeed breaks the resolve-url-loader. As you can see in https://github.com/bholloway/resolve-url-loader/blob/master/index.js#L63 the loader assumes that the sass-loader outputs source maps relative to the output directory. This was changed with this pull request. So this pull request might be considered as a breaking change.
There are some changes needed in resolve-url-loader to fix this. First the outputPath needs to be changed to the context path. With this change it still doesn't work. The loader gets also confused by the sourceRoot property. I might check on this in the next couple of days.

For now this change breaks a lot of installations. Basically everyone who uses resolve-url-loader is effected. So we either need a quick fix in resolve-url-loader or need to revert this change for now and put it in a new version 4.0.

// cc: @bholloway, @jhnns

[bholloway]

Sorry I am not across this, I'm afk for at least a week. Just bad timing. The source map base path isn't well standardised across loaders. I use [adjust-sourcemap-loader] bholloway/adjust-sourcemap-loader) myself. Is this change a done thing? I would leave this for a major release if it changes default behaviour of any kind. The output-relative had to be working for someone up until now and they will be annoyed. Or put it under an optional flag and call it a minor release.

[jhnns]

Ok, I'll flag this version as broken, publish a new major release and then we can sort it out. No stress! 👍

@bholloway would you be open to make a PR to the sass-loader so that we have tests to check against the resolve-url-loader? Seems like more people are using it, so it would be nice to ensure compatibility. It was not obvious for me that this could be breaking change at all. If you do the PR, please mention me so I can merge it asap.

[jhnns]

Shipped as 4.0.0

[cevou]

I created a pull request to resolve-url-loader to work again with the latest version. bholloway/resolve-url-loader#23

[bholloway]

@jhnns given that you can't unpublish 3.2.2. Could you branch an orphan from tag 3.2.1 and re-release as 3.3.3.

It would make it easier to specify to my users sass-loader@<4 and it would definitely move the breaking change to 4.0.0.

[jhnns]

I've already published 3.2.3 which is identical to 3.2.1.

[bholloway]

Ah yes. Sorry I thought I read that somewhere but I didn't see it in the tags. I should have checked with npm.

All good.