From 9c852da69599c35e3cf66c5ebed0eabe66bba2ab Mon Sep 17 00:00:00 2001
From: Ben Newman <ben@meteor.com>
Date: Wed, 19 Feb 2020 12:25:08 -0500
Subject: [PATCH] Make optimisticLookupPackageJson return array of package.json
 objects.

Commit 646fa4e3ee6baea585766807bc930123ba4d03f7 fixed #10547 by
restricting optimisticLookupPackageJson to package.json files with a
"name" property, which effectively skipped over intermediate package.json
files with additional properties.

However, in Node.js 12.16.0 (Meteor 1.9.1+), modules evaluated natively by
Node are considered ECMAScript modules if the closest package.json file
has "type": "module" (or has an .mjs file extension). This poses a problem
for the module.useNode() trick (see packages/modules-runtime/server.js),
because ESM modules cannot be imported using require.

For example, recent versions of the @babel/runtime package have a
@babel/runtime/helpers/esm/package.json file for the ESM versions of its
helpers (which specifies "type": "module"), but that package.json file
does not have a "name" property, because it is not the root package.json
file representing the entire @babel/runtime package.

I considered making the "name" restriction configurable, but that would
have fragmented the caching of optimisticLookupPackageJson. Instead, I
made it return an array of all potentially relevant package.json objects,
which can be safely cached.

This means that the caller has to iterate over the array, but there is
only one call site for this function (in tools/isobuild/package-source.js)
right now, so that wasn't too much work.
---
 tools/fs/optimistic.ts           | 30 ++++++++++++++++++++++++------
 tools/isobuild/package-source.js | 14 +++++++++++---
 2 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/tools/fs/optimistic.ts b/tools/fs/optimistic.ts
index 86eb79ee1f..47e64228c7 100644
--- a/tools/fs/optimistic.ts
+++ b/tools/fs/optimistic.ts
@@ -354,28 +354,46 @@ export const optimisticReadMeteorIgnore = wrap((dir: string) => {
 
 type LookupPkgJsonType = OptimisticWrapperFunction<
   [string, string],
-  ReturnType<typeof optimisticReadJsonOrNull>
+  ReturnType<typeof optimisticReadJsonOrNull>[]
 >;
 
-export const optimisticLookupPackageJson: LookupPkgJsonType =
+// Returns an array of package.json objects encountered in any directory
+// between relDir and absRootDir. If a package.json with a "name" property
+// is found, it will always be the first object in the array.
+export const optimisticLookupPackageJsonArray: LookupPkgJsonType =
 wrap((absRootDir: string, relDir: string) => {
   const absPkgJsonPath = pathJoin(absRootDir, relDir, "package.json");
   const pkgJson = optimisticReadJsonOrNull(absPkgJsonPath);
+
+  // Named package.json files always terminate the search. This
+  // restriction was first introduced to fix #10547, before this function
+  // was updated to return an array instead of a single object, but
+  // returning here is still an important base case.
   if (pkgJson && typeof pkgJson.name === "string") {
-    return pkgJson;
+    return [pkgJson];
   }
 
   const relParentDir = pathDirname(relDir);
   if (relParentDir === relDir) {
-    return null;
+    return [];
   }
 
   // Stop searching if an ancestor node_modules directory is encountered.
   if (pathBasename(relParentDir) === "node_modules") {
-    return null;
+    return [];
   }
 
-  return optimisticLookupPackageJson(absRootDir, relParentDir);
+  const parentArray =
+    optimisticLookupPackageJsonArray(absRootDir, relParentDir);
+
+  if (pkgJson) {
+    // If an intermediate package.json file was found, add its object to
+    // the array. Since these arrays are cached, we don't want to modify
+    // them using parentArray.push(pkgJson), hence concat.
+    return parentArray.concat(pkgJson);
+  }
+
+  return parentArray;
 });
 
 const optimisticIsSymbolicLink = wrap((path: string) => {
diff --git a/tools/isobuild/package-source.js b/tools/isobuild/package-source.js
index 15fb77f4a7..27c332c746 100644
--- a/tools/isobuild/package-source.js
+++ b/tools/isobuild/package-source.js
@@ -34,7 +34,7 @@ import {
   optimisticHashOrNull,
   optimisticStatOrNull,
   optimisticReadMeteorIgnore,
-  optimisticLookupPackageJson,
+  optimisticLookupPackageJsonArray,
 } from "../fs/optimistic";
 
 // XXX: This is a medium-term hack, to avoid having the user set a package name
@@ -1287,8 +1287,16 @@ _.extend(PackageSource.prototype, {
 
       let readOptions = sourceReadOptions;
       if (inNodeModules) {
-        const pkgJson = optimisticLookupPackageJson(self.sourceRoot, dir);
-        if (pkgJson && nodeModulesToRecompile.has(pkgJson.name)) {
+        // This is an array because (in some rare cases) an npm package
+        // may have nested package.json files with additional properties.
+        const pkgJsonArray = optimisticLookupPackageJsonArray(self.sourceRoot, dir);
+
+        // If a package.json file with a "name" property is found, it will
+        // always be the first in the array.
+        const pkgJson = pkgJsonArray[0];
+
+        if (pkgJson && pkgJson.name &&
+            nodeModulesToRecompile.has(pkgJson.name)) {
           // Avoid caching node_modules code recompiled by Meteor.
           cacheKey = false;
         } else {