From 66423df83782e06f53352aef68dc6368335a29fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kh=E1=BA=A3i?= Date: Thu, 10 Aug 2023 05:38:06 +0700 Subject: [PATCH] feat: clear error on invalid node version (#6916) related to #6909 --- .changeset/nervous-buckets-return.md | 6 +++ env/plugin-commands-env/src/envList.ts | 4 +- env/plugin-commands-env/src/envRemove.ts | 4 +- env/plugin-commands-env/src/envUse.ts | 4 +- env/plugin-commands-env/src/node.ts | 6 +-- ...itionSpecifier.ts => parseEnvSpecifier.ts} | 4 +- .../src/parseNodeSpecifier.ts | 44 ++++++++++++++++++ env/plugin-commands-env/test/node.test.ts | 2 +- ...itionSpecifier.ts => parseEnvSpecifier.ts} | 4 +- .../test/parseNodeSpecifier.ts | 45 +++++++++++++++++++ 10 files changed, 109 insertions(+), 14 deletions(-) create mode 100644 .changeset/nervous-buckets-return.md rename env/plugin-commands-env/src/{parseNodeEditionSpecifier.ts => parseEnvSpecifier.ts} (83%) create mode 100644 env/plugin-commands-env/src/parseNodeSpecifier.ts rename env/plugin-commands-env/test/{parseNodeEditionSpecifier.ts => parseEnvSpecifier.ts} (76%) create mode 100644 env/plugin-commands-env/test/parseNodeSpecifier.ts diff --git a/.changeset/nervous-buckets-return.md b/.changeset/nervous-buckets-return.md new file mode 100644 index 0000000000..00bc92d75d --- /dev/null +++ b/.changeset/nervous-buckets-return.md @@ -0,0 +1,6 @@ +--- +"@pnpm/node.fetcher": patch +"pnpm": patch +--- + +Emit a clear error message when users attempt to specify an undownloadable node version [#6916](https://github.com/pnpm/pnpm/pull/6916). diff --git a/env/plugin-commands-env/src/envList.ts b/env/plugin-commands-env/src/envList.ts index ee29f0b975..9ae8ee13b8 100644 --- a/env/plugin-commands-env/src/envList.ts +++ b/env/plugin-commands-env/src/envList.ts @@ -6,7 +6,7 @@ import { PnpmError } from '@pnpm/error' import semver from 'semver' import { getNodeMirror } from './getNodeMirror' import { getNodeVersionsBaseDir, type NvmNodeCommandOptions } from './node' -import { parseNodeEditionSpecifier } from './parseNodeEditionSpecifier' +import { parseEnvSpecifier } from './parseEnvSpecifier' import { getNodeExecPathAndTargetDir, getNodeExecPathInNodeDir } from './utils' export async function envList (opts: NvmNodeCommandOptions, params: string[]) { @@ -43,7 +43,7 @@ async function listLocalVersions (opts: NvmNodeCommandOptions) { async function listRemoteVersions (opts: NvmNodeCommandOptions, versionSpec?: string) { const fetch = createFetchFromRegistry(opts) - const { releaseChannel, versionSpecifier } = parseNodeEditionSpecifier(versionSpec ?? '') + const { releaseChannel, versionSpecifier } = parseEnvSpecifier(versionSpec ?? '') const nodeMirrorBaseUrl = getNodeMirror(opts.rawConfig, releaseChannel) const nodeVersionList = await resolveNodeVersions(fetch, versionSpecifier, nodeMirrorBaseUrl) return nodeVersionList diff --git a/env/plugin-commands-env/src/envRemove.ts b/env/plugin-commands-env/src/envRemove.ts index ecccdcd3ca..2d17dade89 100644 --- a/env/plugin-commands-env/src/envRemove.ts +++ b/env/plugin-commands-env/src/envRemove.ts @@ -6,7 +6,7 @@ import { globalInfo } from '@pnpm/logger' import { resolveNodeVersion } from '@pnpm/node.resolver' import { removeBin } from '@pnpm/remove-bins' import rimraf from '@zkochan/rimraf' -import { parseNodeEditionSpecifier } from './parseNodeEditionSpecifier' +import { parseEnvSpecifier } from './parseEnvSpecifier' import { getNodeExecPathAndTargetDir } from './utils' import { getNodeMirror } from './getNodeMirror' import { getNodeVersionsBaseDir, type NvmNodeCommandOptions } from './node' @@ -17,7 +17,7 @@ export async function envRemove (opts: NvmNodeCommandOptions, params: string[]) } const fetch = createFetchFromRegistry(opts) - const { releaseChannel, versionSpecifier } = parseNodeEditionSpecifier(params[0]) + const { releaseChannel, versionSpecifier } = parseEnvSpecifier(params[0]) const nodeMirrorBaseUrl = getNodeMirror(opts.rawConfig, releaseChannel) const nodeVersion = await resolveNodeVersion(fetch, versionSpecifier, nodeMirrorBaseUrl) const nodeDir = getNodeVersionsBaseDir(opts.pnpmHomeDir) diff --git a/env/plugin-commands-env/src/envUse.ts b/env/plugin-commands-env/src/envUse.ts index 8f4182c055..16ed2cf469 100644 --- a/env/plugin-commands-env/src/envUse.ts +++ b/env/plugin-commands-env/src/envUse.ts @@ -9,7 +9,7 @@ import isWindows from 'is-windows' import symlinkDir from 'symlink-dir' import { getNodeDir, type NvmNodeCommandOptions } from './node' import { getNodeMirror } from './getNodeMirror' -import { parseNodeEditionSpecifier } from './parseNodeEditionSpecifier' +import { parseEnvSpecifier } from './parseEnvSpecifier' import { CURRENT_NODE_DIRNAME, getNodeExecPathInBinDir, getNodeExecPathInNodeDir } from './utils' export async function envUse (opts: NvmNodeCommandOptions, params: string[]) { @@ -17,7 +17,7 @@ export async function envUse (opts: NvmNodeCommandOptions, params: string[]) { throw new PnpmError('NOT_IMPLEMENTED_YET', '"pnpm env use " can only be used with the "--global" option currently') } const fetch = createFetchFromRegistry(opts) - const { releaseChannel, versionSpecifier } = parseNodeEditionSpecifier(params[0]) + const { releaseChannel, versionSpecifier } = parseEnvSpecifier(params[0]) const nodeMirrorBaseUrl = getNodeMirror(opts.rawConfig, releaseChannel) const nodeVersion = await resolveNodeVersion(fetch, versionSpecifier, nodeMirrorBaseUrl) if (!nodeVersion) { diff --git a/env/plugin-commands-env/src/node.ts b/env/plugin-commands-env/src/node.ts index 97e6927589..dc1d7291ee 100644 --- a/env/plugin-commands-env/src/node.ts +++ b/env/plugin-commands-env/src/node.ts @@ -8,7 +8,7 @@ import { getStorePath } from '@pnpm/store-path' import loadJsonFile from 'load-json-file' import writeJsonFile from 'write-json-file' import { getNodeMirror } from './getNodeMirror' -import { parseNodeEditionSpecifier } from './parseNodeEditionSpecifier' +import { parseNodeSpecifier } from './parseNodeSpecifier' export type NvmNodeCommandOptions = Pick /^[0-9]+\.[0-9]+\.[0-9]+$/.test(version) +const STABLE_RELEASE_ERROR_HINT = 'The correct syntax for stable release is strictly X.Y.Z or release/X.Y.Z' + +export function parseNodeSpecifier (specifier: string): NodeSpecifier { + if (specifier.includes('/')) { + const [releaseChannel, useNodeVersion] = specifier.split('/') + + if (releaseChannel === 'release') { + if (!isStableVersion(useNodeVersion)) { + throw new PnpmError('INVALID_NODE_VERSION', `"${specifier}" is not a valid node version`, { + hint: STABLE_RELEASE_ERROR_HINT, + }) + } + } else if (!useNodeVersion.includes(releaseChannel)) { + throw new PnpmError('MISMATCHED_RELEASE_CHANNEL', `The node version (${useNodeVersion}) must contain the release channel (${releaseChannel})`) + } + + return { releaseChannel, useNodeVersion } + } + + const prereleaseMatch = specifier.match(/^[0-9]+\.[0-9]+\.[0-9]+-(nightly|rc|test|v8-canary)(\..+)$/) + if (prereleaseMatch != null) { + return { releaseChannel: prereleaseMatch[1], useNodeVersion: specifier } + } + + if (isStableVersion(specifier)) { + return { releaseChannel: 'release', useNodeVersion: specifier } + } + + let hint: string | undefined + if (['nightly', 'rc', 'test', 'v8-canary'].includes(specifier)) { + hint = `The correct syntax for ${specifier} release is strictly X.Y.Z-${specifier}.W` + } else if (/^[0-9]+\.[0-9]+$/.test(specifier) || /^[0-9]+$/.test(specifier) || ['release', 'stable', 'latest'].includes(specifier)) { + hint = STABLE_RELEASE_ERROR_HINT + } + throw new PnpmError('INVALID_NODE_VERSION', `"${specifier}" is not a valid node version`, { hint }) +} diff --git a/env/plugin-commands-env/test/node.test.ts b/env/plugin-commands-env/test/node.test.ts index 0113cc19b5..7273ce4a01 100644 --- a/env/plugin-commands-env/test/node.test.ts +++ b/env/plugin-commands-env/test/node.test.ts @@ -54,7 +54,7 @@ test('install Node uses node-mirror:release option', async () => { } }) -test('install and rc version of Node.js', async () => { +test('install an rc version of Node.js', async () => { tempDir() const configDir = path.resolve('config') diff --git a/env/plugin-commands-env/test/parseNodeEditionSpecifier.ts b/env/plugin-commands-env/test/parseEnvSpecifier.ts similarity index 76% rename from env/plugin-commands-env/test/parseNodeEditionSpecifier.ts rename to env/plugin-commands-env/test/parseEnvSpecifier.ts index daa948f71f..f57d47a41c 100644 --- a/env/plugin-commands-env/test/parseNodeEditionSpecifier.ts +++ b/env/plugin-commands-env/test/parseEnvSpecifier.ts @@ -1,4 +1,4 @@ -import { parseNodeEditionSpecifier } from '../lib/parseNodeEditionSpecifier' +import { parseEnvSpecifier } from '../lib/parseEnvSpecifier' test.each([ ['6', '6', 'release'], @@ -9,7 +9,7 @@ test.each([ ['argon', 'argon', 'release'], ['latest', 'latest', 'release'], ])('Node.js version selector is parsed', (editionSpecifier, versionSpecifier, releaseChannel) => { - const node = parseNodeEditionSpecifier(editionSpecifier) + const node = parseEnvSpecifier(editionSpecifier) expect(node.versionSpecifier).toMatch(versionSpecifier) expect(node.releaseChannel).toBe(releaseChannel) }) diff --git a/env/plugin-commands-env/test/parseNodeSpecifier.ts b/env/plugin-commands-env/test/parseNodeSpecifier.ts new file mode 100644 index 0000000000..006c2b412b --- /dev/null +++ b/env/plugin-commands-env/test/parseNodeSpecifier.ts @@ -0,0 +1,45 @@ +import { parseNodeSpecifier } from '../lib/parseNodeSpecifier' + +test.each([ + ['rc/16.0.0-rc.0', '16.0.0-rc.0', 'rc'], + ['16.0.0-rc.0', '16.0.0-rc.0', 'rc'], + ['release/16.0.0', '16.0.0', 'release'], + ['16.0.0', '16.0.0', 'release'], +])('Node.js version selector is parsed', (editionSpecifier, useNodeVersion, releaseChannel) => { + const node = parseNodeSpecifier(editionSpecifier) + expect(node.useNodeVersion).toBe(useNodeVersion) + expect(node.releaseChannel).toBe(releaseChannel) +}) + +test.each([ + ['rc/10', '10', 'rc'], + ['rc/10.0', '10.0', 'rc'], + ['rc/10.0.0', '10.0.0', 'rc'], + ['rc/10.0.0.test.0', '10.0.0.test.0', 'rc'], +])('invalid Node.js specifier', (editionSpecifier, useNodeVersion, releaseChannel) => { + expect(() => parseNodeSpecifier(editionSpecifier)).toThrow(`The node version (${useNodeVersion}) must contain the release channel (${releaseChannel})`) +}) + +test.each([ + ['nightly'], + ['rc'], + ['test'], + ['v8-canary'], +])('invalid Node.js specifier', async (specifier) => { + const promise = Promise.resolve().then(() => parseNodeSpecifier(specifier)) + await expect(promise).rejects.toThrow(`"${specifier}" is not a valid node version`) + await expect(promise).rejects.toHaveProperty('hint', `The correct syntax for ${specifier} release is strictly X.Y.Z-${specifier}.W`) +}) + +test.each([ + ['release'], + ['stable'], + ['latest'], + ['release/16.0.0.release.0'], + ['16'], + ['16.0'], +])('invalid Node.js specifier', async (specifier) => { + const promise = Promise.resolve().then(() => parseNodeSpecifier(specifier)) + await expect(promise).rejects.toThrow(`"${specifier}" is not a valid node version`) + await expect(promise).rejects.toHaveProperty('hint', 'The correct syntax for stable release is strictly X.Y.Z or release/X.Y.Z') +})