Dealing with multiple modules defined in a file

Author: stackfull

lib/AngularModuleDefinition.js

@@ -7,21 +7,24 @@ var NullDependency = require('webpack/lib/dependencies/NullDependency');
 
 function AngularModuleDefinitionTemplate() {}
 
+// The definition template leaves the call to angular.module as-is, but assigns
+// the result to an export with the module name.
+//
 AngularModuleDefinitionTemplate.prototype = {
   constructor: AngularModuleDefinitionTemplate,
   apply: function(dep, source){
-    var localModuleVar = dep.localModule.variableName();
-    source.insert(0, "var "+localModuleVar+";\n");
-    source.replace(dep.range[0], dep.range[0]-1, "("+localModuleVar+" = module.exports = ");
+    source.replace(dep.range[0], dep.range[0]-1,
+                   "(module.exports['"+dep.name+"'] = ");
     source.replace(dep.range[1], dep.range[1]-1, ")");
   }
 };
 
 
-function AngularModuleDefinition(range){
+function AngularModuleDefinition(range, name){
   NullDependency.call(this);
   this.Class = AngularModuleDefinition;
   this.range = range;
+  this.name = name;
 }
 
 AngularModuleDefinition.prototype = Object.create(NullDependency.prototype);

lib/AngularModuleDependency.js

@@ -8,13 +8,17 @@ var ModuleDependency = require('webpack/lib/dependencies/ModuleDependency');
 
 function AngularModuleDependencyTemplate() {}
 
+// The dependency template leaves the dependency array as the original strings
+// so that angular can deal with out-of-order definitions. But a
+// __webpack_require__ is inserted at the top to make sure the file is pulled
+// in.
 AngularModuleDependencyTemplate.prototype = {
   constructor: AngularModuleDependencyTemplate,
   apply: function(dep, source, outputOptions, requestShortener){
     if( !dep.range ){
       return;
     }
-    var comment = "", content = "require(";
+    var comment = "", content = "__webpack_require__(";
     if(outputOptions.pathinfo){
       comment += "/*! " + requestShortener.shorten(dep.request) + " */ ";
     }
@@ -24,8 +28,8 @@ AngularModuleDependencyTemplate.prototype = {
       content += "!(function webpackMissingModule() { throw new Error(" +
         JSON.stringify("Cannot find module \"" + dep.request + "\"") + "); }())";
     }
-    content += ").name";
-    source.replace(dep.range[0], dep.range[1]-1, content);
+    content += ");\n";
+    source.insert(0, content);
   },
   applyAsTemplateArgument: function(name, dep, source){
     if( !dep.range ){

lib/index.js

@@ -7,38 +7,67 @@
 var path = require('path');
 
 var LocalModulesHelpers = require("webpack/lib/dependencies/LocalModulesHelpers");
-var LocalModuleDependency = require("webpack/lib/dependencies/LocalModuleDependency");
 var NullFactory = require("webpack/lib/NullFactory");
 var ModuleParserHelpers = require("webpack/lib/ModuleParserHelpers");
 var RequireHeaderDependency = require("webpack/lib/dependencies/RequireHeaderDependency");
 
 var AngularModuleDependency = require('./AngularModuleDependency');
 var AngularModuleDefinition = require('./AngularModuleDefinition');
 
-function AngularPlugin() {}
+function AngularPlugin() {
+}
 
 module.exports = AngularPlugin;
 
+function bindCallbackMethod(source, plugname, obj, method){
+  source.plugin(plugname, method.bind(obj, source));
+}
+
+function containsSlash(str){
+  return str.indexOf("/") >= 0 || str.indexOf("\\") >= 0;
+}
+
+// Try to resolve a file of the form /somepath/module/module
+// (calling it modmod for want of a better term)
+function resolveModModFile(resolver, request, callback){
+  var joined = path.join(request.request, request.request);
+  return resolver.doResolve("file", {
+    path: request.path,
+    request: joined,
+    query: request.query
+  }, callback, true);
+}
+
+function requestIsModModFile(request){
+  if( containsSlash(request.request) || ! request.file ){
+    return false;
+  }
+  var starting = request.path.length - request.request.length;
+  var match = request.path.lastIndexOf(request.request);
+  return match === starting &&
+    (request.path[starting-1] === '/' || request.path[starting-1] === '\\');
+}
 
 AngularPlugin.prototype = {
   constructor: AngularPlugin,
 
   // This is the entrypoint that is called when the plugin is added to a
   // compiler
   apply: function(compiler){
-    compiler.plugin("compilation", this.addDependencyFactories.bind(this));
-    compiler.parser.plugin('expression angular', function(){
-      return ModuleParserHelpers.addParsedVariable(this, 'angular',
-                                                   "require('angular')");
-    });
-    compiler.parser.plugin('call angular.module',
-                           this.parseModuleCall.bind(this, compiler.parser));
-    compiler.resolvers.normal.plugin('module-module',
-                                     this.resolveModule.bind(this, compiler.resolvers.normal));
+    bindCallbackMethod(compiler, "compilation",
+                       this, this.addDependencyFactories);
+    bindCallbackMethod(compiler.parser, "expression angular",
+                       this, this.addAngularVariable);
+    bindCallbackMethod(compiler.parser, "call angular.module",
+                       this, this.parseModuleCall);
+    bindCallbackMethod(compiler.resolvers.normal, "module-module",
+                       this, this.resolveModule);
   },
 
+  // #### Plugin Callbacks
+
   // This sets up the compiler with the right dependency module factories
-  addDependencyFactories: function(compilation, params){
+  addDependencyFactories: function(compiler, compilation, params){
     compilation.dependencyFactories.set(AngularModuleDependency,
                                         params.normalModuleFactory);
     compilation.dependencyTemplates.set(AngularModuleDependency,
@@ -49,89 +78,126 @@ AngularPlugin.prototype = {
                                         new AngularModuleDefinition.Template());
   },
 
+  // This injects the angular module wherever it's used.
+  addAngularVariable: function(parser) {
+    return ModuleParserHelpers.addParsedVariable(parser, 'angular', "require('angular')");
+  },
+
   // Each call to `angular.module()` is analysed here
   parseModuleCall: function(parser, expr){
-      var mod, deps;
-      ModuleParserHelpers.addParsedVariable(parser, 'angular', 
-                                            "require('angular')");
-      switch(expr.arguments.length){
-      // A single argument is the equivalent of `require()`
-      case 1:
-        mod = parser.evaluateExpression(expr.arguments[0]);
-        return this._addDependencyForParameter(parser, expr, mod);
-      // 2 arguments mean a module definition if the 2nd is an array.
-      case 2:
-        mod = parser.evaluateExpression(expr.arguments[0]);
-        // TODO: should we check the module name?
-        this._addModuleDefinition(parser, expr, mod);
-        deps = parser.evaluateExpression(expr.arguments[1]);
-        if( deps.items ){
-          return deps.items.every(
-            this._addDependencyForParameter.bind(this, parser, expr));
-        }
-        return this._addDependencyForParameter(parser, expr, mod);
-      // 3 arguments must be a module definition
-      case 3:
-        mod = parser.evaluateExpression(expr.arguments[0]);
-        // TODO: should we check the module name?
-        this._addModuleDefinition(parser, expr, mod);
-        deps = parser.evaluateExpression(expr.arguments[1]);
-        return deps.items.every(
-          this._addDependencyForParameter.bind(this, parser, expr));
+    this.addAngularVariable(parser);
+    switch(expr.arguments.length){
+      case 1: return this._parseModuleCallSingleArgument(parser, expr);
+      case 2: return this._parseModuleCallTwoArgument(parser, expr);
+      case 3: return this._parseModuleCallThreeArgument(parser, expr);
       default:
         console.warn("Don't recognise angular.module() with " +
                      expr.arguments.length + " args");
-      }
+    }
   },
 
+  // Additional module resolving specific to angular modules.
+  //
+  // We're trying to follow as many existing conventions as possible. Including:
+  // - dots as path separators
+  // - camelCase module names convert to dashed file names
+  // - module, directory and file names all the same.
+  // - files containing multiple modules (shared prefix)
+  //
   resolveModule: function(resolver, request, callback){
-    var fs = resolver.fileSystem;
-    var i = request.request.indexOf(".");
-    if(i < 0) {
-      // Try resolving a file of the form module/module
-      return resolver.doResolve("file", {
-        path: request.path,
-        request: path.join(request.request, request.request),
-        query: request.query
-      }, callback, true);
+    if( containsSlash(request.request) ){
+      return callback();
     }
-    // Try treating `.` as a path separator
-    var moduleName = request.request.substr(0, i);
-    var remainingRequest = request.request.substr(i+1);
-    var modulePath = resolver.join(request.path, moduleName);
-    fs.stat(modulePath, function(err, stat) {
-      if(err || !stat) {
-        if(callback.log){
-          callback.log(modulePath + " doesn't exist (ng module as directory)");
+    var split = request.request.split('.');
+    if( split.length === 1 ) {
+      if( ! requestIsModModFile(request) ){
+        return resolveModModFile(resolver, request, callback);
+      }
+    }else{
+      // Try treating `.` as a path separator, but in a non-greedy way.
+      // There are lots of options here because we allow the file name to be
+      // just a prefix.
+      // prefer the segments to be directories and prefer the full name to be
+      // used.
+      var pairs = [];
+      for( var j = 0; j > (0-split.length); j-- ){
+        var cropped = split.slice(0, j || undefined);
+        for( var i = 0; i < cropped.length; i++ ){
+          pairs.push({ front: cropped.slice(0, i), back: cropped.slice(i) });
         }
-        return callback();
       }
-      if(stat.isDirectory()) {
-        var types = request.directory ? "directory" : ["file", "directory"];
-        return resolver.doResolve(types, {
-          path: modulePath,
-          request: remainingRequest,
+      pairs.shift();
+      var requests = pairs.map(function(p){
+        return {
+          path: path.join.apply(path, [request.path].concat(p.front)),
+          request: p.back.join('.'),
           query: request.query
-        }, callback, true);
-      }
-      if(callback.log){
-        callback.log(modulePath + " is not a directory (ng module as directory)");
-      }
-      return callback();
-    });
+        };
+      });
+
+      return resolver.forEachBail(requests, function(req, cb){
+        return resolver.doResolve(["file", "directory"], req, cb, true);
+      }, function(err, resolved){
+        if( err || !resolved ){
+          return callback(err);
+        }
+        if( resolved.file ){
+          // We're taking this as resolved. It should contain other modules with
+          // this prefix
+          return resolver.doResolve("result", resolved, callback);
+        }
+        if(callback.log){
+          callback.log(" is not an angular module");
+        }
+        callback();
+      });
+    }
+    return callback();
   },
 
+  // #### Private Methods
+
+  // A single argument is the equivalent of `require()`
+  // angular.module(name)
+  _parseModuleCallSingleArgument: function(parser, expr) {
+    var mod = parser.evaluateExpression(expr.arguments[0]);
+    return this._addDependency(parser, expr, mod);
+  },
+
+  // 2 arguments mean a module definition if the 2nd is an array.
+  // angular.module(name, requires)
+  // angular.module(name, configFn)
+  _parseModuleCallTwoArgument: function(parser, expr) {
+    var mod = parser.evaluateExpression(expr.arguments[0]);
+    var deps = parser.evaluateExpression(expr.arguments[1]);
+    this._addModuleDefinition(parser, expr, mod);
+    if( deps.items ){
+      return deps.items.every(
+        this._addDependency.bind(this, parser, expr));
+    }
+    return this._addDependency(parser, expr, mod);
+  },
+
+  // 3 arguments must be a module definition
+  // angular.module(name, requires, configFn)
+  _parseModuleCallThreeArgument: function(parser, expr) {
+    var mod = parser.evaluateExpression(expr.arguments[0]);
+    var deps = parser.evaluateExpression(expr.arguments[1]);
+    this._addModuleDefinition(parser, expr, mod);
+    return deps.items.every(
+      this._addDependency.bind(this, parser, expr));
+  },
 
   _addModuleDefinition: function(parser, expr, mod){
-    var dep = new AngularModuleDefinition(expr.range);
+    var dep = new AngularModuleDefinition(expr.range, mod.string);
     dep.loc = expr.loc;
     dep.localModule = LocalModulesHelpers.addLocalModule(parser.state, mod.string);
     parser.state.current.addDependency(dep);
     return true;
   },
 
   // A dependency (module) has been found
-  _addDependencyForParameter: function(parser, expr, param){
+  _addDependency: function(parser, expr, param){
     if( param.isConditional() ){
       // TODO: not sure what this will output
       parser.state.current.addDependency(
@@ -148,16 +214,12 @@ AngularPlugin.prototype = {
     }
     if( param.isString() ){
       var dep;
-      var localModule = LocalModulesHelpers.getLocalModule(parser.state,
-                                                           param.string);
-      if( localModule ){
-        dep = new LocalModuleDependency(localModule, expr.range);
-        dep.loc = expr.loc;
-        parser.state.current.addDependency(dep);
+      var localModule = LocalModulesHelpers.getLocalModule(parser.state, param.string);
+      if( localModule ) {
         return true;
       }
       dep = new AngularModuleDependency(param.string, param.range);
-      dep.log = expr.loc;
+      dep.loc = param.loc;
       parser.state.current.addDependency(dep);
       return true;
     }

package.json

@@ -23,25 +23,26 @@
     "webpack": "~1.0.0"
   },
   "devDependencies": {
-    "grunt-contrib-jshint": "~0.9.0",
+    "grunt-contrib-jshint": "0.10.0",
     "grunt-contrib-watch": "~0.6.0",
     "load-grunt-tasks": "~0.4.0",
-    "time-grunt": "~0.2.0",
+    "time-grunt": "0.3.1",
     "jshint-stylish": "~0.1.3",
-    "webpack": "~1.0.0",
-    "async": "~0.2.10",
+    "webpack": "1.1.8",
+    "async": "0.7.0",
     "lodash": "~2.4.1",
     "underscore.string": "~2.3.3",
     "karma": "~0.12.0",
     "webpack-dev-server": "~1.2.4",
-    "karma-webpack": "~1.0.1",
+    "karma-webpack": "1.1.0",
     "grunt-karma": "~0.8.2",
-    "karma-jasmine": "~0.2.2",
+    "karma-jasmine": "0.1.5",
     "karma-phantomjs-launcher": "~0.1.2",
-    "exports-loader": "^0.6.2",
+    "exports-loader": "0.6.2",
     "uglify-js": "~2.4.12"
   },
   "scripts": {
     "test": "grunt webpackScenario"
-  }
+  },
+  "dependencies": {}
 }

tasks/webpack-scenario.js

@@ -81,6 +81,7 @@ module.exports = function(grunt){
               var actual = mockFs.files[p];
               actual = _.str.strRight(actual, IGNORE_BEFORE);
               var expected = grunt.file.read(abspath);
+              grunt.file.write('bundle.js', actual);
               var actualZ = normalise(actual);
               var expectedZ = normalise(expected);
               if( expectedZ !== actualZ){
@@ -101,7 +102,7 @@ module.exports = function(grunt){
             grunt.log.debug("compiled "+confFile+" OK.");
             cb();
           }else{
-            cb(new Error(errors.join('\n')));
+            cb(new Error("scenario failed"));
           }
         }
       });