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.

Sign up for GitHub

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

Jump to bottom

feat(gatsby-plugin-sass): add option to enable resolve-url-loader #14272

Merged
merged 14 commits into from
Jul 12, 2019
Merged

feat(gatsby-plugin-sass): add option to enable resolve-url-loader #14272

merged 14 commits into from
Jul 12, 2019

Conversation

carlosruana
Copy link
Contributor

@carlosruana carlosruana commented May 23, 2019 •
edited
Loading

Description

Currently plugin just resolves url() paths relative to the main sass entry file.
resolve-url-loader adds the ability to use paths relative to the file where the url() is being used.

By default if not configured in the gatsby-plugin-sass, the loader is not being used, but you can activate it and also configure resolve-url-loader plugin with the options it provides.

Usage has been also documented in the README.md of the gatsby-plugin-sass

Related Issues

Fixes #7776, #6438
@wardpeet
Copy link
Contributor

Cool 😎 ! Any reason why we shouldn't enable this by default? Any possible breaking scenarios?
@carlosruana
Copy link
Contributor Author

Cool 😎 ! Any reason why we shouldn't enable this by default? Any possible breaking scenarios?

Well, sass and sass-loader do not provide url rewriting by default, they recommend using resolve-url-loader to solve that. So, the expected behavior is using paths in url() relative to the entry file and I would not change defaults as it can confuse plugin users. It would break also projects using images already created with those paths relatives to the entry file and not relative to the file where the image is loaded itself.

Activating it will benefit though users trying to generate documentation from external projects which uses relative url() to the file without having to modify their projects code (that is mainly the reason I need it). Also users generating new documentation with Gatsby from scratch will benefit by being able to add short relative paths as url(./image_name.svg) and not having to remember the main entry file.
@carlosruana carlosruana requested a review from a team as a code owner July 11, 2019 10:56
@slang25
Copy link

slang25 commented Jul 11, 2019

Some anecdotal justification for this change:

I've just picked up gatsbyjs for the first time for a side project (cause it looks great!), and have started to migrate my site to it. This is the first issue I faced with gatsby and I wasn't ready to hack around the problem, and until seeing this PR I was about to drop it and try a different framework altogether!
@carlosruana carlosruana requested a review from a team as a code owner July 12, 2019 08:55
carlosruana and others added 2 commits July 12, 2019 11:09
@carlosruana
Update README.md
e085240 
Running prettier on README
chore: format
2c9155b
wardpeet
wardpeet approved these changes Jul 12, 2019
View reviewed changes
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When green I'll merge this one.

sorry for keeping you waiting @carlosruana! This slipped my mind.
@wardpeet wardpeet changed the title Configuring resolve-url-loader in gatsby-plugin-sass Jul 12, 2019
@wardpeet wardpeet added the bot: merge on green Gatsbot will merge these PRs automatically when all tests passes label Jul 12, 2019
@gatsbybot gatsbybot merged commit bb31a34 into gatsbyjs:master Jul 12, 2019
@gatsbot
Copy link

gatsbot bot commented Jul 12, 2019

Holy buckets, @carlosruana — we just merged your PR to Gatsby! 💪💜

Gatsby is built by awesome people like you. Let us say “thanks” in two ways:

  1. We’d like to send you some Gatsby swag. As a token of our appreciation, you can go to the Gatsby Swag Store and log in with your GitHub account to get a coupon code good for one free piece of swag. We’ve got Gatsby t-shirts, stickers, hats, scrunchies, and much more. (You can also unlock even more free swag with 5 contributions — wink wink nudge nudge.) See gatsby.dev/swag for details.
  2. We just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. Accept the invite by visiting https://github.com/orgs/gatsbyjs/invitation. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

If there’s anything we can do to help, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’.

Thanks again!
johno pushed a commit to johno/gatsby that referenced this pull request Jul 17, 2019
@carlosruana @johno
feat(gatsby-plugin-sass): add option to enable resolve-url-loader (ga…
aa6acab 
…tsbyjs#14272)

* Using resolve-url-loader if configured in the options

* Documenting how to use resolve-url-plugin plugin

* Fixing small typo

* Fixing small typo

* Fixing prettier checks

* Using backtick quotes for strings

* resolve-url-loader needs sass-loader to have sourceMap activated

* Updating documentation

* Updating documentation about sourceMap

* chore: format

* Update README.md

* Update README.md

Running prettier on README

* chore: format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: merge on green Gatsbot will merge these PRs automatically when all tests passes
4 participants
@carlosruana @wardpeet @slang25 @gatsbybot