Merge pull request #10550 from meteor/support-module-syntax-in-ImportScanner

Support module syntax in ImportScanner, rather than using PackageSource#_findSources.

tools/fs/optimistic.js

@@ -5,6 +5,7 @@ import { watch } from "./safe-watcher.js";
 import { sha1 } from "./watch.js";
 import {
   pathSep,
+  pathBasename,
   pathDirname,
   pathIsAbsolute,
   pathJoin,
@@ -287,6 +288,11 @@ export const optimisticLookupPackageJson = wrap((absRootDir, relDir) => {
     return null;
   }
 
+  // Stop searching if an ancestor node_modules directory is encountered.
+  if (pathBasename(relParentDir) === "node_modules") {
+    return null;
+  }
+
   return optimisticLookupPackageJson(absRootDir, relParentDir);
 });
 

tools/isobuild/bundler.js

@@ -1078,13 +1078,20 @@ class Target {
     }
 
     const target = this;
+
+    const linkerCacheDir = this.bundlerCacheDir &&
+          files.pathJoin(this.bundlerCacheDir, "linker");
+
+    const scannerCacheDir = this.bundlerCacheDir &&
+          files.pathJoin(this.bundlerCacheDir, "scanner");
+
     const processor = new compilerPluginModule.CompilerPluginProcessor({
       unibuilds: this.unibuilds,
       arch: this.arch,
       sourceRoot: this.sourceRoot,
       isopackCache: this.isopackCache,
-      linkerCacheDir: this.bundlerCacheDir &&
-        files.pathJoin(this.bundlerCacheDir, 'linker'),
+      linkerCacheDir,
+      scannerCacheDir,
 
       // Takes a CssOutputResource and returns a string of minified CSS,
       // or null to indicate no minification occurred.

tools/isobuild/compiler-plugin.js

@@ -110,6 +110,7 @@ export class CompilerPluginProcessor {
     sourceRoot,
     isopackCache,
     linkerCacheDir,
+    scannerCacheDir,
     minifyCssResource,
   }) {
     Object.assign(this, {
@@ -118,11 +119,16 @@ export class CompilerPluginProcessor {
       sourceRoot,
       isopackCache,
       linkerCacheDir,
+      scannerCacheDir,
       minifyCssResource,
     });
 
-    if (this.linkerCacheDir) {
-      files.mkdir_p(this.linkerCacheDir);
+    if (linkerCacheDir) {
+      files.mkdir_p(linkerCacheDir);
+    }
+
+    if (scannerCacheDir) {
+      files.mkdir_p(scannerCacheDir);
     }
   }
 
@@ -141,7 +147,8 @@ export class CompilerPluginProcessor {
 
       return new PackageSourceBatch(unibuild, self, {
         sourceRoot,
-        linkerCacheDir: self.linkerCacheDir
+        linkerCacheDir: self.linkerCacheDir,
+        scannerCacheDir: self.scannerCacheDir,
       });
     });
 
@@ -1036,6 +1043,7 @@ export class PackageSourceBatch {
   constructor(unibuild, processor, {
     sourceRoot,
     linkerCacheDir,
+    scannerCacheDir,
   }) {
     const self = this;
     buildmessage.assertInJob();
@@ -1044,6 +1052,7 @@ export class PackageSourceBatch {
     self.processor = processor;
     self.sourceRoot = sourceRoot;
     self.linkerCacheDir = linkerCacheDir;
+    self.scannerCacheDir = scannerCacheDir;
     self.importExtensions = [".js", ".json"];
     self._nodeModulesPaths = null;
 
@@ -1293,6 +1302,7 @@ export class PackageSourceBatch {
         sourceRoot: batch.sourceRoot,
         nodeModulesPaths,
         watchSet: batch.unibuild.watchSet,
+        cacheDir: batch.scannerCacheDir,
       });
 
       scanner.addInputFiles(map.get(name).files);

tools/isobuild/import-scanner.js

@@ -23,6 +23,7 @@ import {
   convertToOSPath,
   convertToPosixPath,
   realpathOrNull,
+  writeFileAtomically,
 } from "../fs/files.js";
 
 const {
@@ -34,11 +35,15 @@ const {
 import {
   optimisticReadFile,
   optimisticStatOrNull,
+  optimisticLookupPackageJson,
   optimisticLStatOrNull,
   optimisticHashOrNull,
   shouldWatch,
 } from "../fs/optimistic.js";
 
+import { wrap } from "optimism";
+import { compile as reifyCompile } from "reify/lib/compiler";
+
 import Resolver from "./resolver.js";
 
 const fakeFileStat = {
@@ -55,24 +60,91 @@ const fakeFileStat = {
 // to prevent them from being added to scanner.outputFiles.
 const fakeSymbol = Symbol("fake");
 
-// Default handlers for well-known file extensions.
-// Note that these function expect strings, not Buffer objects.
-const defaultExtensionHandlers = {
-  ".js"(dataString) {
-    // Strip any #! line from the beginning of the file.
-    return dataString.replace(/^#![^\n]*/, "");
-  },
+function stripHashBang(dataString) {
+  return dataString.replace(/^#![^\n]*/, "");
+}
 
-  ".json"(dataString) {
-    const file = this;
-    file.jsonData = JSON.parse(dataString);
+const reifyCompileWithCache = wrap(function ({ dataString }) {
+  return reifyCompile(stripHashBang(dataString)).code;
+}, {
+  makeCacheKey({ hash }) {
+    return hash;
+  }
+});
+
+class DefaultHandlers {
+  constructor({
+    bundleArch,
+    sourceRoot,
+    cacheDir,
+  }) {
+    Object.assign(this, {
+      isWeb: ! archMatches(bundleArch, "os"),
+      sourceRoot,
+      cacheDir,
+    });
+  }
+
+  lookupPackageJson(file) {
+    const relDir = pathRelative(
+      this.sourceRoot,
+      pathDirname(file.absPath),
+    );
+
+    if (relDir.startsWith("..")) {
+      const absParts = file.absPath.split("/");
+      absParts.pop(); // Get rid of base filename.
+      const nmi = absParts.lastIndexOf("node_modules");
+      return optimisticLookupPackageJson(
+        absParts.slice(0, nmi + 1).join("/"),
+        absParts.slice(nmi + 1).join("/"),
+      );
+    }
+
+    return optimisticLookupPackageJson(this.sourceRoot, relDir);
+  }
+
+  js(file) {
+    const pkgJson = this.lookupPackageJson(file);
+
+    // Similar to logic in PackageSource#_findSources.
+    const hasModuleEntryPoint = pkgJson && (
+      this.isWeb
+        ? typeof pkgJson.module === "string"
+        : pkgJson.type === "module"
+    );
+
+    if (! hasModuleEntryPoint) {
+      return stripHashBang(file.dataString);
+    }
+
+    if (this.cacheDir) {
+      const cacheFileName = pathJoin(
+        this.cacheDir,
+        "reify-" + file.hash + ".js",
+      );
+      try {
+        return optimisticReadFile(cacheFileName, "utf8");
+      } catch (e) {
+        if (e.code !== "ENOENT") throw e;
+        const code = reifyCompileWithCache(file);
+        process.nextTick(writeFileAtomically, cacheFileName, code);
+        return code;
+      }
+    } else {
+      return reifyCompileWithCache(file);
+    }
+  }
+
+  json(file) {
+    file.jsonData = JSON.parse(file.dataString);
     return jsonDataToCommonJS(file.jsonData);
-  },
+  }
 
-  ".css"(dataString, hash) {
+  css({ dataString, hash }) {
     return cssToCommonJS(dataString, hash);
   }
-};
+}
 
 function jsonDataToCommonJS(data) {
   return "module.exports = " +
@@ -184,6 +256,7 @@ export default class ImportScanner {
     sourceRoot,
     nodeModulesPaths = [],
     watchSet,
+    cacheDir,
   }) {
     assert.ok(isString(sourceRoot));
 
@@ -192,11 +265,17 @@ export default class ImportScanner {
     this.sourceRoot = sourceRoot;
     this.nodeModulesPaths = nodeModulesPaths;
     this.watchSet = watchSet;
+    this.cacheDir = cacheDir;
     this.absPathToOutputIndex = Object.create(null);
     this.realPathToFiles = Object.create(null);
     this.realPathCache = Object.create(null);
     this.allMissingModules = Object.create(null);
     this.outputFiles = [];
+    this.defaultHandlers = new DefaultHandlers({
+      bundleArch,
+      sourceRoot,
+      cacheDir,
+    });
 
     this.resolver = Resolver.getOrCreate({
       caller: "ImportScanner#constructor",
@@ -274,21 +353,22 @@ export default class ImportScanner {
       // system, but any import statements or require calls in file.data
       // will be interpreted relative to this path, so it needs to be
       // something plausible. #6411 #6383
-      const absPath = pathJoin(this.sourceRoot, file.sourcePath);
+      file.absPath = pathJoin(this.sourceRoot, file.sourcePath);
 
       // This property can have values false, true, "dynamic" (which
       // indicates that the file has been imported, but only dynamically).
       file.imported = false;
 
-      file.absModuleId = file.absModuleId || this._getAbsModuleId(absPath);
+      file.absModuleId = file.absModuleId ||
+        this._getAbsModuleId(file.absPath);
 
-      if (! this._addFile(absPath, file)) {
+      if (! this._addFile(file.absPath, file)) {
         // Collisions can happen if a compiler plugin calls addJavaScript
         // multiple times with the same sourcePath. #6422
-        this._combineFiles(this._getFile(absPath), file);
+        this._combineFiles(this._getFile(file.absPath), file);
       }
 
-      this._addFileByRealPath(file, this._realPath(absPath));
+      this._addFileByRealPath(file, this._realPath(file.absPath));
     });
 
     return this;
@@ -375,27 +455,6 @@ export default class ImportScanner {
   _checkSourceAndTargetPaths(file) {
     file.sourcePath = this._getSourcePath(file);
 
-    // If we're scanning a Meteor package (as indicated by this.name), and we
-    // come across a file whose sourcePath starts with .npm/, it's a file that
-    // was installed by Npm.depends, so we should reroot it relative to one of
-    // this.nodeModulesPaths, rather than preserving the .npm/ path.
-    if (this.name && file.sourcePath.startsWith(".npm/")) {
-      const parts = file.sourcePath.split("/");
-      const nmi = parts.indexOf("node_modules");
-      if (nmi >= 0) {
-        const suffix = parts.slice(nmi + 1).join("/");
-        this.nodeModulesPaths.some(nodeModulesPath => {
-          const newAbsPath = pathJoin(nodeModulesPath, suffix);
-          if (optimisticStatOrNull(newAbsPath)) {
-            file.sourcePath = file.targetPath =
-              pathRelative(this.sourceRoot, newAbsPath);
-            return true;
-          }
-          return false;
-        });
-      }
-    }
-
     if (! isString(file.targetPath)) {
       return;
     }
@@ -957,16 +1016,18 @@ export default class ImportScanner {
       return file.dataString;
     }
 
-    const dotExt = "." + file.type;
-    const dataString = file.data.toString("utf8");
-    file.dataString = defaultExtensionHandlers[dotExt].call(
-      file,
-      dataString,
-      file.hash,
-    );
+    const rawDataString = file.data.toString("utf8");
+    if (file.type === "js") {
+      // Avoid compiling .js file with Reify when all we want is a string
+      // version of file.data.
+      file.dataString = stripHashBang(rawDataString);
+    } else {
+      file.dataString = rawDataString;
+      file.dataString = this.defaultHandlers[file.type](file);
+    }
 
     if (! (file.data instanceof Buffer) ||
-        file.dataString !== dataString) {
+        file.dataString !== rawDataString) {
       file.data = Buffer.from(file.dataString, "utf8");
     }
 
@@ -975,6 +1036,7 @@ export default class ImportScanner {
 
   _readFile(absPath) {
     const info = {
+      absPath,
       data: optimisticReadFile(absPath),
       hash: optimisticHashOrNull(absPath),
     };
@@ -1025,9 +1087,9 @@ export default class ImportScanner {
   }
 
   _readModule(absPath) {
-    let ext = pathExtname(absPath).toLowerCase();
+    const dotExt = pathExtname(absPath).toLowerCase();
 
-    if (ext === ".node") {
+    if (dotExt === ".node") {
       const dataString = "throw new Error(" + JSON.stringify(
         this.isWeb()
           ? "cannot load native .node modules on the client"
@@ -1037,7 +1099,7 @@ export default class ImportScanner {
       const data = Buffer.from(dataString, "utf8");
       const hash = sha1(data);
 
-      return { data, dataString, hash };
+      return { absPath, data, dataString, hash };
     }
 
     try {
@@ -1049,20 +1111,16 @@ export default class ImportScanner {
 
     const dataString = info.dataString;
 
-    if (! has(defaultExtensionHandlers, ext)) {
+    let ext = dotExt.slice(1);
+    if (! has(DefaultHandlers.prototype, ext)) {
       if (canBeParsedAsPlainJS(dataString)) {
-        ext = ".js";
+        ext = "js";
       } else {
         return null;
       }
     }
 
-    info.dataString = defaultExtensionHandlers[ext].call(
-      info,
-      info.dataString,
-      info.hash,
-    );
-
+    info.dataString = this.defaultHandlers[ext](info);
     if (info.dataString !== dataString) {
       info.data = Buffer.from(info.dataString, "utf8");
     }

tools/isobuild/package-source.js

@@ -1343,27 +1343,7 @@ _.extend(PackageSource.prototype, {
       return sources;
     }
 
-    const sources = find("", 0, false);
-
-    if (!isApp && typeof this.npmCacheDirectory === "string") {
-      // If this PackageSource has an npmCacheDirectory, scan it as well for
-      // sources that might need to be compiled.
-      const stat = optimisticStatOrNull(this.npmCacheDirectory);
-      if (stat && stat.isDirectory()) {
-        const relNpmDir = files.pathRelative(
-          this.sourceRoot,
-          this.npmCacheDirectory,
-        );
-        if (! relNpmDir.startsWith("..")) {
-          const relParts = relNpmDir.split("/");
-          const depth = relParts.length;
-          const inNodeModules = relParts.indexOf("node_modules") >= 0;
-          sources.push(...find(relNpmDir, depth, inNodeModules));
-        }
-      }
-    }
-
-    return sources;
+    return files.withCache(() => find("", 0, false));
   }),
 
   _findAssets({

tools/isobuild/unibuild.js

@@ -251,7 +251,6 @@ export class Unibuild {
 
     // Figure out where the npm dependencies go.
     let node_modules = {};
-    let nonLocalNodeModulesBundlePath;
     _.each(unibuild.nodeModulesDirectories, nmd => {
       const bundlePath = _.has(npmDirsToCopy, nmd.sourcePath)
       // We already have this npm directory from another unibuild.
@@ -259,9 +258,6 @@ export class Unibuild {
         : npmDirsToCopy[nmd.sourcePath] =
             nmd.getPreferredBundlePath("isopack");
       node_modules[bundlePath] = nmd.toJSON();
-      if (!nmd.local) {
-        nonLocalNodeModulesBundlePath = nonLocalNodeModulesBundlePath || bundlePath;
-      }
     });
 
     const preferredPaths = Object.keys(node_modules);
@@ -319,37 +315,13 @@ export class Unibuild {
     });
 
     // Output other resources each to their own file
-    _.each(unibuild.resources, resource => {
+    _.each(unibuild.resources, function (resource) {
       if (_.contains(["head", "body"], resource.type)) {
         // already did this one
         return;
       }
 
-      let generatedFilename;
-
-      // Although non-local npm dependencies installed by Npm.depends start
-      // with relative paths like .npm/**/node_modules/*, the files are stored
-      // in the bundle under npm/node_modules, so we need to reroot relative
-      // .npm/ paths against the npm/node_modules path.
-      if (
-        unibuild.pkg.name &&
-        typeof nonLocalNodeModulesBundlePath === "string" &&
-        typeof resource.path === "string" &&
-        resource.path.startsWith(".npm/")
-      ) {
-        const parts = resource.path.split("/");
-        const nmi = parts.indexOf("node_modules");
-        if (nmi >= 0) {
-          // Skip builder.writeToGeneratedFilename below since the file already
-          // (should) exist at this new generatedFilename path.
-          generatedFilename = files.pathJoin(
-            nonLocalNodeModulesBundlePath,
-            parts.slice(nmi + 1).join("/"),
-          );
-        }
-      }
-
-      generatedFilename = generatedFilename ||
+      const generatedFilename =
         builder.writeToGeneratedFilename(
           files.pathJoin(
             unibuildDir,