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

Sass loader #4195

Merged
merged 28 commits into from
Apr 18, 2018
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
4b40a7d
Installs and adds sass loader task in webpack for dev environment.
Fabianopb Mar 20, 2018
0670888
Uses Timer's branch of sass-loader without node-sass dependency.
Fabianopb Mar 20, 2018
fc1b9d4
Adds method for handling SASS modules.
Fabianopb Mar 21, 2018
ac4cf34
Fixes extension of excluded files when looking for scss modules.
Fabianopb Mar 21, 2018
61bd312
Adds support for both .scss and .sass extensions.
Fabianopb Mar 21, 2018
69ce2e6
Uses ExtractTextPlugin with sass-loader to bundle styles for the prod…
Fabianopb Mar 21, 2018
0a81f2a
Bundles SASS modules for the production build.
Fabianopb Mar 21, 2018
5a7f534
Uses the latest version of sass-loader.
Fabianopb Mar 21, 2018
b2573f5
Adds function to create different rules for style loaders in dev envi…
Fabianopb Mar 24, 2018
6e60b6e
Abstracts style loaders to a common function to avoid repetition.
Fabianopb Mar 24, 2018
c8052c5
Simplifies the common function that creates style loaders.
Fabianopb Mar 24, 2018
876c9b3
Creates assets for testing SASS/SCSS support.
Fabianopb Mar 26, 2018
9865439
Creates mock components and unit tests for SASS and SCSS with and wit…
Fabianopb Mar 26, 2018
1629d78
Creates integration tests for SASS/SCSS support.
Fabianopb Mar 26, 2018
479423a
Adds node-sass as a template dependency so sass-loader can be tested.
Fabianopb Mar 26, 2018
dc640e4
Includes sass tests when test component is mounted.
Fabianopb Mar 27, 2018
f44250e
Fixes asserted module name for sass and scss modules tests.
Fabianopb Mar 27, 2018
0f36d90
Removes tests against css imports in SCSS and SASS files.
Fabianopb Mar 27, 2018
c68d168
Merge branch 'next' of https://github.com/facebook/create-react-app i…
Fabianopb Apr 6, 2018
7ba882e
Updates sass-loader to v7.
Fabianopb Apr 13, 2018
2311570
Merge branch 'next' of https://github.com/facebook/create-react-app i…
Fabianopb Apr 13, 2018
3657b78
Uses getCSSModuleLocalIdent from react-dev-utils.
Fabianopb Apr 14, 2018
5a49244
Fixes tests to match the use of getCSSModuleLocalIdent.
Fabianopb Apr 14, 2018
9e67bde
Improves readability of getStyleLoader function.
Fabianopb Apr 14, 2018
4c14a2f
Uses postcss after sass.
Fabianopb Apr 15, 2018
3b8bbb8
Refactors dev config to simplify common function for style loaders.
Fabianopb Apr 15, 2018
d9621fb
Refactors prod config to simplify common function for style loaders.
Fabianopb Apr 15, 2018
22475dc
Use importLoaders config according to css-loader docs.
Fabianopb Apr 17, 2018
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
85 changes: 56 additions & 29 deletions packages/react-scripts/config/webpack.config.dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,31 @@ const postCSSLoaderOptions = {
],
};

// style files regexes
const cssRegex = /\.css$/;
const cssModuleRegex = /\.module\.css$/;
const sassRegex = /\.(scss|sass)$/;
const sassModuleRegex = /\.module\.(scss|sass)$/;

// common function to get style loaders
const getStyleLoaders = (cssOptions, preProcessor) => {
const loaders = [
require.resolve('style-loader'),
{
loader: require.resolve('css-loader'),
options: cssOptions,
},
{
loader: require.resolve('postcss-loader'),
options: postCSSLoaderOptions,
},
];
if (preProcessor) {
loaders.push(require.resolve(preProcessor));
}
return loaders;
};

