Verify version when resolving Node builtin polyfills (#8387)

parcel-bundler · Sep 3, 2022 · 527e477 · 527e477
1 parent 0cc045d
commit 527e477
Show file tree

Hide file tree

Showing 9 changed files with 172 additions and 124 deletions.
diff --git a/package.json b/package.json
@@ -54,6 +54,7 @@
     "mocha-junit-reporter": "^2.0.0",
     "mocha-multi-reporters": "^1.5.1",
     "prettier": "2.4.1",
+    "punycode": "^1.4.1",
     "rimraf": "^3.0.2",
     "semver": "^5.7.1",
     "sinon": "^7.3.1"

diff --git a/packages/core/package-manager/src/NodePackageManager.js b/packages/core/package-manager/src/NodePackageManager.js
@@ -22,6 +22,7 @@ import Module from 'module';
 import path from 'path';
 import semver from 'semver';
 
+import {getModuleParts} from '@parcel/utils';
 import {getConflictingLocalDependencies} from './utils';
 import {installPackage} from './installPackage';
 import pkg from '../package.json';
@@ -138,7 +139,7 @@ export class NodePackageManager implements PackageManager {
   }
 
   async resolve(
-    name: DependencySpecifier,
+    id: DependencySpecifier,
     from: FilePath,
     options?: ?{|
       range?: ?SemverRange,
@@ -147,11 +148,12 @@ export class NodePackageManager implements PackageManager {
     |},
   ): Promise<ResolveResult> {
     let basedir = path.dirname(from);
-    let key = basedir + ':' + name;
+    let key = basedir + ':' + id;
     let resolved = cache.get(key);
     if (!resolved) {
+      let [name] = getModuleParts(id);
       try {
-        resolved = await this.resolver.resolve(name, from);
+        resolved = await this.resolver.resolve(id, from);
       } catch (e) {
         if (
           e.code !== 'MODULE_NOT_FOUND' ||
@@ -189,7 +191,7 @@ export class NodePackageManager implements PackageManager {
             saveDev: options?.saveDev ?? true,
           });
 
-          return this.resolve(name, from, {
+          return this.resolve(id, from, {
             ...options,
             shouldAutoInstall: false,
           });
@@ -230,7 +232,7 @@ export class NodePackageManager implements PackageManager {
 
           if (conflicts == null && options?.shouldAutoInstall === true) {
             await this.install([{name, range}], from);
-            return this.resolve(name, from, {
+            return this.resolve(id, from, {
               ...options,
               shouldAutoInstall: false,
             });

diff --git a/packages/core/package-manager/src/NodeResolverBase.js b/packages/core/package-manager/src/NodeResolverBase.js
@@ -13,7 +13,7 @@ import type {ResolveResult} from './types';
 import Module from 'module';
 import path from 'path';
 import invariant from 'assert';
-import {normalizeSeparators} from '@parcel/utils';
+import {getModuleParts} from '@parcel/utils';
 
 const builtins = {pnpapi: true};
 for (let builtin of Module.builtinModules) {
@@ -102,22 +102,6 @@ export class NodeResolverBase<T> {
       });
   }
 
-  getModuleParts(name: string): [FilePath, ?string] {
-    name = path.normalize(name);
-    let splitOn = name.indexOf(path.sep);
-    if (name.charAt(0) === '@') {
-      splitOn = name.indexOf(path.sep, splitOn + 1);
-    }
-    if (splitOn < 0) {
-      return [normalizeSeparators(name), undefined];
-    } else {
-      return [
-        normalizeSeparators(name.substring(0, splitOn)),
-        name.substring(splitOn + 1) || undefined,
-      ];
-    }
-  }
-
   isBuiltin(name: DependencySpecifier): boolean {
     return !!(builtins[name] || name.startsWith('node:'));
   }
@@ -135,7 +119,7 @@ export class NodeResolverBase<T> {
       };
     }
 
-    let [moduleName, subPath] = this.getModuleParts(id);
+    let [moduleName, subPath] = getModuleParts(id);
     let dir = path.dirname(sourceFile);
     let moduleDir = this.fs.findNodeModule(moduleName, dir);
 

diff --git a/packages/core/utils/src/getModuleParts.js b/packages/core/utils/src/getModuleParts.js
@@ -0,0 +1,23 @@
+// @flow strict-local
+import path from 'path';
+
+import {normalizeSeparators} from './path';
+
+/**
+ * Returns the package name and the optional subpath
+ */
+export default function getModuleParts(_name: string): [string, ?string] {
+  let name = path.normalize(_name);
+  let splitOn = name.indexOf(path.sep);
+  if (name.charAt(0) === '@') {
+    splitOn = name.indexOf(path.sep, splitOn + 1);
+  }
+  if (splitOn < 0) {
+    return [normalizeSeparators(name), undefined];
+  } else {
+    return [
+      normalizeSeparators(name.substring(0, splitOn)),
+      name.substring(splitOn + 1) || undefined,
+    ];
+  }
+}
diff --git a/packages/core/utils/src/index.js b/packages/core/utils/src/index.js
@@ -11,6 +11,7 @@ export {default as countLines} from './countLines';
 export {default as generateBuildMetrics} from './generateBuildMetrics';
 export {default as generateCertificate} from './generateCertificate';
 export {default as getCertificate} from './getCertificate';
+export {default as getModuleParts} from './getModuleParts';
 export {default as getRootDir} from './getRootDir';
 export {default as isDirectoryInside} from './isDirectoryInside';
 export {default as isURL} from './is-url';

diff --git a/packages/resolvers/default/src/DefaultResolver.js b/packages/resolvers/default/src/DefaultResolver.js
@@ -25,9 +25,8 @@ export default (new Resolver({
           ? ['ts', 'tsx', 'js', 'jsx', 'json']
           : [],
       mainFields: ['source', 'browser', 'module', 'main'],
-      packageManager: options.shouldAutoInstall
-        ? options.packageManager
-        : undefined,
+      packageManager: options.packageManager,
+      shouldAutoInstall: options.shouldAutoInstall,
       logger,
     });