refactor: lint dirty modules plugin (#59)

* refactor: use filter on filetimestamps' keys

as opposed to reduce. also use lodash.defaultto for comparing numeric
values to avoid falsy JS bugs

* refactor: use Arrange Act Assert model for tests

and clean up some assertions and specs

* refactor: pulls out defaultFilesGlob constant

* refactor: clean up typos

* refactor: use .test.js file suffix

to enable the recursive flag in mocha.

* refactor: better names for all fixtures

* tests: add coverage for without plugin config

* tests: update NoErrorsPlugin tests

Verify that existing behaviour works as expected

* refactor: minor test tweaks

* tests: correct test of empty plugin config

* tests: remove extra context from lint dirty modules plugin

Author: JaKXz

index.js

@@ -9,6 +9,7 @@ var formatter = require('stylelint').formatters.string;
 // Modules
 var runCompilation = require('./lib/run-compilation');
 var LintDirtyModulesPlugin = require('./lib/lint-dirty-modules-plugin');
+var defaultFilesGlob = require('./lib/constants').defaultFilesGlob;
 
 function apply (options, compiler) {
   options = options || {};
@@ -19,7 +20,7 @@ function apply (options, compiler) {
   }, options, {
     // Default Glob is any directory level of scss and/or sass file,
     // under webpack's context and specificity changed via globbing patterns
-    files: arrify(options.files || '**/*.s?(c|a)ss').map(function (file) {
+    files: arrify(options.files || defaultFilesGlob).map(function (file) {
       return path.join(context, '/', file);
     }),
     context: context

lib/constants.js

@@ -0,0 +1,5 @@
+'use strict';
+
+module.exports = {
+  defaultFilesGlob: '**/*.s?(c|a)ss'
+};

lib/lint-dirty-modules-plugin.js

@@ -1,25 +1,25 @@
 'use strict';
-var minimatch = require('minimatch');
-var reduce = require('lodash.reduce');
+
 var assign = require('object-assign');
+var defaultTo = require('lodash.defaultto');
+var minimatch = require('minimatch');
 var runCompilation = require('./run-compilation');
 
 /**
  * Binds callback with provided options and stores initial values.
  *
  * @param compiler - webpack compiler object
  * @param options - stylelint nodejs options
- * @param callback <function(options, compilitaion)> - callback to call on emit
  */
 function LintDirtyModulesPlugin (compiler, options) {
   this.startTime = Date.now();
   this.prevTimestamps = {};
   this.isFirstRun = true;
   this.compiler = compiler;
   this.options = options;
-  compiler.plugin('emit',
-    this.lint.bind(this) // bind(this) is here to prevent context overriding by webpack
-  );
+
+  // bind(this) is here to prevent context overriding by webpack
+  compiler.plugin('emit', this.lint.bind(this));
 }
 
 /**
@@ -31,16 +31,18 @@ function LintDirtyModulesPlugin (compiler, options) {
  * @param callback - to be called when execution is done
  * @returns {*}
  */
-LintDirtyModulesPlugin.prototype.lint = function (compilation, callback) {
+LintDirtyModulesPlugin.prototype.lint = function lint (compilation, callback) {
   if (this.isFirstRun) {
     this.isFirstRun = false;
     this.prevTimestamps = compilation.fileTimestamps;
     return callback();
   }
+
   var dirtyOptions = assign({}, this.options);
   var glob = dirtyOptions.files.join('|');
   var changedFiles = this.getChangedFiles(compilation.fileTimestamps, glob);
   this.prevTimestamps = compilation.fileTimestamps;
+
   if (changedFiles.length) {
     dirtyOptions.files = changedFiles;
     runCompilation.call(this, dirtyOptions, this.compiler, callback);
@@ -57,18 +59,16 @@ LintDirtyModulesPlugin.prototype.lint = function (compilation, callback) {
  * @param fileTimestamps - an object with keys as filenames and values as their timestamps.
  * e.g. {'/filename.scss': 12444222000}
  * @param glob - glob pattern to match files
+ * @returns {Array} list of globs that contain changed files
  */
-LintDirtyModulesPlugin.prototype.getChangedFiles = function (fileTimestamps, glob) {
-  return reduce(fileTimestamps, function (changedStyleFiles, timestamp, filename) {
-    // Check if file has been changed first ...
-    if ((this.prevTimestamps[filename] || this.startTime) < (fileTimestamps[filename] || Infinity) &&
-      // ... then validate by the glob pattern.
-      minimatch(filename, glob, {matchBase: true})
-    ) {
-      changedStyleFiles = changedStyleFiles.concat(filename);
-    }
-    return changedStyleFiles;
-  }.bind(this), []);
+LintDirtyModulesPlugin.prototype.getChangedFiles = function getChangedFiles (fileTimestamps, glob) {
+  return Object.keys(fileTimestamps).filter(function (filename) {
+    return hasFileChanged.call(this, filename, fileTimestamps[filename]) && minimatch(filename, glob, {matchBase: true});
+  }.bind(this));
 };
 
+function hasFileChanged (filename, timestamp) {
+  return defaultTo(this.prevTimestamps[filename], this.startTime) < defaultTo(timestamp, Infinity);
+}
+
 module.exports = LintDirtyModulesPlugin;

package.json

@@ -33,7 +33,7 @@
   "dependencies": {
     "arrify": "^1.0.1",
     "chalk": "^1.1.3",
-    "lodash.reduce": "^4.6.0",
+    "lodash.defaultto": "^4.14.0",
     "minimatch": "^3.0.3",
     "object-assign": "^4.1.0",
     "stylelint": "^7.7.0"