From ede68cd02740ea726b807a1cf1c09d9bd2d42b56 Mon Sep 17 00:00:00 2001
From: David Glasser <glasser@meteor.com>
Date: Tue, 2 Apr 2013 15:18:49 -0700
Subject: [PATCH] bundle NPM modules correctly. tests mostly pass.

---
 tools/bundler.js       | 41 +++++++++++++++++------------------------
 tools/server/server.js | 41 +++++++++++++++++++++--------------------
 2 files changed, 38 insertions(+), 44 deletions(-)

diff --git a/tools/bundler.js b/tools/bundler.js
index 2a13a2c603..3402b9b84a 100644
--- a/tools/bundler.js
+++ b/tools/bundler.js
@@ -259,9 +259,7 @@ _.extend(Bundle.prototype, {
   // directory within the bundle
   bundleNodeModules: function (pkg) {
     var nodeModulesPath = path.join(pkg.npmDir(), 'node_modules');
-    // use '/' rather than path.join since this is part of a url
-    var relNodeModulesPath = ['packages', pkg.name, 'node_modules'].join('/');
-    this.nodeModulesDirs[relNodeModulesPath] = nodeModulesPath;
+    this.nodeModulesDirs[pkg.name] = nodeModulesPath;
   },
 
   // Sort the packages in dependency order, then, package by package,
@@ -579,28 +577,23 @@ _.extend(Bundle.prototype, {
     }
 
     // `node_modules` directories for packages
-    for (var rel_path in self.nodeModulesDirs) {
-      var path_in_bundle = path.join('app', rel_path);
-      var full_path = path.join(build_path, path_in_bundle);
-
-      // XXX it's bizarre that we would be trying to install npm
-      // modules into a non-existant path, but this happens when we
-      // have an npm dependency only used during bundle time (such as
-      // the less package). we should consider supporting bundle
-      // time-only npm dependencies.
-      if (fs.existsSync(path.dirname(full_path))) {
-        if (nodeModulesMode === 'symlink') {
-          // if we symlink the dev_bundle, also symlink individual package
-          // node_modules.
-          fs.symlinkSync(self.nodeModulesDirs[rel_path], full_path);
-        } else {
-          // otherwise, copy them. if we're skipping the dev_bundle
-          // modules (eg for deploy) we still need the per-package
-          // modules.
-          files.cp_r(self.nodeModulesDirs[rel_path], full_path);
-        }
+    _.each(self.nodeModulesDirs, function (sourceNodeModulesDir, packageName) {
+      files.mkdir_p(path.join(build_path, 'npm'));
+      var buildModulesPath = path.join(build_path, 'npm', packageName);
+      // XXX we should consider supporting bundle time-only npm dependencies
+      // which don't need to be pushed to the server.
+      if (nodeModulesMode === 'symlink') {
+        // if we symlink the dev_bundle, also symlink individual package
+        // node_modules.
+        fs.symlinkSync(sourceNodeModulesDir, buildModulesPath);
+      } else {
+        // otherwise, copy them. if we're skipping the dev_bundle
+        // modules (eg for deploy) we still need the per-package
+        // modules.
+        // XXX this breaks arch-specific modules. oh well.
+        files.cp_r(sourceNodeModulesDir, buildModulesPath);
       }
-    }
+    });
 
     var app_html = self._generate_app_html();
     fs.writeFileSync(path.join(build_path, 'app.html'), app_html);
diff --git a/tools/server/server.js b/tools/server/server.js
index 9bd710f918..d970dc22b7 100644
--- a/tools/server/server.js
+++ b/tools/server/server.js
@@ -237,28 +237,29 @@ var run = function () {
         // said npm module
         require: function(name) {
           var filePathParts = filename.split(path.sep);
-          if (filePathParts[0] !== 'app' || filePathParts[1] !== 'packages') { // XXX it's weird that we're dependent on the dir structure
+          // XXX it's weird that we're dependent on the dir structure
+          if (!(filePathParts[0] === 'app'
+                && (filePathParts[1] === 'packages'
+                    || filePathParts[1] === 'package-tests'))) {
             return require(name); // current no support for npm outside packages. load from dev bundle only
-          } else {
-            var nodeModuleDir = path.join(
-              __dirname,
-              '..' /* get out of server/ */,
-              'app' /* === filePathParts[0] */,
-              'packages' /* === filePathParts[1] */,
-              filePathParts[2] /* package name */,
-              'node_modules',
-              name);
+          }
+          var packageName = filePathParts[2].replace(/\.js$/, '');
+          var nodeModuleDir = path.join(
+            __dirname,
+            '..', // get out of server
+            'npm', packageName, name);
 
-            if (fs.existsSync(nodeModuleDir)) {
-              return require(nodeModuleDir);
-            } else {
-              try {
-                return require(name);
-              } catch (e) {
-                // XXX better message
-                throw new Error("Can't find npm module '" + name + "'. Did you forget to call 'Npm.depends' in package.js within the '" + filePathParts[2] + "' package?");
-              }
-            }
+          if (fs.existsSync(nodeModuleDir)) {
+            return require(nodeModuleDir);
+          }
+          try {
+            return require(name);
+          } catch (e) {
+            // XXX better message
+            throw new Error(
+              "Can't find npm module '" + name +
+                "'. Did you forget to call 'Npm.depends' in package.js " +
+                "within the '" + packageName + "' package?");
           }
         }
       };