Skip to content

Commit

Permalink
feat: use spawn instead of fork
Browse files Browse the repository at this point in the history
  • Loading branch information
mdonnalley committed May 14, 2024
1 parent 96f22e7 commit bafa6c1
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 25 deletions.
4 changes: 2 additions & 2 deletions src/npm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import {readFile} from 'node:fs/promises'
import {createRequire} from 'node:module'
import {join, sep} from 'node:path'

import {ExecOptions, Output, fork} from './fork.js'
import {LogLevel} from './log-level.js'
import {ExecOptions, Output, spawn} from './spawn.js'

const debug = makeDebug('@oclif/plugin-plugins:npm')

Expand Down Expand Up @@ -36,7 +36,7 @@ export class NPM {

debug(`${options.cwd}: ${bin} ${args.join(' ')}`)
try {
const output = await fork(bin, args, options)
const output = await spawn(bin, args, options)
debug('npm done')
return output
} catch (error: unknown) {
Expand Down
2 changes: 1 addition & 1 deletion src/plugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ 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} from './npm.js'
import {Output} from './spawn.js'
import {uniqWith} from './util.js'
import {Yarn} from './yarn.js'

Expand Down
40 changes: 21 additions & 19 deletions src/fork.ts → src/spawn.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {Errors, ux} from '@oclif/core'
import makeDebug from 'debug'
import {fork as cpFork} from 'node:child_process'
import {spawn as cpSpawn} from 'node:child_process'
import {npmRunPathEnv} from 'npm-run-path'

import {LogLevel} from './log-level.js'
Expand All @@ -15,27 +15,29 @@ export type Output = {
stdout: string[]
}

const debug = makeDebug('@oclif/plugin-plugins:fork')
const debug = makeDebug('@oclif/plugin-plugins:spawn')

export async function fork(modulePath: string, args: string[] = [], {cwd, logLevel}: ExecOptions): Promise<Output> {
export async function spawn(modulePath: string, args: string[] = [], {cwd, logLevel}: ExecOptions): Promise<Output> {
return new Promise((resolve, reject) => {
const forked = cpFork(modulePath, args, {
// On windows, the global path to npm could be .cmd, .exe, or .js. If it's a .js file, we need to run it with node.
if (process.platform === 'win32' && modulePath.endsWith('.js')) {
args.unshift(`"${modulePath}"`)
modulePath = 'node'
}

debug('modulePath', modulePath)
debug('args', args)
const spawned = cpSpawn(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'],
stdio: 'pipe',
windowsVerbatimArguments: true,
...(process.platform === 'win32' && modulePath.toLowerCase().endsWith('.cmd') && {shell: true}),
})

const possibleLastLinesOfNpmInstall = ['up to date', 'added']
Expand All @@ -56,8 +58,8 @@ export async function fork(modulePath: string, args: string[] = [], {cwd, logLev
return logLevel !== 'silent'
}

forked.stderr?.setEncoding('utf8')
forked.stderr?.on('data', (d: Buffer) => {
spawned.stderr?.setEncoding('utf8')
spawned.stderr?.on('data', (d: Buffer) => {
const output = d.toString().trim()
stderr.push(output)
if (shouldPrint(output)) {
Expand All @@ -66,8 +68,8 @@ export async function fork(modulePath: string, args: string[] = [], {cwd, logLev
} else debug(output)
})

forked.stdout?.setEncoding('utf8')
forked.stdout?.on('data', (d: Buffer) => {
spawned.stdout?.setEncoding('utf8')
spawned.stdout?.on('data', (d: Buffer) => {
const output = d.toString().trim()
stdout.push(output)
if (shouldPrint(output)) {
Expand All @@ -76,8 +78,8 @@ export async function fork(modulePath: string, args: string[] = [], {cwd, logLev
} else debug(output)
})

forked.on('error', reject)
forked.on('exit', (code: number) => {
spawned.on('error', reject)
spawned.on('exit', (code: number) => {
if (code === 0) {
resolve({stderr, stdout})
} else {
Expand Down
6 changes: 3 additions & 3 deletions src/yarn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import makeDebug from 'debug'
import {createRequire} from 'node:module'
import {fileURLToPath} from 'node:url'

import {ExecOptions, Output, fork} from './fork.js'
import {LogLevel} from './log-level.js'
import {ExecOptions, Output, spawn} from './spawn.js'

const require = createRequire(import.meta.url)
const debug = makeDebug('@oclif/plugin-plugins:yarn')
Expand Down Expand Up @@ -32,7 +32,7 @@ export class Yarn {

debug(`${options.cwd}: ${bin} ${args.join(' ')}`)
try {
const output = await fork(bin, args, options)
const output = await spawn(bin, args, options)
debug('yarn done')
return output
} catch (error: unknown) {
Expand All @@ -54,7 +54,7 @@ export class Yarn {
this.bin = await which('yarn')
}

if (this.bin) {
if (!this.bin) {
throw new Errors.CLIError('yarn not found')
}

Expand Down

0 comments on commit bafa6c1

Please sign in to comment.