// This is the development configuration.
// It is focused on developer experience and fast rebuilds.
// The production configuration is different and lives in a separate file.
Expand Down Expand Up @@ -243,41 +268,43 @@ module.exports = {
// in development "style" loader enables hot editing of CSS.
// By default we support CSS Modules with the extension .module.css
{
test: /\.css$/,
exclude: /\.module\.css$/,
use: [
require.resolve('style-loader'),
{
loader: require.resolve('css-loader'),
options: {
importLoaders: 1,
},
},
{
loader: require.resolve('postcss-loader'),
options: postCSSLoaderOptions,
},
],
test: cssRegex,
exclude: cssModuleRegex,
use: getStyleLoaders({
importLoaders: 1,
}),
},
// Adds support for CSS Modules (https://github.com/css-modules/css-modules)
// using the extension .module.css
{
test: /\.module\.css$/,
use: [
require.resolve('style-loader'),
{
loader: require.resolve('css-loader'),
options: {
importLoaders: 1,
modules: true,
getLocalIdent: getCSSModuleLocalIdent,
},
},
test: cssModuleRegex,
use: getStyleLoaders({
importLoaders: 1,
modules: true,
getLocalIdent: getCSSModuleLocalIdent,
}),
},
// Opt-in support for SASS (using .scss or .sass extensions).
// Chains the sass-loader with the css-loader and the style-loader
// to immediately apply all styles to the DOM.
// By default we support SASS Modules with the
// extensions .module.scss or .module.sass
{
test: sassRegex,
exclude: sassModuleRegex,
use: getStyleLoaders({}, 'sass-loader'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is importLoaders: 1 not needed here? If not, can you please add a comment explaining why. It's not immediately obvious to me.

Copy link
Contributor Author

@Fabianopb Fabianopb Apr 15, 2018

Choose a reason for hiding this comment

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

@Timer, as far as I understood importLoaders here would allow e.g. to import SASS into CSS files, which is not common, and as SASS already handles imports, so this wouldn't be needed. Could anyone please confirm that just to see if I haven't misunderstood? webpack-contrib/css-loader#228

Copy link
Contributor

Choose a reason for hiding this comment

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

I could see someone importing a sass file in a css file; if there's no downsides/caveats we may as well add it and a corresponding test.

Copy link
Contributor Author

@Fabianopb Fabianopb Apr 15, 2018

Choose a reason for hiding this comment

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

In fact I've just tried importing a sass into a css and it works, so I wonder what's the use for importLoaders here?... Anyone with better understanding on the css-loader to give us a hint?

Choose a reason for hiding this comment

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

here is the original issue and attached diff adding this option, not sure if it'll help: webpack-contrib/css-loader#21

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated using {importLoaders: 2} according to css-loader docs: https://github.com/webpack-contrib/css-loader#importloaders

},
// Adds support for CSS Modules, but using SASS
// using the extension .module.scss or .module.sass
{
test: sassModuleRegex,
use: getStyleLoaders(
{
loader: require.resolve('postcss-loader'),
options: postCSSLoaderOptions,
modules: true,
getLocalIdent: getCSSModuleLocalIdent,
},
],
'sass-loader'
),
},
// The GraphQL loader preprocesses GraphQL queries in .graphql files.
{
Expand Down
147 changes: 89 additions & 58 deletions packages/react-scripts/config/webpack.config.prod.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,49 @@ const postCSSLoaderOptions = {
flexbox: 'no-2009',
}),
],
sourceMap: shouldUseSourceMap,
};

// style files regexes
const cssRegex = /\.css$/;
const cssModuleRegex = /\.module\.css$/;
const sassRegex = /\.(scss|sass)$/;
const sassModuleRegex = /\.module\.(scss|sass)$/;

// common function to get style loaders
const getStyleLoaders = (cssOptions, preProcessor) => {
const loaders = [
{
loader: require.resolve('css-loader'),
options: cssOptions,
},
{
loader: require.resolve('postcss-loader'),
options: postCSSLoaderOptions,
},
];
if (preProcessor) {
loaders.push({
loader: require.resolve(preProcessor),
options: {
sourceMap: shouldUseSourceMap,
},
});
}
return ExtractTextPlugin.extract(
Object.assign(
{
fallback: {
loader: require.resolve('style-loader'),
options: {
hmr: false,
},
},
use: loaders,
},
extractTextPluginOptions
)
);
};

