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

'problems with url()' no longer a problem? #144

Closed
bholloway opened this issue Aug 13, 2015 · 3 comments
Closed

'problems with url()' no longer a problem? #144

bholloway opened this issue Aug 13, 2015 · 3 comments

Comments

@bholloway
Copy link
Contributor

Awesome loader.

In your readme.md you note the problem we have all encountered with relative url() statements in SASS.

I have written a resolve-url-loader which I hope addresses the issue and I would value your opinion.

It is actually a port of some code I have been using in Browserify for quite a while on a number of large projects. I am less familiar with Webpack and defer to your greater experience.

@jhnns
Copy link
Member

jhnns commented Aug 14, 2015

I did just skim through your source code and it looks good! 👍 You did even use urlToRequest(). Would you mind sending me a PR with the updated README? I would be happy to mention your loader.

I was just wondering if this behavior could/should be part of the css-loader when source maps are present. In this case we could avoid parsing the CSS twice.

@bholloway
Copy link
Contributor Author

Will get going on a PR.

Regarding consolidation with the css loader - I think you are correct in your existing readme.md where you imply that the root of the problem is with Sass.

__
For Sass I believe the recommended practice is a root scss file and a set of library scss files prefixed with an underscore. The assets being specified relative to the output css, not any of the scss source files. This makes all fully relative references root-relative to the output folder.

As you point out, this is a deficit from Sass not managing the url(); all values must be final. However in the context of the scss file it is counter-intuitive: Conversely an equivalent css file will resolve assets relative to the css in which it is specified.

In particular it makes it difficult to cherry-pick files from different libraries. Given only the library file, we have no way of reliably detecting the intended output directory and so cannot resolve relative references in the file. Frameworks such as bootstrap-sass are forced to compose url() with variables to allow users to work around their individual use case.
__

Given that we are 'fixing' this behaviour, my gut tells me it would be more correct to encapsulate this in the sass loader rather than address it in the css loader, regardless of where the efficiencies lie.

That said the convention is against being monolithic. And for those comfortable with the standard sass behaviour you would certainly want to save them the parsing and synchronous file-system operations involved in the 'fix'.

But I would say first see whether this solution proves to be successful and is the dominant use case for sass loader.

@Shinobi881
Copy link

@bholloway You rock so freaking hard!!!!

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

No branches or pull requests

3 participants