Merge pull request #336 from serverless-heaven/refactor-configuration

Refactor configuration

README.md

@@ -29,6 +29,7 @@ WebPack's [Tree-Shaking][link-webpack-tree] optimization.
 ## Recent improvements and important changes
 
 * Webpack 2 support has been dropped in favor of Webpack 4
+* Cleaned up configuration. You should now use a `custom.webpack` object to configure everything relevant for the plugin. The old configuration still works but will be removed in the next major release. For details see below.
 * This 5.0.0 prerelease is based on the current 4.4.0
 
 For the complete release notes see the end of this document.
@@ -48,12 +49,27 @@ plugins:
 
 ## Configure
 
-By default the plugin will look for a `webpack.config.js` in the service directory.
-Alternatively, you can specify a different file or configuration in `serverless.yml`:
+The configuration of the plugin is done by defining a `custom: webpack` object in your `serverless.yml` with your specific configuration. All settings are optional and will be set to reasonable defaults if missing.
+
+See the sections below for detailed descriptions of the settings. The defaults are:
+
+```yaml
+custom:
+  webpack:
+    webpackConfig: 'webpack.config.js'   # Name of webpack configuration file
+    webpackIncludeModules: false   # Node modules configuration for packaging
+    packager: 'npm'   # Reserved for future use. Any other values will not work right now.
+    packExternalModulesMaxBuffer: 200 * 1024   # Size of stdio buffers for spawned child processes
+```
+
+### Webpack configuration file
+
+By default the plugin will look for a `webpack.config.js` in the service directory. Alternatively, you can specify a different file or configuration in `serverless.yml`.
 
 ```yaml
 custom:
-  webpack: ./folder/my-webpack.config.js
+  webpack:
+    webpackConfig: ./folder/my-webpack.config.js
 ```
 
 A base Webpack configuration might look like this:
