From a5d1e6c4011e343496f129c94968ce23de77aa6e Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Thu, 11 Apr 2024 09:59:27 -0600 Subject: [PATCH] fix: use yarn to install deps when linking yarn plugins --- package.json | 3 +- src/fork.ts | 92 ++++++++++++++++++++++++++++++++ src/npm.ts | 99 +++-------------------------------- src/plugins.ts | 35 ++++++++++--- src/yarn.ts | 139 +++++++++++++++++++++++++++++++++++++++++++++++++ yarn.lock | 36 +++---------- 6 files changed, 276 insertions(+), 128 deletions(-) create mode 100644 src/fork.ts create mode 100644 src/yarn.ts diff --git a/package.json b/package.json index 3fd396ea..9dc468a0 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,8 @@ "npm-package-arg": "^11.0.1", "npm-run-path": "^5.3.0", "semver": "^7.6.0", - "validate-npm-package-name": "^5.0.0" + "validate-npm-package-name": "^5.0.0", + "yarn": "^1.22.22" }, "devDependencies": { "@commitlint/config-conventional": "^18", diff --git a/src/fork.ts b/src/fork.ts new file mode 100644 index 00000000..20e797d2 --- /dev/null +++ b/src/fork.ts @@ -0,0 +1,92 @@ +import {Errors, ux} from '@oclif/core' +import makeDebug from 'debug' +import {fork as cpFork} from 'node:child_process' +import {npmRunPathEnv} from 'npm-run-path' + +import {LogLevel} from './log-level.js' + +export type ExecOptions = { + cwd: string + logLevel: LogLevel +} + +export type Output = { + stderr: string[] + stdout: string[] +} + +const debug = makeDebug('@oclif/plugin-plugins:fork') + +export async function fork(modulePath: string, args: string[] = [], {cwd, logLevel}: ExecOptions): Promise { + return new Promise((resolve, reject) => { + const forked = cpFork(modulePath, args, { + cwd, + env: { + ...npmRunPathEnv(), + // Disable husky hooks because a plugin might be trying to install them, which will + // break the install since the install location isn't a .git directory. + HUSKY: '0', + }, + execArgv: process.execArgv + .join(' ') + // Remove --loader ts-node/esm from execArgv so that the subprocess doesn't fail if it can't find ts-node. + // The ts-node/esm loader isn't need to execute npm or yarn commands anyways. + .replace('--loader ts-node/esm', '') + .replace('--loader=ts-node/esm', '') + .split(' ') + .filter(Boolean), + stdio: [0, null, null, 'ipc'], + }) + + const possibleLastLinesOfNpmInstall = ['up to date', 'added'] + const stderr: string[] = [] + const stdout: string[] = [] + const loggedStderr: string[] = [] + const loggedStdout: string[] = [] + + const shouldPrint = (str: string): boolean => { + // For ux cleanliness purposes, don't print the final line of npm install output if + // the log level is 'notice' and there's no other output. + const noOtherOutput = loggedStderr.length === 0 && loggedStdout.length === 0 + const isLastLine = possibleLastLinesOfNpmInstall.some((line) => str.startsWith(line)) + if (noOtherOutput && isLastLine && logLevel === 'notice') { + return false + } + + return logLevel !== 'silent' + } + + forked.stderr?.setEncoding('utf8') + forked.stderr?.on('data', (d: Buffer) => { + const output = d.toString().trim() + stderr.push(output) + if (shouldPrint(output)) { + loggedStderr.push(output) + ux.log(output) + } else debug(output) + }) + + forked.stdout?.setEncoding('utf8') + forked.stdout?.on('data', (d: Buffer) => { + const output = d.toString().trim() + stdout.push(output) + if (shouldPrint(output)) { + loggedStdout.push(output) + ux.log(output) + } else debug(output) + }) + + forked.on('error', reject) + forked.on('exit', (code: number) => { + if (code === 0) { + resolve({stderr, stdout}) + } else { + reject( + new Errors.CLIError(`${modulePath} ${args.join(' ')} exited with code ${code}`, { + suggestions: ['Run with DEBUG=@oclif/plugin-plugins* to see debug output.'], + }), + ) + } + }) + }) +} diff --git a/src/npm.ts b/src/npm.ts index 53e57729..85e59074 100644 --- a/src/npm.ts +++ b/src/npm.ts @@ -1,103 +1,18 @@ -import {Errors, Interfaces, ux} from '@oclif/core' +import {Interfaces, ux} from '@oclif/core' import makeDebug from 'debug' -import {fork as cpFork} from 'node:child_process' import {readFile} from 'node:fs/promises' import {createRequire} from 'node:module' import {join, sep} from 'node:path' -import {npmRunPathEnv} from 'npm-run-path' +import {ExecOptions, Output, fork} from './fork.js' import {LogLevel} from './log-level.js' const debug = makeDebug('@oclif/plugin-plugins:npm') -type ExecOptions = { - cwd: string - logLevel: LogLevel -} - type InstallOptions = ExecOptions & { prod?: boolean } -export type NpmOutput = { - stderr: string[] - stdout: string[] -} - -async function fork(modulePath: string, args: string[] = [], {cwd, logLevel}: ExecOptions): Promise { - return new Promise((resolve, reject) => { - const forked = cpFork(modulePath, args, { - cwd, - env: { - ...npmRunPathEnv(), - // Disable husky hooks because a plugin might be trying to install them, which will - // break the install since the install location isn't a .git directory. - HUSKY: '0', - }, - execArgv: process.execArgv - .join(' ') - // Remove --loader ts-node/esm from execArgv so that the subprocess doesn't fail if it can't find ts-node. - // The ts-node/esm loader isn't need to execute npm commands anyways. - .replace('--loader ts-node/esm', '') - .replace('--loader=ts-node/esm', '') - .split(' ') - .filter(Boolean), - stdio: [0, null, null, 'ipc'], - }) - - const possibleLastLinesOfNpmInstall = ['up to date', 'added'] - const stderr: string[] = [] - const stdout: string[] = [] - const loggedStderr: string[] = [] - const loggedStdout: string[] = [] - - const shouldPrint = (str: string): boolean => { - // For ux cleanliness purposes, don't print the final line of npm install output if - // the log level is 'notice' and there's no other output. - const noOtherOutput = loggedStderr.length === 0 && loggedStdout.length === 0 - const isLastLine = possibleLastLinesOfNpmInstall.some((line) => str.startsWith(line)) - if (noOtherOutput && isLastLine && logLevel === 'notice') { - return false - } - - return logLevel !== 'silent' - } - - forked.stderr?.setEncoding('utf8') - forked.stderr?.on('data', (d: Buffer) => { - const output = d.toString().trim() - stderr.push(output) - if (shouldPrint(output)) { - loggedStderr.push(output) - ux.log(output) - } else debug(output) - }) - - forked.stdout?.setEncoding('utf8') - forked.stdout?.on('data', (d: Buffer) => { - const output = d.toString().trim() - stdout.push(output) - if (shouldPrint(output)) { - loggedStdout.push(output) - ux.log(output) - } else debug(output) - }) - - forked.on('error', reject) - forked.on('exit', (code: number) => { - if (code === 0) { - resolve({stderr, stdout}) - } else { - reject( - new Errors.CLIError(`${modulePath} ${args.join(' ')} exited with code ${code}`, { - suggestions: ['Run with DEBUG=@oclif/plugin-plugins* to see debug output.'], - }), - ) - } - }) - }) -} - export class NPM { private bin: string | undefined private config: Interfaces.Config @@ -108,7 +23,7 @@ export class NPM { this.logLevel = logLevel } - async exec(args: string[] = [], options: ExecOptions): Promise { + async exec(args: string[] = [], options: ExecOptions): Promise { const bin = await this.findNpm() debug('npm binary path', bin) @@ -130,20 +45,20 @@ export class NPM { } } - async install(args: string[], opts: InstallOptions): Promise { + async install(args: string[], opts: InstallOptions): Promise { const prod = opts.prod ? ['--omit', 'dev'] : [] return this.exec(['install', ...args, ...prod, '--no-audit'], opts) } - async uninstall(args: string[], opts: ExecOptions): Promise { + async uninstall(args: string[], opts: ExecOptions): Promise { return this.exec(['uninstall', ...args], opts) } - async update(args: string[], opts: ExecOptions): Promise { + async update(args: string[], opts: ExecOptions): Promise { return this.exec(['update', ...args], opts) } - async view(args: string[], opts: ExecOptions): Promise { + async view(args: string[], opts: ExecOptions): Promise { return this.exec(['view', ...args], {...opts, logLevel: 'silent'}) } diff --git a/src/plugins.ts b/src/plugins.ts index 03c14595..ef434d72 100644 --- a/src/plugins.ts +++ b/src/plugins.ts @@ -7,9 +7,11 @@ import {basename, dirname, join, resolve} from 'node:path' import {fileURLToPath} from 'node:url' import {gt, valid, validRange} from 'semver' +import {Output} from './fork.js' import {LogLevel} from './log-level.js' -import {NPM, NpmOutput} from './npm.js' +import {NPM} from './npm.js' import {uniqWith} from './util.js' +import {Yarn} from './yarn.js' type UserPJSON = { dependencies: Record @@ -58,7 +60,7 @@ function extractIssuesLocation( } } -function notifyUser(plugin: Config, output: NpmOutput): void { +function notifyUser(plugin: Config, output: Output): void { const containsWarnings = [...output.stdout, ...output.stderr].some((l) => l.includes('npm WARN')) if (containsWarnings) { ux.logToStderr(chalk.bold.yellow(`\nThese warnings can only be addressed by the owner(s) of ${plugin.name}.`)) @@ -231,11 +233,30 @@ export default class Plugins { this.isValidPlugin(c) if (install) { - await this.npm.install([], { - cwd: c.root, - logLevel: this.logLevel, - prod: false, - }) + if (await fileExists(join(c.root, 'yarn.lock'))) { + this.debug('installing dependencies with yarn') + const yarn = new Yarn({config: this.config, logLevel: this.logLevel}) + await yarn.install([], { + cwd: c.root, + logLevel: this.logLevel, + prod: false, + }) + } else if (await fileExists(join(c.root, 'package-lock.json'))) { + this.debug('installing dependencies with npm') + await this.npm.install([], { + cwd: c.root, + logLevel: this.logLevel, + prod: false, + }) + } else if (await fileExists(join(c.root, 'pnpm-lock.yaml'))) { + ux.warn( + `pnpm is not supported for installing after a link. The link succeeded, but you may need to run 'pnpm install' in ${c.root}.`, + ) + } else { + ux.warn( + `No lockfile found in ${c.root}. The link succeeded, but you may need to install the dependencies in your project.`, + ) + } } await this.add({name: c.name, root: c.root, type: 'link'}) diff --git a/src/yarn.ts b/src/yarn.ts new file mode 100644 index 00000000..490bfc9a --- /dev/null +++ b/src/yarn.ts @@ -0,0 +1,139 @@ +import {Errors, Interfaces, ux} from '@oclif/core' +import makeDebug from 'debug' +import {fork as cpFork} from 'node:child_process' +import {createRequire} from 'node:module' +import {fileURLToPath} from 'node:url' +import {npmRunPathEnv} from 'npm-run-path' + +import {LogLevel} from './log-level.js' + +const require = createRequire(import.meta.url) +const debug = makeDebug('@oclif/plugin-plugins:yarn') + +type ExecOptions = { + cwd: string + logLevel: LogLevel +} + +type InstallOptions = ExecOptions & { + prod?: boolean +} + +export type NpmOutput = { + stderr: string[] + stdout: string[] +} + +async function fork(modulePath: string, args: string[] = [], {cwd, logLevel}: ExecOptions): Promise { + return new Promise((resolve, reject) => { + const forked = cpFork(modulePath, args, { + cwd, + env: { + ...npmRunPathEnv(), + // Disable husky hooks because a plugin might be trying to install them, which will + // break the install since the install location isn't a .git directory. + HUSKY: '0', + }, + execArgv: process.execArgv + .join(' ') + // Remove --loader ts-node/esm from execArgv so that the subprocess doesn't fail if it can't find ts-node. + // The ts-node/esm loader isn't need to execute npm commands anyways. + .replace('--loader ts-node/esm', '') + .replace('--loader=ts-node/esm', '') + .split(' ') + .filter(Boolean), + stdio: [0, null, null, 'ipc'], + }) + + const possibleLastLinesOfNpmInstall = ['up to date', 'added'] + const stderr: string[] = [] + const stdout: string[] = [] + const loggedStderr: string[] = [] + const loggedStdout: string[] = [] + + const shouldPrint = (str: string): boolean => { + // For ux cleanliness purposes, don't print the final line of npm install output if + // the log level is 'notice' and there's no other output. + const noOtherOutput = loggedStderr.length === 0 && loggedStdout.length === 0 + const isLastLine = possibleLastLinesOfNpmInstall.some((line) => str.startsWith(line)) + if (noOtherOutput && isLastLine && logLevel === 'notice') { + return false + } + + return logLevel !== 'silent' + } + + forked.stderr?.setEncoding('utf8') + forked.stderr?.on('data', (d: Buffer) => { + const output = d.toString().trim() + stderr.push(output) + if (shouldPrint(output)) { + loggedStderr.push(output) + ux.log(output) + } else debug(output) + }) + + forked.stdout?.setEncoding('utf8') + forked.stdout?.on('data', (d: Buffer) => { + const output = d.toString().trim() + stdout.push(output) + if (shouldPrint(output)) { + loggedStdout.push(output) + ux.log(output) + } else debug(output) + }) + + forked.on('error', reject) + forked.on('exit', (code: number) => { + if (code === 0) { + resolve({stderr, stdout}) + } else { + reject( + new Errors.CLIError(`${modulePath} ${args.join(' ')} exited with code ${code}`, { + suggestions: ['Run with DEBUG=@oclif/plugin-plugins* to see debug output.'], + }), + ) + } + }) + }) +} + +export class Yarn { + private config: Interfaces.Config + private logLevel: LogLevel + + public constructor({config, logLevel}: {config: Interfaces.Config; logLevel: LogLevel}) { + this.config = config + this.logLevel = logLevel + } + + async exec(args: string[] = [], options: ExecOptions): Promise { + const bin = await this.findYarn() + debug('yarn binary path', bin) + + if (options.logLevel === 'verbose') args.push('--verbose') + if (this.config.npmRegistry) args.push(`--registry=${this.config.npmRegistry}`) + + if (options.logLevel !== 'notice' && options.logLevel !== 'silent') { + ux.logToStderr(`${options.cwd}: ${bin} ${args.join(' ')}`) + } + + debug(`${options.cwd}: ${bin} ${args.join(' ')}`) + try { + const output = await fork(bin, args, options) + debug('yarn done') + return output + } catch (error: unknown) { + debug('yarn error', error) + throw error + } + } + + async install(args: string[], opts: InstallOptions): Promise { + return this.exec(['install', ...args], opts) + } + + private async findYarn(): Promise { + return require.resolve('yarn/bin/yarn.js', {paths: [this.config.root, fileURLToPath(import.meta.url)]}) + } +} diff --git a/yarn.lock b/yarn.lock index 8a43909e..d5112576 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6381,16 +6381,7 @@ string-argv@0.3.2: resolved "https://registry.npmjs.org/string-argv/-/string-argv-0.3.2.tgz" integrity sha512-aqD2Q0144Z+/RqG52NeHEkZauTAUWJO8c6yTftGJKO3Tja5tUgIfmIl6kExvhtxSDP7fXB6DvzkfMpCd/F3G+Q== -"string-width-cjs@npm:string-width@^4.2.0": - version "4.2.3" - resolved "https://registry.npmjs.org/string-width/-/string-width-4.2.3.tgz" - integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== - dependencies: - emoji-regex "^8.0.0" - is-fullwidth-code-point "^3.0.0" - strip-ansi "^6.0.1" - -"string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: +"string-width-cjs@npm:string-width@^4.2.0", "string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: version "4.2.3" resolved "https://registry.npmjs.org/string-width/-/string-width-4.2.3.tgz" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -6451,14 +6442,7 @@ string_decoder@^1.1.1: dependencies: safe-buffer "~5.2.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1": - version "6.0.1" - resolved "https://registry.npmjs.org/strip-ansi/-/strip-ansi-6.0.1.tgz" - integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== - dependencies: - ansi-regex "^5.0.1" - -strip-ansi@^6.0.0, strip-ansi@^6.0.1: +"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.npmjs.org/strip-ansi/-/strip-ansi-6.0.1.tgz" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -6912,7 +6896,7 @@ workerpool@6.2.1: resolved "https://registry.npmjs.org/workerpool/-/workerpool-6.2.1.tgz" integrity sha512-ILEIE97kDZvF9Wb9f6h5aXK4swSlKGUcOEGiIYb2OOu/IrDU9iwj0fD//SsA6E5ibwJxpEvhullJY4Sl4GcpAw== -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0: version "7.0.0" resolved "https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-7.0.0.tgz" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== @@ -6930,15 +6914,6 @@ wrap-ansi@^6.2.0: string-width "^4.1.0" strip-ansi "^6.0.0" -wrap-ansi@^7.0.0: - version "7.0.0" - resolved "https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-7.0.0.tgz" - integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== - dependencies: - ansi-styles "^4.0.0" - string-width "^4.1.0" - strip-ansi "^6.0.0" - wrap-ansi@^8.1.0: version "8.1.0" resolved "https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-8.1.0.tgz" @@ -7036,6 +7011,11 @@ yargs@^17.0.0: y18n "^5.0.5" yargs-parser "^21.1.1" +yarn@^1.22.22: + version "1.22.22" + resolved "https://registry.yarnpkg.com/yarn/-/yarn-1.22.22.tgz#ac34549e6aa8e7ead463a7407e1c7390f61a6610" + integrity sha512-prL3kGtyG7o9Z9Sv8IPfBNrWTDmXB4Qbes8A9rEzt6wkJV8mUvoirjU0Mp3GGAU06Y0XQyA3/2/RQFVuK7MTfg== + yn@3.1.1: version "3.1.1" resolved "https://registry.npmjs.org/yn/-/yn-3.1.1.tgz"