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

[gatsby-plugin-sass] new options & [gatsby-plugin-react-css-modules] new package #1900

Closed
wants to merge 0 commits into from

Conversation

mingaldrichgan
Copy link
Contributor

@mingaldrichgan mingaldrichgan commented Aug 24, 2017

1. [gatsby-plugin-sass] new options

SASS defaults to 5 digits of precision, which is low enough to cause off-by-1px errors in Bootstrap. See:

I confirmed (by transpiling the bootstrap-sass package and looking at the maximum number of decimal places in the CSS output) that appending ?precision=8 to the sass Webpack loader increases the precision from 5 to 8.

I also wanted to make localIdentName configurable—if a project has more than one index.module.scss file (in different folders), for example, it would be helpful to add [path] to the module class name.

2. [gatsby-plugin-react-css-modules] new package

Gatsby plugin for babel-plugin-react-css-modules—see README.md for details.

@KyleAMathews
Copy link
Contributor

KyleAMathews commented Aug 24, 2017

Deploy preview failed.

Built with commit dd5b650

https://app.netlify.com/sites/using-glamor/deploys/599e7c59424ef241a5709df2

@mingaldrichgan
Copy link
Contributor Author

Any possibility of viewing the build logs?

@mingaldrichgan mingaldrichgan changed the title [gatsby-plugin-sass] Add precision and localIdentName options gatsby-plugin-sass options & gatsby-plugin-react-css-modules (new package) Aug 24, 2017
@mingaldrichgan mingaldrichgan changed the title gatsby-plugin-sass options & gatsby-plugin-react-css-modules (new package) [gatsby-plugin-sass] new options & [gatsby-plugin-react-css-modules] new package Aug 24, 2017
@KyleAMathews
Copy link
Contributor

This looks fantastic! Nice new options + great tests. gatsby-plugin-react-css-modules fills in another nice gap as well.

On build problems — build issues on Netlify are rather opaque. I want to create a new build system for the example sites and will hopefully have time to build it soon.

@KyleAMathews
Copy link
Contributor

KyleAMathews commented Aug 24, 2017

I merged in master which includes #1903 which was breaking builds.

@KyleAMathews
Copy link
Contributor

Oh and do you think that perhaps the [path] should be added by default to the css module paths?

@mingaldrichgan
Copy link
Contributor Author

Re: adding [path] by default, not a bad idea. I personally don't care as long as it's configurable. 😎 Let me know if you'd like me to add it! And I should probably add the same options to the built-in CSS loader and gatsby-plugin-postcss-sass as well, for consistency. Would you like me to include all of the above in the same pull request?

@KyleAMathews
Copy link
Contributor

Would you like me to include all of the above in the same pull request?

Yeah, let's do it! Thanks!

@mingaldrichgan
Copy link
Contributor Author

mingaldrichgan commented Aug 25, 2017

@KyleAMathews I noticed that gatsby-plugin-sass does not load any CSS preprocessors for the develop-html stage, whereas gatsby-plugin-postcss-sass, gatsby-plugin-stylus, and the core webpack.config.js do. I'm not yet familiar enough with the details of each stage to know if it's necessary—do you know if this was left out by mistake?

No develop-html:

Has develop-html:

@KyleAMathews
Copy link
Contributor

Hmm probably we do always want to set the null-loader as otherwise webpack will throw an error. Probably this hasn't been a problem for the sass loader as it'd only show up if someone imported a sass file into a html.js which you don't generally.

But for completeness, yeah, the sass plugin should have a setting for develop-html.

@gatsbybot
Copy link
Collaborator

Deploy preview ready!

Built with commit ac72be2

https://deploy-preview-1900--gatsbygram.netlify.com

@mingaldrichgan
Copy link
Contributor Author

Closed this pull request and opened a new one: #1934

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

Successfully merging this pull request may close these issues.

3 participants