// This is the production configuration.
Expand Down Expand Up @@ -255,69 +298,57 @@ module.exports = {
// in the main CSS file.
// By default we support CSS Modules with the extension .module.css
{
test: /\.css$/,
exclude: /\.module\.css$/,
loader: ExtractTextPlugin.extract(
Object.assign(
{
fallback: {
loader: require.resolve('style-loader'),
options: {
hmr: false,
},
},
use: [
{
loader: require.resolve('css-loader'),
options: {
importLoaders: 1,
minimize: true,
sourceMap: shouldUseSourceMap,
},
},
{
loader: require.resolve('postcss-loader'),
options: postCSSLoaderOptions,
},
],
},
extractTextPluginOptions
)
),
test: cssRegex,
exclude: cssModuleRegex,
loader: getStyleLoaders({
importLoaders: 1,
minimize: true,
sourceMap: shouldUseSourceMap,
}),
// Note: this won't work without `new ExtractTextPlugin()` in `plugins`.
},
// Adds support for CSS Modules (https://github.com/css-modules/css-modules)
// using the extension .module.css
{
test: /\.module\.css$/,
loader: ExtractTextPlugin.extract(
Object.assign(
{
fallback: {
loader: require.resolve('style-loader'),
options: {
hmr: false,
},
},
use: [
{
loader: require.resolve('css-loader'),
options: {
importLoaders: 1,
minimize: true,
sourceMap: shouldUseSourceMap,
modules: true,
getLocalIdent: getCSSModuleLocalIdent,
},
},
{
loader: require.resolve('postcss-loader'),
options: postCSSLoaderOptions,
},
],
},
extractTextPluginOptions
)
test: cssRegex,
loader: getStyleLoaders({
importLoaders: 1,
minimize: true,
sourceMap: shouldUseSourceMap,
modules: true,
getLocalIdent: getCSSModuleLocalIdent,
}),
// Note: this won't work without `new ExtractTextPlugin()` in `plugins`.
},
// Opt-in support for SASS. The logic here is somewhat similar
// as in the CSS routine, except that "sass-loader" runs first
// to compile SASS files into CSS.
// By default we support SASS Modules with the
// extensions .module.scss or .module.sass
{
test: sassRegex,
exclude: sassModuleRegex,
loader: getStyleLoaders(
{
minimize: true,
sourceMap: shouldUseSourceMap,
},
'sass-loader'
),
// Note: this won't work without `new ExtractTextPlugin()` in `plugins`.
},
// Adds support for CSS Modules, but using SASS
// using the extension .module.scss or .module.sass
{
test: sassModuleRegex,
loader: getStyleLoaders(
{
minimize: true,
sourceMap: shouldUseSourceMap,
modules: true,
getLocalIdent: getCSSModuleLocalIdent,
},
'sass-loader'
),
// Note: this won't work without `new ExtractTextPlugin()` in `plugins`.
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"chai": "3.5.0",
"jsdom": "9.8.3",
"mocha": "3.2.0",
"node-sass": "4.8.3",
"normalize.css": "7.0.0",
"prop-types": "15.5.6",
"test-integrity": "1.0.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,42 @@ describe('Integration', () => {
);
});

it('scss inclusion', async () => {
const doc = await initDOM('scss-inclusion');

expect(
doc.getElementsByTagName('style')[0].textContent.replace(/\s/g, '')
).to.match(/#feature-scss-inclusion\{background:.+;color:.+}/);
});

it('scss modules inclusion', async () => {
const doc = await initDOM('scss-modules-inclusion');

expect(
doc.getElementsByTagName('style')[0].textContent.replace(/\s/g, '')
).to.match(
/.+scss-styles_scssModulesInclusion.+\{background:.+;color:.+}/
);
});

it('sass inclusion', async () => {
const doc = await initDOM('sass-inclusion');

expect(
doc.getElementsByTagName('style')[0].textContent.replace(/\s/g, '')
).to.match(/#feature-sass-inclusion\{background:.+;color:.+}/);
});

it('sass modules inclusion', async () => {
const doc = await initDOM('sass-modules-inclusion');

expect(
doc.getElementsByTagName('style')[0].textContent.replace(/\s/g, '')
).to.match(
/.+sass-styles_sassModulesInclusion.+\{background:.+;color:.+}/
);
});

it('graphql files inclusion', async () => {
const doc = await initDOM('graphql-inclusion');
const children = doc.getElementById('graphql-inclusion').children;
Expand Down
20 changes: 20 additions & 0 deletions packages/react-scripts/fixtures/kitchensink/src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,26 @@ class App extends Component {
this.setFeature(f.default)
);
break;
case 'scss-inclusion':
import('./features/webpack/ScssInclusion').then(f =>
this.setFeature(f.default)
);
break;
case 'scss-modules-inclusion':
import('./features/webpack/ScssModulesInclusion').then(f =>
this.setFeature(f.default)
);
break;
case 'sass-inclusion':
import('./features/webpack/SassInclusion').then(f =>
this.setFeature(f.default)
);
break;
case 'sass-modules-inclusion':
import('./features/webpack/SassModulesInclusion').then(f =>
this.setFeature(f.default)
);
break;
case 'custom-interpolation':
import('./features/syntax/CustomInterpolation').then(f =>
this.setFeature(f.default)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* Copyright (c) 2015-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import React from 'react';
import './assets/sass-styles.sass';

export default () => <p id="feature-sass-inclusion">We love useless text.</p>;
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* Copyright (c) 2015-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import React from 'react';
import ReactDOM from 'react-dom';
import SassInclusion from './SassInclusion';

describe('sass inclusion', () => {
it('renders without crashing', () => {
const div = document.createElement('div');
ReactDOM.render(<SassInclusion />, div);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/**
* Copyright (c) 2015-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import React from 'react';
import styles from './assets/sass-styles.module.sass';

export default () => (
<p className={styles.sassModulesInclusion}>SASS Modules are working!</p>
);
Loading