New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(env): add remove command to pnpm env #5263
Changes from 5 commits
ae714f4
fc9895c
9881273
f621290
31e4d25
7056dca
a23eebf
fc94aa8
6d7ca26
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@pnpm/plugin-commands-env": minor | ||
--- | ||
|
||
Enhance `pnpm env` with the `uninstall` command. | ||
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,12 +1,13 @@ | ||||||||||||||||||||||||||||
import { promises as fs } from 'fs' | ||||||||||||||||||||||||||||
import { promises as fs, existsSync } from 'fs' | ||||||||||||||||||||||||||||
import path from 'path' | ||||||||||||||||||||||||||||
import { docsUrl } from '@pnpm/cli-utils' | ||||||||||||||||||||||||||||
import PnpmError from '@pnpm/error' | ||||||||||||||||||||||||||||
import { createFetchFromRegistry } from '@pnpm/fetch' | ||||||||||||||||||||||||||||
import { resolveNodeVersion } from '@pnpm/node.resolver' | ||||||||||||||||||||||||||||
import { globalInfo } from '@pnpm/logger' | ||||||||||||||||||||||||||||
import cmdShim from '@zkochan/cmd-shim' | ||||||||||||||||||||||||||||
import renderHelp from 'render-help' | ||||||||||||||||||||||||||||
import { getNodeDir, NvmNodeCommandOptions } from './node' | ||||||||||||||||||||||||||||
import { getNodeDir, NvmNodeCommandOptions, getNodeVersionsBaseDir } from './node' | ||||||||||||||||||||||||||||
import getNodeMirror from './getNodeMirror' | ||||||||||||||||||||||||||||
import { parseNodeEditionSpecifier } from './parseNodeEditionSpecifier' | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
|
@@ -24,14 +25,27 @@ export const commandNames = ['env'] | |||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
export function help () { | ||||||||||||||||||||||||||||
return renderHelp({ | ||||||||||||||||||||||||||||
description: 'Install and use the specified version of Node.js. The npm CLI bundled with the given Node.js version gets installed as well.', | ||||||||||||||||||||||||||||
description: 'Manage Node.js versions.', | ||||||||||||||||||||||||||||
descriptionLists: [ | ||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||
title: 'Commands', | ||||||||||||||||||||||||||||
list: [ | ||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||
description: 'Installs the specified version of Node.JS. The npm CLI bundled with the given Node.js version gets installed as well.', | ||||||||||||||||||||||||||||
name: 'use', | ||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||
description: 'Uninstalls the specified version of Node.JS.', | ||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||
name: 'remove\nuninstall', | ||||||||||||||||||||||||||||
shortAlias: 'rm,\nun', | ||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just write remove and rm
Suggested change
|
||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||
], | ||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||
title: 'Options', | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
list: [ | ||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||
description: 'Installs Node.js globally', | ||||||||||||||||||||||||||||
description: 'Manages Node.js versions globally', | ||||||||||||||||||||||||||||
name: '--global', | ||||||||||||||||||||||||||||
shortAlias: '-g', | ||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||
|
@@ -40,12 +54,17 @@ export function help () { | |||||||||||||||||||||||||||
], | ||||||||||||||||||||||||||||
url: docsUrl('env'), | ||||||||||||||||||||||||||||
usages: [ | ||||||||||||||||||||||||||||
'pnpm env use --global <version>', | ||||||||||||||||||||||||||||
'pnpm env [command] [options] <version>', | ||||||||||||||||||||||||||||
'pnpm env use --global 16', | ||||||||||||||||||||||||||||
'pnpm env use --global lts', | ||||||||||||||||||||||||||||
'pnpm env use --global argon', | ||||||||||||||||||||||||||||
'pnpm env use --global latest', | ||||||||||||||||||||||||||||
'pnpm env use --global rc/16', | ||||||||||||||||||||||||||||
'pnpm env remove --global 16', | ||||||||||||||||||||||||||||
'pnpm env remove --global lts', | ||||||||||||||||||||||||||||
'pnpm env remove --global argon', | ||||||||||||||||||||||||||||
'pnpm env remove --global latest', | ||||||||||||||||||||||||||||
'pnpm env remove --global rc/16', | ||||||||||||||||||||||||||||
], | ||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
@@ -98,6 +117,68 @@ export async function handler (opts: NvmNodeCommandOptions, params: string[]) { | |||||||||||||||||||||||||||
return `Node.js ${nodeVersion as string} is activated | ||||||||||||||||||||||||||||
${dest} -> ${src}` | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
case 'remove': | ||||||||||||||||||||||||||||
case 'rm': | ||||||||||||||||||||||||||||
case 'uninstall': | ||||||||||||||||||||||||||||
case 'un': { | ||||||||||||||||||||||||||||
if (!opts.global) { | ||||||||||||||||||||||||||||
throw new PnpmError('NOT_IMPLEMENTED_YET', '"pnpm env use <version>" can only be used with the "--global" option currently') | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
const fetch = createFetchFromRegistry(opts) | ||||||||||||||||||||||||||||
const { releaseChannel, versionSpecifier } = parseNodeEditionSpecifier(params[1]) | ||||||||||||||||||||||||||||
const nodeMirrorBaseUrl = getNodeMirror(opts.rawConfig, releaseChannel) | ||||||||||||||||||||||||||||
const nodeVersion = await resolveNodeVersion(fetch, versionSpecifier, nodeMirrorBaseUrl) | ||||||||||||||||||||||||||||
const nodeDir = getNodeVersionsBaseDir(opts.pnpmHomeDir) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
if (!nodeVersion) { | ||||||||||||||||||||||||||||
throw new PnpmError('COULD_NOT_RESOLVE_NODEJS', `Couldn't find Node.js version matching ${params[1]}`) | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
const versionDir = path.resolve(nodeDir, nodeVersion) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
if (!existsSync(versionDir)) { | ||||||||||||||||||||||||||||
throw new PnpmError('ENV_NO_NODE_DIRECTORY', `Couldn't find Node.js directory in ${versionDir}`) | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
const nodePath = path.resolve(opts.pnpmHomeDir, process.platform === 'win32' ? 'node.exe' : 'node') | ||||||||||||||||||||||||||||
const nodeLink = await fs.readlink(nodePath).catch(() => '') | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
if (nodeLink.includes(versionDir)) { | ||||||||||||||||||||||||||||
mark-omarov marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||
globalInfo(`Node.JS version ${nodeVersion} was detected as the default one, uninstalling ...`) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
const npmPath = path.resolve(opts.pnpmHomeDir, 'npm') | ||||||||||||||||||||||||||||
const npxPath = path.resolve(opts.pnpmHomeDir, 'npx') | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
await Promise.all([ | ||||||||||||||||||||||||||||
fs.unlink(nodePath), | ||||||||||||||||||||||||||||
fs.unlink(npmPath), | ||||||||||||||||||||||||||||
fs.unlink(npxPath), | ||||||||||||||||||||||||||||
]).catch(err => { | ||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just use a try/catch syntax
Suggested change
|
||||||||||||||||||||||||||||
const { code = '' } = err | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
if (code.toLowerCase() !== 'enoent') { | ||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It will be also upper case. No need to lower it
Suggested change
|
||||||||||||||||||||||||||||
throw err | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
const [processMajorVersion, processMinorVersion] = process.versions.node.split('.') | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||
* Support for earlier Node.JS versions | ||||||||||||||||||||||||||||
* `fs.rmdir` is deprecated and `fs.rm` was added in v14.14.0 | ||||||||||||||||||||||||||||
* @see https://nodejs.org/dist/latest-v16.x/docs/api/fs.html#fsrmpath-options-callback | ||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||
if (Number(processMajorVersion) === 14 && Number(processMinorVersion) <= 13) { | ||||||||||||||||||||||||||||
await fs.rmdir(versionDir, { recursive: true }) | ||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||
await fs.rm(versionDir, { recursive: true }) | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just use this rimraf lib: pnpm/packages/core/package.json Line 56 in d904b83
|
||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
return `Node.js ${nodeVersion} is uninstalled | ||||||||||||||||||||||||||||
${versionDir}` | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
default: { | ||||||||||||||||||||||||||||
throw new PnpmError('ENV_UNKNOWN_SUBCOMMAND', 'This subcommand is not known') | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ import fs from 'fs' | |
import path from 'path' | ||
import PnpmError from '@pnpm/error' | ||
import { tempDir } from '@pnpm/prepare' | ||
import { env } from '@pnpm/plugin-commands-env' | ||
import { env, node } from '@pnpm/plugin-commands-env' | ||
import * as execa from 'execa' | ||
import nock from 'nock' | ||
import PATH from 'path-name' | ||
|
@@ -133,3 +133,81 @@ test('it re-attempts failed downloads', async () => { | |
nock.cleanAll() | ||
} | ||
}) | ||
|
||
describe('env uninstall', () => { | ||
test('fail if --global is missing', async () => { | ||
tempDir() | ||
|
||
await expect( | ||
env.handler({ | ||
bin: process.cwd(), | ||
global: false, | ||
pnpmHomeDir: process.cwd(), | ||
rawConfig: {}, | ||
}, ['uninstall', 'lts']) | ||
).rejects.toEqual(new PnpmError('NOT_IMPLEMENTED_YET', '"pnpm env use <version>" can only be used with the "--global" option currently')) | ||
}) | ||
|
||
test('fail if can not resolve Node.js version', async () => { | ||
tempDir() | ||
|
||
await expect( | ||
env.handler({ | ||
bin: process.cwd(), | ||
global: true, | ||
pnpmHomeDir: process.cwd(), | ||
rawConfig: {}, | ||
}, ['uninstall', 'non-existing-version']) | ||
).rejects.toEqual(new PnpmError('COULD_NOT_RESOLVE_NODEJS', 'Couldn\'t find Node.js version matching non-existing-version')) | ||
}) | ||
|
||
test('fail if trying to uninstall version that is not installed', async () => { | ||
tempDir() | ||
|
||
const nodeDir = node.getNodeVersionsBaseDir(process.cwd()) | ||
|
||
await expect( | ||
env.handler({ | ||
bin: process.cwd(), | ||
global: true, | ||
pnpmHomeDir: process.cwd(), | ||
rawConfig: {}, | ||
}, ['uninstall', '16.4.0']) | ||
).rejects.toEqual(new PnpmError('ENV_NO_NODE_DIRECTORY', `Couldn't find Node.js directory in ${path.resolve(nodeDir, '16.4.0')}`)) | ||
}) | ||
|
||
test('install and uninstall Node.js by exact version', async () => { | ||
tempDir() | ||
|
||
const configDir = path.resolve('config') | ||
|
||
await env.handler({ | ||
bin: process.cwd(), | ||
configDir, | ||
global: true, | ||
pnpmHomeDir: process.cwd(), | ||
rawConfig: {}, | ||
}, ['use', '16.4.0']) | ||
|
||
const opts = { | ||
env: { | ||
[PATH]: `${process.cwd()}${path.delimiter}${process.env[PATH] as string}`, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand how this test works. Even if you remove the node.js from home path, won't execa pick the system installed node.js? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm sorry, I didn't have the capacity to respond yesterday. So, you're correct, it should have picked other (in my case nvm) installed versions. I was going to restrict path to only the current folder, but I see that you've done that already. I think it's worth mentioning that execa, for some reason (I didn't get the chance to investigate further yet) tries to resolve the "node" in the temp folder first, I'm not sure why that's happening exactly. Anyway, thanks for merging! |
||
}, | ||
extendEnv: false, | ||
} | ||
|
||
{ | ||
const { stdout } = execa.sync('node', ['-v'], opts) | ||
expect(stdout.toString()).toBe('v16.4.0') | ||
} | ||
|
||
await env.handler({ | ||
bin: process.cwd(), | ||
global: true, | ||
pnpmHomeDir: process.cwd(), | ||
rawConfig: {}, | ||
}, ['uninstall', '16.4.0']) | ||
|
||
expect(() => execa.sync('node', ['-v'], opts)).toThrowError() | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.