Merge branch 'release-1.3' into cordova-improvements

tools/isobuild/build-plugin.js

@@ -473,6 +473,17 @@ _.extend(exports.InputFile.prototype, {
    */
   error: function (options) {
     var self = this;
+    if (self.getFileOptions().lazy === true) {
+      // Files with fileOptions.lazy === true were not explicitly added to
+      // the source batch via api.addFiles or api.mainModule, so any
+      // compilation errors should not be fatal. Attempting compilation is
+      // still important for lazy files that might end up being imported
+      // later, which is why we defang the error here, instead of avoiding
+      // compilation preemptively. Note also that actual exceptions will
+      // still cause build errors.
+      return;
+    }
+
     var path = self.getPathInPackage();
     var packageName = self.getPackageName();
     if (packageName) {

tools/isobuild/compiler-plugin.js

@@ -379,20 +379,12 @@ class ResourceSlot {
       return lazy;
     }
 
-    const sourcePath = this.inputResource.path;
-
-    if (sourcePath.endsWith(".json")) {
-      // JSON files have no side effects, so there is no reason for them
-      // ever to be evaluated eagerly.
-      return true;
-    }
-
     // If file.lazy was not previously defined, mark the file lazy if it
     // is contained by an imports directory. Note that any files contained
     // by a node_modules directory will already have been marked lazy in
     // PackageSource#_inferFileOptions.
     return this.packageSourceBatch.useMeteorInstall &&
-      files.pathDirname(sourcePath)
+      files.pathDirname(this.inputResource.path)
         .split(files.pathSep)
         .indexOf("imports") >= 0;
   }
@@ -634,19 +626,11 @@ export class PackageSourceBatch {
   // Returns a map from package names to arrays of JS output files.
   static computeJsOutputFilesMap(sourceBatches) {
     const map = new Map;
-    const pkgSourceBatches = [];
-    let appSourceBatch;
 
     sourceBatches.forEach(batch => {
       const name = batch.unibuild.pkg.name || null;
       const inputFiles = [];
 
-      if (name) {
-        pkgSourceBatches.push(batch);
-      } else {
-        appSourceBatch = batch;
-      }
-
       batch.resourceSlots.forEach(slot => {
         inputFiles.push(...slot.jsOutputResources);
       });
@@ -662,8 +646,9 @@ export class PackageSourceBatch {
     }
 
     const allMissingNodeModules = Object.create(null);
+    const scannerMap = new Map;
 
-    function scan(batch) {
+    sourceBatches.forEach(batch => {
       const name = batch.unibuild.pkg.name || null;
       const isApp = ! name;
 
@@ -692,9 +677,44 @@ export class PackageSourceBatch {
         });
       }
 
-      if (isApp) {
-        scanner.addNodeModules(allMissingNodeModules);
+      scannerMap.set(name, scanner);
+    });
 
+    const missingMap = new Map;
+
+    Object.keys(allMissingNodeModules).forEach(id => {
+      const parts = id.split("/");
+      let name = null;
+
+      if (parts[0] === "meteor") {
+        if (parts.length > 2) {
+          name = parts[1];
+          parts[1] = ".";
+          id = parts.slice(1).join("/");
+        } else {
+          return;
+        }
+      }
+
+      if (! scannerMap.has(name)) {
+        return;
+      }
+
+      if (missingMap.has(name)) {
+        missingMap.get(name).push(id);
+      } else {
+        missingMap.set(name, [id]);
+      }
+    });
+
+    missingMap.forEach((ids, name) => {
+      scannerMap.get(name).addNodeModules(ids);
+    });
+
+    scannerMap.forEach((scanner, name) => {
+      const isApp = ! name;
+
+      if (isApp) {
         const appFilesWithoutNodeModules = [];
 
         scanner.getOutputFiles().forEach(file => {
@@ -723,13 +743,7 @@ export class PackageSourceBatch {
       } else {
         map.set(name, scanner.getOutputFiles());
       }
-    }
-
-    pkgSourceBatches.forEach(scan);
-
-    if (appSourceBatch) {
-      scan(appSourceBatch);
-    }
+    });
 
     return map;
   }

tools/isobuild/import-scanner.js

@@ -1,5 +1,5 @@
 import assert from "assert";
-import {isString, has, keys, each, without} from "underscore";
+import {isString, has, keys, each, map, without} from "underscore";
 import {sha1, readAndWatchFileWithHash} from "../fs/watch.js";
 import {matches as archMatches} from "../utils/archinfo.js";
 import {findImportedModuleIdentifiers} from "./js-analyze.js";
@@ -125,8 +125,11 @@ export default class ImportScanner {
   }
 
   getOutputFiles(options) {
-    // Return all installable output files.
-    return this.outputFiles.filter(file => !! file.installPath);
+    // Return all installable output files that are either eager or
+    // imported by another module.
+    return this.outputFiles.filter(file => {
+      return file.installPath && (! file.lazy || file.imported);
+    });
   }
 
   _findImportedModuleIdentifiers(file) {
@@ -306,9 +309,8 @@ export default class ImportScanner {
     }
 
     const dirs = this._splitPath(pathDirname(installPath));
-    const bundlingClientApp =
-      ! this.name && // Indicates we are bundling an app.
-      archMatches(this.bundleArch, "web");
+    const isApp = ! this.name;
+    const bundlingForWeb = archMatches(this.bundleArch, "web");
 
     for (let dir of dirs) {
       if (dir.charAt(0) === "." ||
@@ -319,12 +321,21 @@ export default class ImportScanner {
         return;
       }
 
-      if (bundlingClientApp && (dir === "server" ||
-                                dir === "private")) {
-        // If we're bundling an app for a client architecture, any files
-        // contained by a server-only directory that is not contained by
-        // a node_modules directory must be ignored.
-        return;
+      if (isApp) {
+        if (bundlingForWeb) {
+          if (dir === "server" ||
+              dir === "private") {
+            // If we're bundling an app for a client architecture, any files
+            // contained by a server-only directory that is not contained by
+            // a node_modules directory must be ignored.
+            return;
+          }
+        } else if (dir === "client") {
+          // If we're bundling an app for a server architecture, any files
+          // contained by a client-only directory that is not contained by
+          // a node_modules directory must be ignored.
+          return;
+        }
       }
 
       if (dir === "node_modules") {
@@ -460,16 +471,13 @@ export default class ImportScanner {
 
     if (! resolved) {
       const parts = id.split("/");
-      if (parts[0] !== "meteor") { // Exclude meteor/... packages.
-        // If the imported identifier is neither absolute nor relative,
-        // but top-level, then it might be satisfied by a package
-        // installed in the top-level node_modules directory, and we
-        // should record the missing dependency so that we can include it
-        // in the app bundle.
-        const missing = file.missingNodeModules || Object.create(null);
-        missing[id] = true;
-        file.missingNodeModules = missing;
-      }
+      // If the imported identifier is neither absolute nor relative, but
+      // top-level, then it might be satisfied by a package installed in
+      // the top-level node_modules directory, and we should record the
+      // missing dependency so that we can include it in the app bundle.
+      const missing = file.missingNodeModules || Object.create(null);
+      missing[id] = true;
+      file.missingNodeModules = missing;
     }
 
     // If the dependency is still not resolved, it might be handled by the
@@ -502,23 +510,46 @@ export default class ImportScanner {
   _resolvePkgJsonMain(dirPath, seenDirPaths) {
     const pkgJsonPath = pathJoin(dirPath, "package.json");
     const pkg = this._readPkgJson(pkgJsonPath);
+    if (! pkg) {
+      return null;
+    }
 
-    if (pkg && isString(pkg.main)) {
+    let main = pkg.main;
+
+    if (archMatches(this.bundleArch, "web") &&
+        isString(pkg.browser)) {
+      main = pkg.browser;
+    }
+
+    if (isString(main)) {
       // The "main" field of package.json does not have to begin with ./
       // to be considered relative, so first we try simply appending it to
       // the directory path before falling back to a full resolve, which
       // might return a package from a node_modules directory.
-      const resolved = this._joinAndStat(dirPath, pkg.main) ||
+      const resolved = this._joinAndStat(dirPath, main) ||
         // The _tryToResolveImportedPath method takes a file object as its
         // first parameter, but only the .sourcePath property is ever
         // used, so we can get away with passing a fake directory file
         // object with only that property.
         this._tryToResolveImportedPath({
           sourcePath: dirPath,
-        }, pkg.main, seenDirPaths);
+        }, main, seenDirPaths);
 
       if (resolved) {
-        this._addPkgJsonToOutput(pkgJsonPath, pkg);
+        // Output a JS module that exports just the "name", "version", and
+        // "main" properties defined in the package.json file.
+        const pkgSubset = {
+          name: pkg.name,
+        };
+
+        if (has(pkg, "version")) {
+          pkgSubset.version = pkg.version;
+        }
+
+        pkgSubset.main = main;
+
+        this._addPkgJsonToOutput(pkgJsonPath, pkgSubset);
+
         return resolved;
       }
     }
@@ -528,13 +559,9 @@ export default class ImportScanner {
 
   _addPkgJsonToOutput(pkgJsonPath, pkg) {
     if (! has(this.absPathToOutputIndex, pkgJsonPath)) {
-      const data = new Buffer(
-        // Output a JS module that exports just the "name", "version", and
-        // "main" properties defined in the package.json file.
-        "exports.name = " + JSON.stringify(pkg.name) + ";\n" +
-        "exports.version = " + JSON.stringify(pkg.version) + ";\n" +
-        "exports.main = " + JSON.stringify(pkg.main) + ";\n"
-      );
+      const data = new Buffer(map(pkg, (value, key) => {
+        return `exports.${key} = ${JSON.stringify(value)};\n`;
+      }).join(""));
 
       const relPkgJsonPath = pathRelative(this.sourceRoot, pkgJsonPath);
 

tools/isobuild/linker.js

@@ -112,12 +112,10 @@ _.extend(Module.prototype, {
     // preserving the line numbers.
     if (self.useGlobalNamespace &&
         ! self.useMeteorInstall) {
-      return _.map(self.files, function (file) {
-        if (file.lazy) {
-          // Ignore lazy files unless we have a module system.
-          return;
-        }
+      // Ignore lazy files unless we have a module system.
+      const eagerFiles = _.filter(self.files, file => ! file.lazy);
 
+      return _.map(eagerFiles, function (file) {
         const cacheKey = JSON.stringify([
           file.sourceHash, file.bare, file.servePath]);
 

tools/isobuild/package-source.js

@@ -1383,6 +1383,7 @@ _.extend(PackageSource.prototype, {
     const anyLevelExcludes = [
       /^tests\/$/,
       /^node_modules\/$/,
+      arch === "os" ? /^client\/$/ : /^server\/$/,
       ...sourceReadOptions.exclude
     ];
 

tools/tests/apps/modules/client/eager.js

@@ -1 +0,0 @@
-export const name = module.id;

tools/tests/apps/modules/client/only.js

@@ -0,0 +1 @@
+export default module.id;

tools/tests/apps/modules/packages/modules-test-package/illegal.html

@@ -0,0 +1 @@
+<div>This is not a valid template file!</div>

tools/tests/apps/modules/packages/modules-test-package/os-stub.js

@@ -0,0 +1,2 @@
+export {platform} from "os-browserify/browser";
+export const name = module.id;

tools/tests/apps/modules/packages/modules-test-package/package.js

@@ -5,8 +5,13 @@ Package.describe({
   documentation: 'README.md'
 });
 
+Npm.depends({
+  "os-browserify": "0.2.0"
+});
+
 Package.onUse(function(api) {
   api.use('ecmascript');
+  api.use('templating');
   api.mainModule("client.js", "client");
   api.mainModule("server.js", "server");
   api.export("ModulesTestPackage");

tools/tests/apps/modules/tests.js

@@ -5,12 +5,6 @@ import {Meteor as ImportedMeteor} from "meteor/meteor";
 describe("app modules", () => {
   it("can be imported using absolute identifiers", () => {
     assert.strictEqual(require("/tests"), exports);
-    assert.strictEqual(
-      // Client modules should be importable by server modules, though not
-      // vice-versa.
-      require("/client/eager").name,
-      "/client/eager.js"
-    );
   });
 
   it("can have different file extensions", () => {
@@ -62,6 +56,8 @@ describe("app modules", () => {
     if (Meteor.isServer) {
       assert.strictEqual(typeof error, "undefined");
       assert.strictEqual(result, "/server/only.js");
+      assert.strictEqual(require("./server/only"),
+                         require("/server/only"));
     }
 
     if (Meteor.isClient) {
@@ -69,6 +65,27 @@ describe("app modules", () => {
     }
   });
 
+  it("cannot import client modules on server", () => {
+    let error;
+    let result;
+    try {
+      result = require("./client/only").default;
+    } catch (expectedOnServer) {
+      error = expectedOnServer;
+    }
+
+    if (Meteor.isClient) {
+      assert.strictEqual(typeof error, "undefined");
+      assert.strictEqual(result, "/client/only.js");
+      assert.strictEqual(require("./client/only"),
+                         require("/client/only"));
+    }
+
+    if (Meteor.isServer) {
+      assert.ok(error instanceof Error);
+    }
+  });
+
   it("should not be parsed in strictMode", () => {
     let foo = 1234;
     delete foo;
@@ -210,6 +227,20 @@ describe("Meteor packages", () => {
     const mtp = require("meteor/modules-test-package");
     assert.strictEqual(mtp.where, Meteor.isServer ? "server" : "client");
   });
+
+  it("should expose their files for import", () => {
+    const osStub = require("meteor/modules-test-package/os-stub");
+
+    assert.strictEqual(
+      osStub.platform(),
+      "browser"
+    );
+
+    assert.strictEqual(
+      osStub.name,
+      "/node_modules/meteor/modules-test-package/os-stub.js"
+    );
+  });
 });
 
 describe("async functions", () => {