fix(commonjs): validate node-resolve peer version (#1038)

lukastaegert · lukastaegert · commit 0dc11e6d8087 · 2022-04-24T07:58:20.000+02:00
diff --git a/packages/commonjs/package.json b/packages/commonjs/package.json
@@ -26,6 +26,7 @@
     "ci:test": "pnpm test -- --verbose && pnpm test:ts",
     "prebuild": "del-cli dist",
     "prepare": "if [ ! -d 'dist' ]; then pnpm build; fi",
+    "prepublishOnly": "pnpm build",
     "prerelease": "pnpm build",
     "pretest": "pnpm build",
     "release": "pnpm plugin:release --workspace-root -- --pkg $npm_package_name",
@@ -60,7 +61,7 @@
   },
   "devDependencies": {
     "@rollup/plugin-json": "^4.1.0",
-    "@rollup/plugin-node-resolve": "^13.0.6",
+    "@rollup/plugin-node-resolve": "^13.1.0",
     "locate-character": "^2.0.5",
     "require-relative": "^0.8.7",
     "rollup": "^2.67.3",
diff --git a/packages/commonjs/src/index.js b/packages/commonjs/src/index.js
@@ -23,7 +23,7 @@ import { hasCjsKeywords } from './parse';
 import { getEsImportProxy, getStaticRequireProxy, getUnknownRequireProxy } from './proxies';
 import getResolveId from './resolve-id';
 import { getResolveRequireSourcesAndGetMeta } from './resolve-require-sources';
-import validateRollupVersion from './rollup-version';
+import validateVersion from './rollup-version';
 import transformCommonjs from './transform-commonjs';
 import { getName, getStrictRequiresFilter, normalizePathSlashes } from './utils';
 
@@ -186,8 +186,12 @@ export default function commonjs(options = {}) {
       return { ...rawOptions, plugins };
     },
 
-    buildStart() {
-      validateRollupVersion(this.meta.rollupVersion, peerDependencies.rollup);
+    buildStart({ plugins }) {
+      validateVersion(this.meta.rollupVersion, peerDependencies.rollup, 'rollup');
+      const nodeResolve = plugins.find(({ name }) => name === 'node-resolve');
+      if (nodeResolve) {
+        validateVersion(nodeResolve.version, '^13.0.6', '@rollup/plugin-node-resolve');
+      }
       if (options.namedExports != null) {
         this.warn(
           'The namedExports option from "@rollup/plugin-commonjs" is deprecated. Named exports are now handled automatically.'
diff --git a/packages/commonjs/src/rollup-version.js b/packages/commonjs/src/rollup-version.js
@@ -1,20 +1,30 @@
-export default function validateRollupVersion(rollupVersion, peerDependencyVersion) {
-  const [major, minor] = rollupVersion.split('.').map(Number);
-  const versionRegexp = /\^(\d+\.\d+)\.\d+/g;
+export default function validateVersion(actualVersion, peerDependencyVersion, name) {
+  const versionRegexp = /\^(\d+\.\d+\.\d+)/g;
   let minMajor = Infinity;
   let minMinor = Infinity;
+  let minPatch = Infinity;
   let foundVersion;
   // eslint-disable-next-line no-cond-assign
   while ((foundVersion = versionRegexp.exec(peerDependencyVersion))) {
-    const [foundMajor, foundMinor] = foundVersion[1].split('.').map(Number);
+    const [foundMajor, foundMinor, foundPatch] = foundVersion[1].split('.').map(Number);
     if (foundMajor < minMajor) {
       minMajor = foundMajor;
       minMinor = foundMinor;
+      minPatch = foundPatch;
     }
   }
-  if (major < minMajor || (major === minMajor && minor < minMinor)) {
+  if (!actualVersion) {
     throw new Error(
-      `Insufficient Rollup version: "@rollup/plugin-commonjs" requires at least rollup@${minMajor}.${minMinor} but found rollup@${rollupVersion}.`
+      `Insufficient ${name} version: "@rollup/plugin-commonjs" requires at least ${name}@${minMajor}.${minMinor}.${minPatch}.`
+    );
+  }
+  const [major, minor, patch] = actualVersion.split('.').map(Number);
+  if (
+    major < minMajor ||
+    (major === minMajor && (minor < minMinor || (minor === minMinor && patch < minPatch)))
+  ) {
+    throw new Error(
+      `Insufficient ${name} version: "@rollup/plugin-commonjs" requires at least ${name}@${minMajor}.${minMinor}.${minPatch} but found ${name}@${actualVersion}.`
     );
   }
 }
diff --git a/packages/commonjs/test/test.js b/packages/commonjs/test/test.js
@@ -2,7 +2,7 @@
 import * as path from 'path';
 import os from 'os';
 
-import resolve from '@rollup/plugin-node-resolve';
+import nodeResolve from '@rollup/plugin-node-resolve';
 
 import test from 'ava';
 import { getLocator } from 'locate-character';
@@ -91,7 +91,7 @@ test('supports an object of multiple entry points', async (t) => {
       b: require.resolve('./fixtures/samples/multiple-entry-points/b.js'),
       c: require.resolve('./fixtures/samples/multiple-entry-points/c.js')
     },
-    plugins: [resolve(), commonjs()]
+    plugins: [nodeResolve(), commonjs()]
   });
 
   const { output } = await bundle.generate({
@@ -212,7 +212,7 @@ test.serial('handles symlinked node_modules with preserveSymlinks: false', (t) =
         throw new Error(`Unexpected warning: ${warning.message}`);
       },
       plugins: [
-        resolve({
+        nodeResolve({
           preserveSymlinks: false,
           preferBuiltins: false
         }),
@@ -400,7 +400,7 @@ test('rewrites top-level defines', async (t) => {
 test('respects options.external', async (t) => {
   const bundle = await rollup({
     input: 'fixtures/samples/external/main.js',
-    plugins: [resolve(), commonjs()],
+    plugins: [nodeResolve(), commonjs()],
     external: ['baz']
   });
 
@@ -731,3 +731,35 @@ test('does not transform typeof exports for mixed modules', async (t) => {
   t.is(code.includes('typeof exports'), true, '"typeof exports" not found in the code');
   t.snapshot(code);
 });
+
+test('throws when using an old node_resolve version', async (t) => {
+  let error = null;
+  try {
+    await rollup({
+      input: 'ignored',
+      plugins: [commonjs(), { name: nodeResolve().name }]
+    });
+  } catch (err) {
+    error = err;
+  }
+  t.like(error, {
+    message:
+      'Insufficient @rollup/plugin-node-resolve version: "@rollup/plugin-commonjs" requires at least @rollup/plugin-node-resolve@13.0.6.'
+  });
+});
+
+test('throws when using an inadequate node_resolve version', async (t) => {
+  let error = null;
+  try {
+    await rollup({
+      input: 'ignored',
+      plugins: [commonjs(), { name: nodeResolve().name, version: '13.0.5' }]
+    });
+  } catch (err) {
+    error = err;
+  }
+  t.like(error, {
+    message:
+      'Insufficient @rollup/plugin-node-resolve version: "@rollup/plugin-commonjs" requires at least @rollup/plugin-node-resolve@13.0.6 but found @rollup/plugin-node-resolve@13.0.5.'
+  });
+});
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