@@ -209,7 +225,8 @@ module.exports = {
 ```yaml
 # serverless.yml
 custom:
-  webpackIncludeModules: true # enable auto-packing of external modules
+  webpack:
+    webpackIncludeModules: true # enable auto-packing of external modules
 ```
 
 
@@ -223,8 +240,9 @@ use a different package file, set `packagePath` to your custom `package.json`:
 ```yaml
 # serverless.yml
 custom:
-  webpackIncludeModules:
-    packagePath: '../package.json' # relative path to custom package.json file.
+  webpack:
+    webpackIncludeModules:
+      packagePath: '../package.json' # relative path to custom package.json file.
 ```
 > Note that only relative path is supported at the moment.
 
@@ -246,10 +264,11 @@ your service's production dependencies in `package.json`.
 ```yaml
 # serverless.yml
 custom:
-  webpackIncludeModules:
-    forceInclude:
-      - module1
-      - module2
+  webpack:
+    webpackIncludeModules:
+      forceInclude:
+        - module1
+        - module2
 ```
 
 #### Forced exclusion
@@ -262,10 +281,11 @@ Just add them to the `forceExclude` array property and they will not be packaged
 ```yaml
 # serverless.yml
 custom:
-  webpackIncludeModules:
-    forceExclude:
-      - module1
-      - module2
+  webpack:
+    webpackIncludeModules:
+      forceExclude:
+        - module1
+        - module2
 ```
 
 If you specify a module in both arrays, `forceInclude` and `forceExclude`, the

lib/Configuration.js

@@ -0,0 +1,78 @@
+'use strict';
+/**
+ * Plugin configuration.
+ */
+
+const _ = require('lodash');
+
+/** 
+ * Plugin defaults
+ */
+const DefaultConfig = {
+  webpackConfig: 'webpack.config.js',
+  webpackIncludeModules: false,
+  packager: 'npm',
+  packExternalModulesMaxBuffer: 200 * 1024,
+  config: null
+};
+
+class Configuration {
+
+  constructor(custom) {
+
+    this._config = {};
+    this._hasLegacyConfig = false;
+
+    // Set configuration from sls.service.custom. We fall back to the
+    // old configuration to keep backwards compatibility.
+    if (custom) {
+      if (custom.webpackIncludeModules) {
+        this._config.webpackIncludeModules = custom.webpackIncludeModules;
+        this._hasLegacyConfig = true;
+      }
+      if (custom.packExternalModulesMaxBuffer) {
+        this._config.packExternalModulesMaxBuffer = custom.packExternalModulesMaxBuffer;
+        this._hasLegacyConfig = true;
+      }
+      if (_.isString(custom.webpack)) {
+        this._config.webpackConfig = custom.webpack;
+        this._hasLegacyConfig = true;
+      } else {
+        _.assign(this._config, custom.webpack || {});
+      }
+    }
+
+    // Set defaults for all missing properties
+    _.defaults(this._config, DefaultConfig);
+  }
+
+  get webpackConfig() {
+    return this._config.webpackConfig;
+  }
+
+  get webpackIncludeModules() {
+    return this._config.webpackIncludeModules;
+  }
+
+  get packExternalModulesMaxBuffer() {
+    return this._config.packExternalModulesMaxBuffer;
+  }
+
+  get packager() {
+    return this._config.packager;
+  }
+
+  get config() {
+    return this._config.config;
+  }
+
+  get hasLegacyConfig() {
+    return this._hasLegacyConfig;
+  }
+
+  toJSON() {
+    return _.omitBy(this._config, _.isNil);
+  }
+}
+
+module.exports = Configuration;

lib/Configuration.test.js

@@ -0,0 +1,116 @@
+'use strict';
+/**
+ * Unit tests for Configuration.
+ */
+
+const chai = require('chai');
+const Configuration = require('./Configuration');
+
+const expect = chai.expect;
+
+describe('Configuration', () => {
+  describe('defaults', () => {
+    let expectedDefaults;
+
+    before(() => {
+      expectedDefaults = {
+        webpackConfig: 'webpack.config.js',
+        webpackIncludeModules: false,
+        packager: 'npm',
+        packExternalModulesMaxBuffer: 200 * 1024,
+        config: null
+      };
+    });
+
+    it('should set default configuration without custom', () => {
+      const config = new Configuration();
+      expect(config).to.have.a.property('_config').that.deep.equals(expectedDefaults);
+      expect(config).to.have.a.property('hasLegacyConfig').that.is.false;
+    });
+
+    it('should set default configuration without webpack property', () => {
+      const config = new Configuration({});
+      expect(config).to.have.a.property('_config').that.deep.equals(expectedDefaults);
+      expect(config).to.have.a.property('hasLegacyConfig').that.is.false;
+    });
+  });
+
+  describe('with legacy configuration', () => {
+    it('should use custom.webpackIncludeModules', () => {
+      const testCustom = { webpackIncludeModules: { forceInclude: ['mod1'] } };
+      const config = new Configuration(testCustom);
+      expect(config).to.have.a.property('webpackIncludeModules').that.deep.equals(testCustom.webpackIncludeModules);
+    });
+
+    it('should use custom.packExternalModulesMaxBuffer', () => {
+      const testCustom = { packExternalModulesMaxBuffer: 4711 };
+      const config = new Configuration(testCustom);
+      expect(config).to.have.a.property('packExternalModulesMaxBuffer').that.equals(4711);
+    });
+
+    it('should use custom.webpack as string', () => {
+      const testCustom = { webpack: 'myWebpackFile.js' };
+      const config = new Configuration(testCustom);
+      expect(config).to.have.a.property('webpackConfig').that.equals('myWebpackFile.js');
+    });
+
+    it('should detect it', () => {
+      const testCustom = { webpack: 'myWebpackFile.js' };
+      const config = new Configuration(testCustom);
+      expect(config).to.have.a.property('hasLegacyConfig').that.is.true;
+    });
+
+    it('should add defaults', () => {
+      const testCustom = { 
+        webpackIncludeModules: { forceInclude: ['mod1'] },
+        webpack: 'myWebpackFile.js'
+      };
+      const config = new Configuration(testCustom);
+      expect(config).to.have.a.property('webpackIncludeModules').that.deep.equals(testCustom.webpackIncludeModules);
+      expect(config._config).to.deep.equal({
+        webpackConfig: 'myWebpackFile.js',
+        webpackIncludeModules: { forceInclude: ['mod1'] },
+        packager: 'npm',
+        packExternalModulesMaxBuffer: 200 * 1024,
+        config: null
+      });
+    });
+  });
+
+  describe('with a configuration object', () => {
+    it('should use it and add any defaults', () => {
+      const testCustom = {
+        webpack: {
+          webpackIncludeModules: { forceInclude: ['mod1'] },
+          webpackConfig: 'myWebpackFile.js'
+        }
+      };
+      const config = new Configuration(testCustom);
+      expect(config._config).to.deep.equal({
+        webpackConfig: 'myWebpackFile.js',
+        webpackIncludeModules: { forceInclude: ['mod1'] },
+        packager: 'npm',
+        packExternalModulesMaxBuffer: 200 * 1024,
+        config: null
+      });
+    });
+
+    it('should favor new configuration', () => {
+      const testCustom = {
+        webpackIncludeModules: { forceExclude: ['mod2'] },
+        webpack: {
+          webpackIncludeModules: { forceInclude: ['mod1'] },
+          webpackConfig: 'myWebpackFile.js'
+        }
+      };
+      const config = new Configuration(testCustom);
+      expect(config._config).to.deep.equal({
+        webpackConfig: 'myWebpackFile.js',
+        webpackIncludeModules: { forceInclude: ['mod1'] },
+        packager: 'npm',
+        packExternalModulesMaxBuffer: 200 * 1024,
+        config: null
+      });
+    });
+  });
+});

lib/packExternalModules.js

@@ -174,10 +174,7 @@ module.exports = {
 
     const stats = this.compileStats;
 
-    const includes = (
-      this.serverless.service.custom &&
-      this.serverless.service.custom.webpackIncludeModules
-    );
+    const includes = this.configuration.webpackIncludeModules;
 
     if (!includes) {
       return BbPromise.resolve();
@@ -190,11 +187,11 @@ module.exports = {
     const packageJsonPath = path.join(process.cwd(), packagePath);
 
     // Determine and create packager
-    return BbPromise.try(() => Packagers.get.call(this, 'npm'))
+    return BbPromise.try(() => Packagers.get.call(this, this.configuration.packager))
     .then(packager => {
       // Get first level dependency graph
       this.options.verbose && this.serverless.cli.log(`Fetch dependency graph from ${packageJsonPath}`);
-      const maxExecBufferSize = this.serverless.service.custom.packExternalModulesMaxBuffer || 200 * 1024;
+      const maxExecBufferSize = this.configuration.packExternalModulesMaxBuffer;
 
       return packager.getProdDependencies(path.dirname(packageJsonPath), 1, maxExecBufferSize)
       .then(dependencyGraph => {

lib/validate.js

@@ -6,6 +6,7 @@ const fse = require('fs-extra');
 const glob = require('glob');
 const lib = require('./index');
 const _ = require('lodash');
+const Configuration = require('./Configuration');
 
 /**
  * For automatic entry detection we sort the found files to solve ambiguities.
@@ -74,11 +75,14 @@ module.exports = {
       };
     };
 
-    this.webpackConfig = (
-      this.serverless.service.custom &&
-      this.serverless.service.custom.webpack ||
-      'webpack.config.js'
-    );
+    // Initialize plugin configuration
+    this.configuration = new Configuration(this.serverless.service.custom);
+    this.options.verbose && this.serverless.cli.log(`Using configuration:\n${JSON.stringify(this.configuration, null, 2)}`);
+    if (this.configuration.hasLegacyConfig) {
+      this.serverless.cli.log('Legacy configuration detected. Consider to use "custom.webpack" as object (see README).');
+    }
+
+    this.webpackConfig = this.configuration.config || this.configuration.webpackConfig;
 
     // Expose entries - must be done before requiring the webpack configuration
     const entries = {};