Adjust resolve-url-loader to work with sass-loader 4.0

bholloway / resolve-url-loader

Webpack loader that resolves relative paths in url() statements based on the original source file

563 stars 71 forks source link

Adjust resolve-url-loader to work with sass-loader 4.0 #23

Closed cevou closed 8 years ago

cevou commented 8 years ago

This fixes #22. After the change in https://github.com/jtangelder/sass-loader/pull/250 source maps are not created in the output directory anymore. This is now done in the context directory. This changes the behavior and also fixes some path problems in source maps on windows.

jhnns commented 8 years ago

Nice! 👍

Awesome work @cevou

philipchmalts commented 8 years ago

Would be awesome to get this merged soon!

bholloway commented 8 years ago

Thanks @cevou for your work on this.

I just started a new job so I'm still installing tools. Give me a day or two and I will be in a better position to help. In the mean time all I can do is make suggestions.

bholloway commented 8 years ago

Ok @cevou I created feature/sass-loader-4 which

synched latested changes with master (keepQuery option)
included your PR
added support for both versions of sass-loader, as proposed

I wanted to keep your commits to credit your contribution. Please let me know if you would be happy with this result.

I have tested with a 'todo' project and it works for sass-loader@~3.1.x and sass-loader@4.0.0.

Tomorrow I hope to try some of the weird use-cases people have sent me.

bholloway commented 8 years ago

FYI. I am about to consider the following historical use cases. As far as I recall both are fixed.

sass-rel-paths-issue
~~webpack-sass-url-issue~~ (solved without resolve-url-loader)

I will need to adjust the package.json to use the feature/sass-loader-4 branch. They will still refer to whichever stale/deleted branch solved the issue at the time.

I will check with both sass-loader@3.2.3 and sass-loader@4.0.0.

cevou commented 8 years ago

Sounds good to me. Anything I should do before this can be released?

bholloway commented 8 years ago

@cevou It seems I am short of use cases.

For your project, if you can use feature/sass-loader-4, and please try both sass-loader@3.2.3 and sass-loader@4.0.0. I will then be a lot more confident.

bholloway commented 8 years ago

Ok. I am happy. @cevou let me know when you are happy.

I was thinking that once you are happy I will merge feature/sass-loader-4 into pr/23, so that we can see all the code here. Then I will merge the PR.

I have not done it that way before so any opinions would be valued.

cevou commented 8 years ago

I will check my projects when I'm at home later today.

cevou commented 8 years ago

I'm happy, too :smile: I also cherry-picked your changes and added them to the pull request.

bholloway commented 8 years ago

published as 1.6.0

cevou commented 8 years ago

Thx

bholloway commented 8 years ago

No worries @cevou. Thanks for your help.

Sorry I was not around earlier. Thanks @jhnns for fiddling the semver to minimise the impact in the mean time.

Klathmon commented 8 years ago

Just wanted to drop in and say thanks to everyone.

This hit me when I upgraded sass loader to 4.0, I found this issue after a bit of searching, followed it and locked my versions, and just upgraded this morning without any issues.

jhnns commented 8 years ago

Much appreciated! 👍