Refactoring each loader a bit, adding a test for less.js (others are still a todo)

weaverryan · weaverryan · commit 15112b72e274 · 2017-06-15T14:07:56.000-04:00
diff --git a/lib/config-generator.js b/lib/config-generator.js
@@ -21,10 +21,10 @@ const missingLoaderTransformer = require('./friendly-errors/transformers/missing
 const missingLoaderFormatter = require('./friendly-errors/formatters/missing-loader');
 const missingPostCssConfigTransformer = require('./friendly-errors/transformers/missing-postcss-config');
 const missingPostCssConfigFormatter = require('./friendly-errors/formatters/missing-postcss-config');
-const cssLoaders = require('./loaders/css');
-const sassLoaders = require('./loaders/sass');
-const lessLoaders = require('./loaders/less');
-const babelLoaders = require('./loaders/babel');
+const cssLoaderUtil = require('./loaders/css');
+const sassLoaderUtil = require('./loaders/sass');
+const lessLoaderUtil = require('./loaders/less');
+const babelLoaderUtil = require('./loaders/babel');
 
 class ConfigGenerator {
     /**
@@ -106,13 +106,13 @@ class ConfigGenerator {
                 // match .js and .jsx
                 test: /\.jsx?$/,
                 exclude: /(node_modules|bower_components)/,
-                use: babelLoaders(this.webpackConfig)
+                use: babelLoaderUtil.getLoaders(this.webpackConfig)
             },
             {
                 test: /\.css$/,
                 use: ExtractTextPlugin.extract({
                     fallback: 'style-loader' + this.getSourceMapOption(),
-                    use: cssLoaders(this.webpackConfig)
+                    use: cssLoaderUtil.getLoaders(this.webpackConfig)
                 })
             },
             {
@@ -138,7 +138,7 @@ class ConfigGenerator {
                 test: /\.s[ac]ss$/,
                 use: ExtractTextPlugin.extract({
                     fallback: 'style-loader' + this.getSourceMapOption(),
-                    use: sassLoaders(this.webpackConfig)
+                    use: sassLoaderUtil.getLoaders(this.webpackConfig)
                 })
             });
         }
@@ -148,7 +148,7 @@ class ConfigGenerator {
                 test: /\.less/,
                 use: ExtractTextPlugin.extract({
                     fallback: 'style-loader' + this.getSourceMapOption(),
-                    use: lessLoaders(this.webpackConfig)
+                    use: lessLoaderUtil.getLoaders(this.webpackConfig)
                 })
             });
         }
diff --git a/lib/loaders/babel.js b/lib/loaders/babel.js
@@ -15,51 +15,53 @@ const loaderFeatures = require('../loader-features');
  * @param {WebpackConfig} webpackConfig
  * @return {Array} of loaders to use for Babel
  */
-module.exports = function(webpackConfig) {
-    let babelConfig = {
-        // improves performance by caching babel compiles
-        // we add this option ALWAYS
-        // https://github.com/babel/babel-loader#options
-        cacheDirectory: true
-    };
+module.exports = {
+    getLoaders(webpackConfig) {
+        let babelConfig = {
+            // improves performance by caching babel compiles
+            // we add this option ALWAYS
+            // https://github.com/babel/babel-loader#options
+            cacheDirectory: true
+        };
 
-    // configure babel (unless the user is specifying .babelrc)
-    // todo - add a sanity check for their babelrc contents
-    if (!webpackConfig.doesBabelRcFileExist()) {
-        Object.assign(babelConfig, {
-            presets: [
-                ['env', {
-                    // modules don't need to be transformed - webpack will parse
-                    // the modules for us. This is a performance improvement
-                    // https://babeljs.io/docs/plugins/preset-env/#optionsmodules
-                    modules: false,
-                    targets: {
-                        browsers: '> 1%',
-                        uglify: true
-                    },
-                    useBuiltIns: true
-                }]
-            ],
-        });
+        // configure babel (unless the user is specifying .babelrc)
+        // todo - add a sanity check for their babelrc contents
+        if (!webpackConfig.doesBabelRcFileExist()) {
+            Object.assign(babelConfig, {
+                presets: [
+                    ['env', {
+                        // modules don't need to be transformed - webpack will parse
+                        // the modules for us. This is a performance improvement
+                        // https://babeljs.io/docs/plugins/preset-env/#optionsmodules
+                        modules: false,
+                        targets: {
+                            browsers: '> 1%',
+                            uglify: true
+                        },
+                        useBuiltIns: true
+                    }]
+                ],
+            });
 
-        if (webpackConfig.useReact) {
-            loaderFeatures.ensureLoaderPackagesExist('react');
+            if (webpackConfig.useReact) {
+                loaderFeatures.ensureLoaderPackagesExist('react');
 
-            babelConfig.presets.push('react');
+                babelConfig.presets.push('react');
+            }
+
+            // allow for babel config to be controlled
+            webpackConfig.babelConfigurationCallback.apply(
+                // use babelConfig as the this variable
+                babelConfig,
+                [babelConfig]
+            );
         }
 
-        // allow for babel config to be controlled
-        webpackConfig.babelConfigurationCallback.apply(
-            // use babelConfig as the this variable
-            babelConfig,
-            [babelConfig]
-        );
+        return [
+            {
+                loader: 'babel-loader',
+                options: babelConfig
+            }
+        ];
     }
-
-    return [
-        {
-            loader: 'babel-loader',
-            options: babelConfig
-        }
-    ];
 };
diff --git a/lib/loaders/css.js b/lib/loaders/css.js
@@ -15,27 +15,29 @@ const loaderFeatures = require('../loader-features');
  * @param {WebpackConfig} webpackConfig
  * @return {Array} of loaders to use for CSS files
  */
-module.exports = function(webpackConfig) {
-    const cssLoaders = [
-        {
-            loader: 'css-loader',
-            options: {
-                minimize: webpackConfig.isProduction(),
-                sourceMap: webpackConfig.useSourceMaps
-            }
-        },
-    ];
+module.exports = {
+    getLoaders(webpackConfig) {
+        const cssLoaders = [
+            {
+                loader: 'css-loader',
+                options: {
+                    minimize: webpackConfig.isProduction(),
+                    sourceMap: webpackConfig.useSourceMaps
+                }
+            },
+        ];
 
-    if (webpackConfig.usePostCssLoader) {
-        loaderFeatures.ensureLoaderPackagesExist('postcss');
+        if (webpackConfig.usePostCssLoader) {
+            loaderFeatures.ensureLoaderPackagesExist('postcss');
 
-        cssLoaders.push({
-            loader: 'postcss-loader',
-            options: {
-                sourceMap: webpackConfig.useSourceMaps
-            }
-        });
-    }
+            cssLoaders.push({
+                loader: 'postcss-loader',
+                options: {
+                    sourceMap: webpackConfig.useSourceMaps
+                }
+            });
+        }
 
-    return cssLoaders;
+        return cssLoaders;
+    }
 };
diff --git a/lib/loaders/less.js b/lib/loaders/less.js
@@ -10,22 +10,24 @@
 'use strict';
 
 const loaderFeatures = require('../loader-features');
-const cssLoaders = require('./css');
+const cssLoader = require('./css');
 
 /**
  * @param {WebpackConfig} webpackConfig
  * @return {Array} of loaders to use for Less files
  */
-module.exports = function(webpackConfig) {
-    loaderFeatures.ensureLoaderPackagesExist('less');
+module.exports = {
+    getLoaders(webpackConfig) {
+        loaderFeatures.ensureLoaderPackagesExist('less');
 
-    return [
-        ...cssLoaders(webpackConfig),
-        {
-            loader: 'less-loader',
-            options: {
-                sourceMap: webpackConfig.useSourceMaps
-            }
-        },
-    ];
+        return [
+            ...cssLoader.getLoaders(webpackConfig),
+            {
+                loader: 'less-loader',
+                options: {
+                    sourceMap: webpackConfig.useSourceMaps
+                }
+            },
+        ];
+    }
 };
diff --git a/lib/loaders/sass.js b/lib/loaders/sass.js
@@ -10,35 +10,37 @@
 'use strict';
 
 const loaderFeatures = require('../loader-features');
-const cssLoaders = require('./css');
+const cssLoader = require('./css');
 
 /**
  * @param {WebpackConfig} webpackConfig
  * @return {Array} of loaders to use for Sass files
  */
-module.exports = function(webpackConfig) {
-    loaderFeatures.ensureLoaderPackagesExist('sass');
+module.exports = {
+    getLoaders(webpackConfig) {
+        loaderFeatures.ensureLoaderPackagesExist('sass');
+
+        const sassLoaders = [...cssLoader.getLoaders(webpackConfig)];
+        if (true === webpackConfig.sassOptions.resolve_url_loader) {
+            // responsible for resolving SASS url() paths
+            // without this, all url() paths must be relative to the
+            // entry file, not the file that contains the url()
+            sassLoaders.push({
+                loader: 'resolve-url-loader',
+                options: {
+                    sourceMap: webpackConfig.useSourceMaps
+                }
+            });
+        }
 
-    const sassLoaders = [...cssLoaders(webpackConfig)];
-    if (true === webpackConfig.sassOptions.resolve_url_loader) {
-        // responsible for resolving SASS url() paths
-        // without this, all url() paths must be relative to the
-        // entry file, not the file that contains the url()
         sassLoaders.push({
-            loader: 'resolve-url-loader',
+            loader: 'sass-loader',
             options: {
-                sourceMap: webpackConfig.useSourceMaps
+                // needed by the resolve-url-loader
+                sourceMap: (true === webpackConfig.sassOptions.resolve_url_loader) || webpackConfig.useSourceMaps
             }
         });
-    }
 
-    sassLoaders.push({
-        loader: 'sass-loader',
-        options: {
-            // needed by the resolve-url-loader
-            sourceMap: (true === webpackConfig.sassOptions.resolve_url_loader) || webpackConfig.useSourceMaps
-        }
-    });
-
-    return sassLoaders;
+        return sassLoaders;
+    }
 };
diff --git a/package.json b/package.json
@@ -62,6 +62,7 @@
     "nsp": "^2.6.3",
     "postcss-loader": "^1.3.3",
     "sass-loader": "^6.0.3",
+    "sinon": "^2.3.4",
     "zombie": "^5.0.5"
   }
 }
diff --git a/test/loaders/less.js b/test/loaders/less.js
@@ -0,0 +1,42 @@
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+'use strict';
+
+const expect = require('chai').expect;
+const WebpackConfig = require('../../lib/WebpackConfig');
+const RuntimeConfig = require('../../lib/config/RuntimeConfig');
+const lessLoader = require('../../lib/loaders/less');
+const cssLoader = require('../../lib/loaders/css');
+const sinon = require('sinon');
+
+function createConfig() {
+    const runtimeConfig = new RuntimeConfig();
+    runtimeConfig.context = __dirname;
+    runtimeConfig.babelRcFileExists = false;
+
+    return new WebpackConfig(runtimeConfig);
+}
+
+describe('loaders/less', () => {
+    it('getLoaders() basic usage', () => {
+        const config = createConfig();
+        config.enableSourceMaps(true);
+
+        // make the cssLoader return nothing
+        sinon.stub(cssLoader, 'getLoaders')
+            .callsFake(() => []);
+
+        const actualLoaders = lessLoader.getLoaders(config);
+        expect(actualLoaders).to.have.lengthOf(1);
+        expect(actualLoaders[0].options.sourceMap).to.be.true;
+
+        cssLoader.getLoaders.restore();
+    });
+});
diff --git a/yarn.lock b/yarn.lock

Original file line number	Diff line number	Diff line change
`@@ -62,6 +62,7 @@`
`62`	`62`	`"nsp": "^2.6.3",`
`63`	`63`	`"postcss-loader": "^1.3.3",`
`64`	`64`	`"sass-loader": "^6.0.3",`
	`65`	`+ "sinon": "^2.3.4",`
`65`	`66`	`"zombie": "^5.0.5"`
`66`	`67`	`}`
`67`	`68`	`}`