bug fixes for watchify

joshwnj · joshwnj · commit eb116727ea52 · 2015-12-01T20:55:41.000+11:00
+ using a local copy of file-system-loader for now (will move over to
css-modules-loader-core later)
+ file-system-loader uses dependency-graph to record dependencies
+ removed dedupeSources (no longer needed)
+ global transform is optional (false by default)
+ updated tests to match
diff --git a/file-system-loader.js b/file-system-loader.js
@@ -0,0 +1,136 @@
+'use strict';
+
+var DepGraph = require('dependency-graph').DepGraph;
+
+Object.defineProperty(exports, '__esModule', {
+  value: true
+});
+
+var _createClass = (function () { function defineProperties(target, props) { for (var i = 0; i < props.length; i++) { var descriptor = props[i]; descriptor.enumerable = descriptor.enumerable || false; descriptor.configurable = true; if ('value' in descriptor) descriptor.writable = true; Object.defineProperty(target, descriptor.key, descriptor); } } return function (Constructor, protoProps, staticProps) { if (protoProps) defineProperties(Constructor.prototype, protoProps); if (staticProps) defineProperties(Constructor, staticProps); return Constructor; }; })();
+
+function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { 'default': obj }; }
+
+function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError('Cannot call a class as a function'); } }
+
+var _indexJs = require('css-modules-loader-core/lib/index.js');
+
+var _indexJs2 = _interopRequireDefault(_indexJs);
+
+var _fs = require('fs');
+
+var _fs2 = _interopRequireDefault(_fs);
+
+var _path = require('path');
+
+var _path2 = _interopRequireDefault(_path);
+
+// Sorts dependencies in the following way:
+// AAA comes before AA and A
+// AB comes after AA and before A
+// All Bs come after all As
+// This ensures that the files are always returned in the following order:
+// - In the order they were required, except
+// - After all their dependencies
+var traceKeySorter = function traceKeySorter(a, b) {
+  if (a.length < b.length) {
+    return a < b.substring(0, a.length) ? -1 : 1;
+  } else if (a.length > b.length) {
+    return a.substring(0, b.length) <= b ? -1 : 1;
+  } else {
+    return a < b ? -1 : 1;
+  }
+};
+
+var FileSystemLoader = (function () {
+  function FileSystemLoader(root, plugins) {
+    _classCallCheck(this, FileSystemLoader);
+
+    this.root = root;
+    this.sources = {};
+    this.traces = {};
+    this.importNr = 0;
+    this.core = new _indexJs2['default'](plugins);
+    this.tokensByFile = {};
+    this.deps = new DepGraph();
+  }
+
+  _createClass(FileSystemLoader, [{
+    key: 'fetch',
+    value: function fetch(_newPath, relativeTo, _trace) {
+      var _this = this;
+
+      var newPath = _newPath.replace(/^["']|["']$/g, ''),
+          trace = _trace || String.fromCharCode(this.importNr++);
+      return new Promise(function (resolve, reject) {
+        var relativeDir = _path2['default'].dirname(relativeTo),
+            rootRelativePath = _path2['default'].resolve(relativeDir, newPath),
+            fileRelativePath = _path2['default'].resolve(_path2['default'].join(_this.root, relativeDir), newPath);
+
+        // if the path is not relative or absolute, try to resolve it in node_modules
+        if (newPath[0] !== '.' && newPath[0] !== '/') {
+          try {
+            fileRelativePath = require.resolve(newPath);
+          } catch (e) {}
+        }
+
+        // first time? add a node
+        if (_trace === undefined) {
+          if (!_this.deps.hasNode(fileRelativePath)) {
+            _this.deps.addNode(fileRelativePath);
+          }
+        }
+        // otherwise add a dependency
+        else {
+          var parentFilePath = _path2['default'].join(_this.root, relativeTo);
+          if (!_this.deps.hasNode(parentFilePath)) {
+            console.error('NO NODE', parentFilePath, fileRelativePath)
+          }
+          if (!_this.deps.hasNode(fileRelativePath)) {
+            _this.deps.addNode(fileRelativePath);
+          }
+          _this.deps.addDependency(parentFilePath, fileRelativePath);
+        }
+
+        var tokens = _this.tokensByFile[fileRelativePath];
+        if (tokens) {
+          return resolve(tokens);
+        }
+
+        _fs2['default'].readFile(fileRelativePath, 'utf-8', function (err, source) {
+          if (err) reject(err);
+          _this.core.load(source, rootRelativePath, trace, _this.fetch.bind(_this)).then(function (_ref) {
+            var injectableSource = _ref.injectableSource;
+            var exportTokens = _ref.exportTokens;
+
+            _this.sources[fileRelativePath] = injectableSource;
+            _this.traces[trace] = fileRelativePath;
+            _this.tokensByFile[fileRelativePath] = exportTokens;
+            resolve(exportTokens);
+          }, reject);
+        });
+      });
+    }
+  }, {
+    key: 'finalSource',
+    get: function () {
+      var traces = this.traces;
+      var sources = this.sources;
+      var written = new Set();
+
+      return Object.keys(traces).sort(traceKeySorter).map(function (key) {
+        var filename = traces[key];
+        if (written.has(filename)) {
+          return null;
+        }
+        written.add(filename);
+
+        return sources[filename];
+      }).join('');
+    }
+  }]);
+
+  return FileSystemLoader;
+})();
+
+exports['default'] = FileSystemLoader;
+module.exports = exports['default'];
diff --git a/index.js b/index.js
@@ -5,7 +5,7 @@ var fs = require('fs');
 var path = require('path');
 var Cmify = require('./cmify');
 var Core = require('css-modules-loader-core');
-var FileSystemLoader = require('css-modules-loader-core/lib/file-system-loader');
+var FileSystemLoader = require('./file-system-loader');
 var assign = require('object-assign');
 var stringHash = require('string-hash');
 var ReadableStream = require('stream').Readable;
@@ -16,7 +16,7 @@ var ReadableStream = require('stream').Readable;
 */
 function generateShortName (name, filename, css) {
   // first occurrence of the name
-  // TOOD: better match with regex
+  // TODO: better match with regex
   var i = css.indexOf('.' + name);
   var numLines = css.substr(0, i).split(/[\r\n]/).length;
 
@@ -74,19 +74,6 @@ function normalizeManifestPaths (tokensByFile, rootDir) {
   return output;
 }
 
-function dedupeSources (sources) {
-  var foundHashes = {}
-  Object.keys(sources).forEach(function (key) {
-    var hash = stringHash(sources[key]);
-    if (foundHashes[hash]) {
-      delete sources[key];
-    }
-    else {
-      foundHashes[hash] = true;
-    }
-  })
-}
-
 // caches
 //
 // persist these for as long as the process is running. #32
@@ -105,6 +92,11 @@ module.exports = function (browserify, options) {
   if (rootDir) { rootDir = path.resolve(rootDir); }
   if (!rootDir) { rootDir = process.cwd(); }
 
+  var transformOpts = {};
+  if (options.global) {
+    transformOpts.global = true;
+  }
+
   var cssOutFilename = options.output || options.o;
   var jsonOutFilename = options.json || options.jsonOutput;
   var sourceKey = cssOutFilename;
@@ -173,13 +165,8 @@ module.exports = function (browserify, options) {
     tokensByFile[filename] = loader.tokensByFile[filename] = null;
 
     loader.fetch(relFilename, '/').then(function (tokens) {
-      var newFiles = Object.keys(loader.tokensByFile)
-      var oldFiles = Object.keys(tokensByFile)
-      var diff = newFiles.filter(function (f) {
-        return oldFiles.indexOf(f) === -1
-      })
-
-      var output = diff.map(function (f) {
+      var deps = loader.deps.dependenciesOf(filename);
+      var output = deps.map(function (f) {
         return "require('" + f + "')\n"
       }) + '\n\n' + 'module.exports = ' + JSON.stringify(tokens);
 
@@ -193,7 +180,7 @@ module.exports = function (browserify, options) {
     });
   };
 
-  browserify.transform(Cmify);
+  browserify.transform(Cmify, transformOpts);
 
   // ----
 
@@ -205,10 +192,6 @@ module.exports = function (browserify, options) {
     bundle.emit('css stream', compiledCssStream);
 
     bundle.on('end', function () {
-      // under certain conditions (eg. with shared libraries) we can end up with
-      // multiple occurrences of the same rule, so we need to remove duplicates
-      dedupeSources(loader.sources)
-
       // Combine the collected sources for a single bundle into a single CSS file
       var css = loader.finalSource;
 
diff --git a/package.json b/package.json
@@ -5,6 +5,7 @@
   "main": "index.js",
   "dependencies": {
     "css-modules-loader-core": "^1.0.0",
+    "dependency-graph": "^0.4.1",
     "object-assign": "^3.0.0",
     "promise-polyfill": "^2.1.0",
     "string-hash": "^1.1.0",
diff --git a/tests/cases/compose-node-module/expected.css b/tests/cases/compose-node-module/expected.css
@@ -1,4 +1,4 @@
-._cool_styles_styles__foo {
+._node_modules_cool_styles_styles__foo {
   color: #F00;
 }
 ._styles__foo {
diff --git a/tests/index.js b/tests/index.js
@@ -8,6 +8,8 @@ var path = require('path');
 var casesDir = path.join(__dirname, 'cases');
 var cssOutFilename = 'out.css';
 
+var globalCases = ['compose-node-module', 'import-node-module'];
+
 function runTestCase (dir) {
   tape('case: ' + dir, function (t) {
     var fakeFs = {
@@ -30,6 +32,9 @@ function runTestCase (dir) {
       rootDir: path.join(casesDir, dir)
       , output: cssOutFilename
       , generateScopedName: cssModulesify.generateLongName
+
+      // only certain cases will use a global transform
+      , global: globalCases.indexOf(dir) !== -1
     });
 
     b.bundle(function (err) {

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-._cool_styles_styles__foo {`
	`1`	`+._node_modules_cool_styles_styles__foo {`
`2`	`2`	`color: #F00;`
`3`	`3`	`}`
`4`	`4`	`._styles__foo {`