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

Add support for CSS modules #295

Merged
merged 3 commits into from
Jul 6, 2016
Merged

Add support for CSS modules #295

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d6e5388
Add support for CSS modules
jakubrohleder May 23, 2016
34f197e
Minor fixes
jakubrohleder Jun 9, 2016
5b101be
Merge branch 'master' into feature/css-modules
jakubrohleder Jun 11, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,9 @@ if (process.env.NODE_ENV === 'production') {
// Then in your <head>
{css}
```
### CSS modules

[Css modules](https://github.com/css-modules/css-modules) are support by default for all files with `.module.(css|less|scss|sass)` extension.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be "CSS Modules" here plus in the header.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps add examples of two css files where one (main.css) wouldn't use CSS Modules and another (my-awesome-component.module.css) would be.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By example, you mean to add example to gatsby-starter-default (or other starter) or to readme?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well that would be great too (adding to the starter) :-) but I meant here to add a simple example like:

// Uses CSS Modules
import './my-component.module.css'

// Doesn't use CSS Modules
import './main.css'


### Configuring Babel

Expand Down
42 changes: 42 additions & 0 deletions lib/utils/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ module.exports = (program, directory, stage, webpackPort = 1500, routes = []) =>
},
__PREFIX_LINKS__: program.prefixLinks,
}),
new ExtractTextPlugin('styles.css'),
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't extract styles here and in build-javascript on purpose as we already do that in the build-css stage. Doing it again would be wasted effort.

Copy link
Contributor Author

@jakubrohleder jakubrohleder May 24, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Without this line the following snippet:

import styles from './style.css'

is producing an empty object. I don't fully understand the reason of it.

Maybe there is a better alternative for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh hmmm... when building the HTML pages you mean? That does make sense.

Probably what you need to do is just copy the modules css loader config from develop to build-html as right now in build-html all css loaders are turned off. https://github.com/jakubrohleder/gatsby/blob/d6e5388ea0b5581e4422afd02110808716d13e02/lib/utils/webpack.config.js#L357-L373

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did some testing: for build-html step css-modules loaders has to be turned on (or just not turned off), they use ExtractTextPlugin so it also has to be included in the build step configuration. On the other hand they are not needed in build-javascript step. I don't know why previously it seemed to me that this is necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems reasonable. And since this is a new experimental change that doesn't conflict with anything (unless someone named their css file *.modules.css) we can ship something we're not 100% on and refine it after the fact.

]
case 'build-javascript':
return [
Expand All @@ -132,6 +133,7 @@ module.exports = (program, directory, stage, webpackPort = 1500, routes = []) =>
}),
new webpack.optimize.DedupePlugin(),
new webpack.optimize.UglifyJsPlugin(),
new ExtractTextPlugin('styles.css'),
]
default:
throw new Error(`The state requested ${stage} doesn't exist.`)
Expand Down Expand Up @@ -262,20 +264,43 @@ module.exports = (program, directory, stage, webpackPort = 1500, routes = []) =>
loader: 'file-loader',
})

const cssModulesConf = 'css?modules&importLoaders=1'
const cssModulesConfDev =
`${cssModulesConf}&sourceMap&localIdentName=[name]---[local]---[hash:base64:5]`
Copy link
Contributor

Choose a reason for hiding this comment

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

localIdentName is just for the generated class names correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly, by default class names are rewritten to hashes, but for development it is more convenient to have extra [name]---[local] prefix, so you can easily identify module where class comes from.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍


switch (stage) {
case 'develop':

config.loader('css', {
test: /\.css$/,
exclude: /\.module\.css$/,
loaders: ['style', 'css', 'postcss'],
})
config.loader('less', {
test: /\.less/,
exclude: /\.module\.less$/,
loaders: ['style', 'css', 'less'],
})
config.loader('sass', {
test: /\.(sass|scss)/,
exclude: /\.module\.(sass|scss)$/,
loaders: ['style', 'css', 'sass'],
})

// Css modules
Copy link
Contributor

Choose a reason for hiding this comment

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

Proper capitalization

config.loader('cssModules', {
test: /\.module\.css$/,
loaders: ['style', cssModulesConfDev, 'postcss'],
})
config.loader('lessModules', {
test: /\.module\.less/,
loaders: ['style', cssModulesConfDev, 'less'],
})
config.loader('sassModules', {
test: /\.module\.(sass|scss)/,
loaders: ['style', cssModulesConfDev, 'sass'],
})

config.merge({
postcss: [
require('postcss-import')(),
Expand All @@ -289,16 +314,33 @@ module.exports = (program, directory, stage, webpackPort = 1500, routes = []) =>
case 'build-css':
config.loader('css', {
test: /\.css$/,
exclude: /\.module\.css$/,
loader: ExtractTextPlugin.extract(['css', 'postcss']),
})
config.loader('less', {
test: /\.less/,
exclude: /\.module\.less$/,
loader: ExtractTextPlugin.extract(['css', 'less']),
})
config.loader('sass', {
test: /\.(sass|scss)/,
exclude: /\.module\.(sass|scss)$/,
loader: ExtractTextPlugin.extract(['css', 'sass']),
})

// Css modules
config.loader('cssModules', {
test: /\.module\.css$/,
loader: ExtractTextPlugin.extract('style', [cssModulesConf, 'postcss']),
})
config.loader('lessModules', {
test: /\.module\.less/,
loader: ExtractTextPlugin.extract('style', [cssModulesConf, 'less']),
})
config.loader('sassModules', {
test: /\.module\.(sass|scss)/,
loader: ExtractTextPlugin.extract('style', [cssModulesConf, 'sass']),
})
config.merge({
postcss: [
require('postcss-import')(),
Expand Down