diff --git a/.changeset/tidy-dragons-wait.md b/.changeset/tidy-dragons-wait.md new file mode 100644 index 0000000000..a712f68028 --- /dev/null +++ b/.changeset/tidy-dragons-wait.md @@ -0,0 +1,6 @@ +--- +'@lg-tools/link': minor +'@lg-tools/cli': minor +--- + +developer experience slightly improves by adding more detailed logging to spawn processes in the link script. Previously, running link script would lead to a group of processes being spawned in parallel all piping their stdout and stderr to the console in verbose mode which made it difficult to distinguish what each line of output was from which process. The command and working directory are also now logged for each process along with their exit code. diff --git a/tools/link/src/utils/createLinkFrom.spec.ts b/tools/link/src/utils/createLinkFrom.spec.ts index daf57cb3a5..834603d923 100644 --- a/tools/link/src/utils/createLinkFrom.spec.ts +++ b/tools/link/src/utils/createLinkFrom.spec.ts @@ -1,13 +1,11 @@ -import { ChildProcess } from 'child_process'; -import xSpawn from 'cross-spawn'; import fsx from 'fs-extra'; import path from 'path'; import { createLinkFrom } from './createLinkFrom'; -import { MockChildProcess } from './mocks.testutils'; +import * as spawnLoggedModule from './spawnLogged'; describe('tools/link/createLinkFrom', () => { - let spawnSpy: jest.SpyInstance; + let spawnLoggedSpy: jest.SpyInstance; beforeAll(() => { fsx.emptyDirSync('./tmp'); @@ -18,30 +16,32 @@ describe('tools/link/createLinkFrom', () => { }); beforeEach(() => { - spawnSpy = jest.spyOn(xSpawn, 'spawn'); - spawnSpy.mockImplementation((..._args) => new MockChildProcess()); + spawnLoggedSpy = jest + .spyOn(spawnLoggedModule, 'spawnLogged') + .mockResolvedValue(undefined); }); afterEach(() => { - spawnSpy.mockRestore(); + spawnLoggedSpy.mockRestore(); fsx.emptyDirSync('./tmp'); }); afterAll(() => { - fsx.rmdirSync('./tmp/'); + fsx.removeSync('./tmp/'); }); - test('calls `npm link` command from package directory', () => { - createLinkFrom(path.resolve('./tmp/'), { + test('calls `npm link` command from package directory', async () => { + await createLinkFrom(path.resolve('./tmp/'), { scopeName: '@example', scopePath: 'scope', packageName: 'test-package', }); - expect(spawnSpy).toHaveBeenCalledWith( + expect(spawnLoggedSpy).toHaveBeenCalledWith( 'npm', ['link'], expect.objectContaining({ + name: 'link_src:test-package', cwd: expect.stringContaining('tmp/scope/test-package'), }), ); diff --git a/tools/link/src/utils/createLinkFrom.ts b/tools/link/src/utils/createLinkFrom.ts index 482acac8c0..f8d55b4490 100644 --- a/tools/link/src/utils/createLinkFrom.ts +++ b/tools/link/src/utils/createLinkFrom.ts @@ -1,10 +1,10 @@ /* eslint-disable no-console */ import { getPackageManager, SupportedPackageManager } from '@lg-tools/meta'; import chalk from 'chalk'; -import { spawn } from 'cross-spawn'; import path from 'path'; import { findDirectory } from './findDirectory'; +import { spawnLogged } from './spawnLogged'; import { PackageDetails } from './types'; interface CreateLinkOptions extends PackageDetails { @@ -16,7 +16,7 @@ interface CreateLinkOptions extends PackageDetails { * Runs the pnpm link command in a leafygreen-ui package directory * @returns Promise that resolves when the pnpm link command has finished */ -export function createLinkFrom( +export async function createLinkFrom( source: string = process.cwd(), { scopeName, @@ -27,29 +27,30 @@ export function createLinkFrom( }: CreateLinkOptions, ): Promise { const scopeSrc = scopePath; - return new Promise(resolve => { - const packagesDirectory = findDirectory(source, scopeSrc); - packageManager = packageManager ?? getPackageManager(source); + const packagesDirectory = findDirectory(source, scopeSrc); - if (packagesDirectory) { - verbose && - console.log( - 'Creating link for:', - chalk.green(`${scopeName}/${packageName}`), - ); + if (packagesDirectory) { + verbose && + console.log( + 'Creating link for:', + chalk.green(`${scopeName}/${packageName}`), + ); + + const resolvedPackageManager = + packageManager ?? getPackageManager(packagesDirectory); - spawn(packageManager, ['link'], { + try { + await spawnLogged(resolvedPackageManager, ['link'], { + name: `link_src:${packageName}`, cwd: path.join(packagesDirectory, packageName), - stdio: verbose ? 'inherit' : 'ignore', - }) - .on('close', resolve) - .on('error', () => { - throw new Error(`Couldn't create link for package: ${packageName}`); - }); - } else { - throw new Error( - `Can't find a ${scopeSrc} directory in ${process.cwd()} or any of its parent directories.`, - ); + verbose, + }); + } catch (_) { + throw new Error(`Couldn't create link for package: ${packageName}`); } - }); + } else { + throw new Error( + `Can't find a ${scopeSrc} directory in ${process.cwd()} or any of its parent directories.`, + ); + } } diff --git a/tools/link/src/utils/install.spec.ts b/tools/link/src/utils/install.spec.ts index caf18307d1..06ddcb3c88 100644 --- a/tools/link/src/utils/install.spec.ts +++ b/tools/link/src/utils/install.spec.ts @@ -1,24 +1,23 @@ -import { ChildProcess } from 'child_process'; -import xSpawn from 'cross-spawn'; import fsx from 'fs-extra'; import { installPackages } from './install'; -import { MockChildProcess } from './mocks.testutils'; +import * as spawnLoggedModule from './spawnLogged'; describe('tools/link/utils/install', () => { - let spawnSpy: jest.SpyInstance; + let spawnLoggedSpy: jest.SpyInstance; beforeAll(() => { fsx.ensureDirSync('./tmp/'); }); beforeEach(() => { - spawnSpy = jest.spyOn(xSpawn, 'spawn'); - spawnSpy.mockImplementation((..._args) => new MockChildProcess()); + spawnLoggedSpy = jest + .spyOn(spawnLoggedModule, 'spawnLogged') + .mockResolvedValue(undefined); }); afterEach(() => { - spawnSpy.mockRestore(); + spawnLoggedSpy.mockRestore(); fsx.emptyDirSync('./tmp'); }); @@ -28,20 +27,24 @@ describe('tools/link/utils/install', () => { test('runs `npm install` command', async () => { await installPackages('./tmp'); - expect(spawnSpy).toHaveBeenCalledWith( + expect(spawnLoggedSpy).toHaveBeenCalledWith( 'npm', ['install'], - expect.objectContaining({}), + expect.objectContaining({ + cwd: './tmp', + }), ); }); test('runs install command using local package manager', async () => { fsx.createFileSync('./tmp/pnpm-lock.yaml'); await installPackages('./tmp'); - expect(spawnSpy).toHaveBeenCalledWith( + expect(spawnLoggedSpy).toHaveBeenCalledWith( 'pnpm', ['install'], - expect.objectContaining({}), + expect.objectContaining({ + cwd: './tmp', + }), ); }); }); diff --git a/tools/link/src/utils/install.ts b/tools/link/src/utils/install.ts index a24652cb00..4e6b638b9a 100644 --- a/tools/link/src/utils/install.ts +++ b/tools/link/src/utils/install.ts @@ -1,7 +1,8 @@ import { getPackageManager, SupportedPackageManager } from '@lg-tools/meta'; -import { spawn } from 'cross-spawn'; import fsx from 'fs-extra'; +import { spawnLogged } from './spawnLogged'; + export async function installPackages( path: string, options?: { @@ -9,21 +10,19 @@ export async function installPackages( verbose?: boolean; }, ): Promise { - return new Promise((resolve, reject) => { - if (fsx.existsSync(path)) { - const pkgMgr = options?.packageManager ?? getPackageManager(path); + if (fsx.existsSync(path)) { + const pkgMgr = options?.packageManager ?? getPackageManager(path); - spawn(pkgMgr, ['install'], { + try { + await spawnLogged(pkgMgr, ['install'], { + name: 'install', cwd: path, - stdio: options?.verbose ? 'inherit' : 'ignore', - }) - .on('close', resolve) - .on('error', err => { - throw new Error(`Error installing packages\n` + err); - }); - } else { - console.error(`Path ${path} does not exist`); - reject(); + verbose: options?.verbose, + }); + } catch (err) { + throw new Error(`Error installing packages\n` + err); } - }); + } else { + throw new Error(`Path ${path} does not exist`); + } } diff --git a/tools/link/src/utils/linkPackageTo.spec.ts b/tools/link/src/utils/linkPackageTo.spec.ts index b1b02b0147..63ad120534 100644 --- a/tools/link/src/utils/linkPackageTo.spec.ts +++ b/tools/link/src/utils/linkPackageTo.spec.ts @@ -1,13 +1,11 @@ -import { ChildProcess } from 'child_process'; -import xSpawn from 'cross-spawn'; import fsx from 'fs-extra'; import path from 'path'; import { linkPackageTo } from './linkPackageTo'; -import { MockChildProcess } from './mocks.testutils'; +import * as spawnLoggedModule from './spawnLogged'; describe('tools/link/linkPackageTo', () => { - let spawnSpy: jest.SpyInstance; + let spawnLoggedSpy: jest.SpyInstance; beforeAll(() => { fsx.ensureDirSync('./tmp/app'); @@ -15,12 +13,13 @@ describe('tools/link/linkPackageTo', () => { }); beforeEach(() => { - spawnSpy = jest.spyOn(xSpawn, 'spawn'); - spawnSpy.mockImplementation((..._args) => new MockChildProcess()); + spawnLoggedSpy = jest + .spyOn(spawnLoggedModule, 'spawnLogged') + .mockResolvedValue(undefined); }); afterEach(() => { - spawnSpy.mockRestore(); + spawnLoggedSpy.mockRestore(); fsx.emptyDirSync('./tmp'); }); @@ -28,16 +27,17 @@ describe('tools/link/linkPackageTo', () => { fsx.rmdirSync('./tmp/'); }); - test('calls `npm link ` from the destination directory', () => { - linkPackageTo(path.resolve('./tmp/app'), { + test('calls `npm link ` from the destination directory', async () => { + await linkPackageTo(path.resolve('./tmp/app'), { scopeName: '@example', packageName: 'test-package', }); - expect(spawnSpy).toHaveBeenCalledWith( + expect(spawnLoggedSpy).toHaveBeenCalledWith( 'npm', ['link', '@example/test-package'], expect.objectContaining({ + name: 'link_dst:test-package', cwd: expect.stringContaining('tmp/app'), }), ); diff --git a/tools/link/src/utils/linkPackageTo.ts b/tools/link/src/utils/linkPackageTo.ts index b6e5aafe55..9a2f68a572 100644 --- a/tools/link/src/utils/linkPackageTo.ts +++ b/tools/link/src/utils/linkPackageTo.ts @@ -1,7 +1,7 @@ import { getPackageManager, SupportedPackageManager } from '@lg-tools/meta'; import chalk from 'chalk'; -import { spawn } from 'cross-spawn'; +import { spawnLogged } from './spawnLogged'; import { PackageDetails } from './types'; interface LinkPackagesToOptions @@ -14,23 +14,24 @@ interface LinkPackagesToOptions * Runs the pnpm link command in the destination directory * @returns Promise that resolves when the pnpm link command has finished */ -export function linkPackageTo( +export async function linkPackageTo( destination: string, { scopeName, packageName, verbose, packageManager }: LinkPackagesToOptions, ): Promise { const fullPackageName = `${scopeName}/${packageName}`; - return new Promise(resolve => { - // eslint-disable-next-line no-console - verbose && console.log('Linking package:', chalk.blue(fullPackageName)); - packageManager = packageManager ?? getPackageManager(destination); + // eslint-disable-next-line no-console + verbose && console.log('Linking package:', chalk.blue(fullPackageName)); - spawn(packageManager, ['link', fullPackageName], { + const resolvedPackageManager = + packageManager ?? getPackageManager(destination); + + try { + await spawnLogged(resolvedPackageManager, ['link', fullPackageName], { + name: `link_dst:${packageName}`, cwd: destination, - stdio: verbose ? 'inherit' : 'ignore', - }) - .on('close', resolve) - .on('error', () => { - throw new Error(`Couldn't link package: ${fullPackageName}`); - }); - }); + verbose, + }); + } catch (_) { + throw new Error(`Couldn't link package: ${fullPackageName}`); + } } diff --git a/tools/link/src/utils/linkPackagesForScope.spec.ts b/tools/link/src/utils/linkPackagesForScope.spec.ts index dfd6705393..ea2384f212 100644 --- a/tools/link/src/utils/linkPackagesForScope.spec.ts +++ b/tools/link/src/utils/linkPackagesForScope.spec.ts @@ -1,13 +1,11 @@ -import { ChildProcess } from 'child_process'; -import xSpawn from 'cross-spawn'; import fsx from 'fs-extra'; import path from 'path'; import { linkPackagesForScope } from './linkPackagesForScope'; -import { MockChildProcess } from './mocks.testutils'; +import * as spawnLoggedModule from './spawnLogged'; describe('tools/link/linkPackagesForScope', () => { - let spawnSpy: jest.SpyInstance; + let spawnLoggedSpy: jest.SpyInstance; beforeAll(() => { fsx.emptyDirSync('./tmp'); @@ -16,12 +14,13 @@ describe('tools/link/linkPackagesForScope', () => { }); beforeEach(() => { - spawnSpy = jest.spyOn(xSpawn, 'spawn'); - spawnSpy.mockImplementation((..._args) => new MockChildProcess()); + spawnLoggedSpy = jest + .spyOn(spawnLoggedModule, 'spawnLogged') + .mockResolvedValue(undefined); }); afterEach(() => { - spawnSpy.mockRestore(); + spawnLoggedSpy.mockRestore(); fsx.emptyDirSync('./tmp'); }); @@ -48,19 +47,21 @@ describe('tools/link/linkPackagesForScope', () => { ); // Creates links - expect(spawnSpy).toHaveBeenCalledWith( + expect(spawnLoggedSpy).toHaveBeenCalledWith( 'npm', ['link'], expect.objectContaining({ + name: 'link_src:test-package', cwd: expect.stringContaining('tmp/packages/scope/test-package'), }), ); // Consumes links - expect(spawnSpy).toHaveBeenCalledWith( + expect(spawnLoggedSpy).toHaveBeenCalledWith( 'npm', ['link', '@example/test-package'], expect.objectContaining({ + name: 'link_dst:test-package', cwd: expect.stringContaining('tmp/app'), }), ); diff --git a/tools/link/src/utils/spawnLogged.spec.ts b/tools/link/src/utils/spawnLogged.spec.ts new file mode 100644 index 0000000000..fc4719920a --- /dev/null +++ b/tools/link/src/utils/spawnLogged.spec.ts @@ -0,0 +1,171 @@ +/* eslint-disable no-console */ +import chalk from 'chalk'; + +import { SpawnLogger } from './spawnLogged'; + +describe('tools/link/utils/spawnLogged', () => { + let originalConsoleLog: typeof console.log; + let originalConsoleError: typeof console.error; + let logSpy: jest.SpyInstance; + let errorSpy: jest.SpyInstance; + + beforeEach(() => { + originalConsoleLog = console.log; + originalConsoleError = console.error; + logSpy = jest.spyOn(console, 'log').mockImplementation(); + errorSpy = jest.spyOn(console, 'error').mockImplementation(); + }); + + afterEach(() => { + logSpy.mockRestore(); + errorSpy.mockRestore(); + console.log = originalConsoleLog; + console.error = originalConsoleError; + }); + + test('executes a successful command and logs output', async () => { + const spawnLogger = new SpawnLogger(); + + await expect( + spawnLogger.spawn('echo', ['hi!'], { + name: 'me', + cwd: process.cwd(), + verbose: false, + }), + ).resolves.toBeUndefined(); + + expect(logSpy).toHaveBeenCalledTimes(2); + + expect(logSpy).toHaveBeenNthCalledWith( + 1, + '\n' + + `${chalk.cyan('[me]')} ${chalk.dim('→')} ${chalk.dim(process.cwd())}` + + '\n' + + `${chalk.cyan('[me]')} ${chalk.cyan('$')} ${chalk.bold('echo hi!')}` + + '\n', + ); + + expect(logSpy).toHaveBeenNthCalledWith( + 2, + `${chalk.cyan('[me]')} ${chalk.dim('→')} ${chalk.dim( + 'finished successfully', + )}` + '\n', + ); + }); + + test('executes a successful command with verbose output', async () => { + const spawnLogger = new SpawnLogger(); + + await expect( + spawnLogger.spawn('echo', ['hi!'], { + name: 'me', + cwd: process.cwd(), + verbose: true, + }), + ).resolves.toBeUndefined(); + + expect(logSpy).toHaveBeenCalledTimes(3); + + expect(logSpy).toHaveBeenNthCalledWith( + 1, + '\n' + + `${chalk.cyan('[me]')} ${chalk.dim('→')} ${chalk.dim(process.cwd())}` + + '\n' + + `${chalk.cyan('[me]')} ${chalk.cyan('$')} ${chalk.bold('echo hi!')}` + + '\n', + ); + + expect(logSpy).toHaveBeenNthCalledWith( + 2, + `${chalk.cyan('[me]')} ${chalk.dim('hi!')}`, + ); + + expect(logSpy).toHaveBeenNthCalledWith( + 3, + `${chalk.cyan('[me]')} ${chalk.dim('→')} ${chalk.dim( + 'finished successfully', + )}` + '\n', + ); + }); + + test('executes a failing command and throws error', async () => { + const spawnLogger = new SpawnLogger(); + + await expect( + spawnLogger.spawn('sh', ['-c', 'exit 1'], { + name: 'you', + cwd: process.cwd(), + verbose: false, + }), + ).rejects.toThrow('Command failed with exit code 1'); + + // Should log the command being executed + expect(logSpy).toHaveBeenCalledTimes(2); + + expect(logSpy).toHaveBeenNthCalledWith( + 1, + '\n' + + `${chalk.cyan('[you]')} ${chalk.dim('→')} ${chalk.dim(process.cwd())}` + + '\n' + + `${chalk.cyan('[you]')} ${chalk.cyan('$')} ${chalk.bold( + 'sh -c exit 1', + )}` + + '\n', + ); + + expect(logSpy).toHaveBeenNthCalledWith( + 2, + `${chalk.cyan('[you]')} ${chalk.dim('→')} ${chalk.red( + 'finished with exit code 1', + )}\n`, + ); + }); + + test('executes a failing command with verbose output', async () => { + const spawnLogger = new SpawnLogger(); + + await expect( + spawnLogger.spawn('sh', ['-c', 'echo hi >&2 && exit 10'], { + name: 'you', + cwd: process.cwd(), + verbose: true, + }), + ).rejects.toThrow('Command failed with exit code 10'); + + // Should log the command being executed + expect(logSpy).toHaveBeenCalledTimes(2); + expect(errorSpy).toHaveBeenCalledTimes(1); + + expect(errorSpy).toHaveBeenNthCalledWith( + 1, + `${chalk.cyan('[you]')} ${chalk.reset('hi')}`, + ); + + expect(logSpy).toHaveBeenNthCalledWith( + 2, + `${chalk.cyan('[you]')} ${chalk.dim('→')} ${chalk.red( + 'finished with exit code 10', + )}\n`, + ); + }); + + test('handles environment variables', async () => { + const spawnLogger = new SpawnLogger(); + + await expect( + spawnLogger.spawn('sh', ['-c', 'echo $TEST_VAR'], { + name: 'env-test', + cwd: process.cwd(), + env: { TEST_VAR: 'test-value', PATH: process.env.PATH }, + verbose: true, + }), + ).resolves.toBeUndefined(); + + expect(logSpy).toHaveBeenCalledTimes(3); + + expect(logSpy).toHaveBeenNthCalledWith( + 2, + `${chalk.cyan('[env-test]')} ${chalk.dim('test-value')}`, + ); + }); +}); diff --git a/tools/link/src/utils/spawnLogged.ts b/tools/link/src/utils/spawnLogged.ts new file mode 100644 index 0000000000..45dde74e96 --- /dev/null +++ b/tools/link/src/utils/spawnLogged.ts @@ -0,0 +1,137 @@ +/* eslint-disable no-console */ +import chalk from 'chalk'; +import spawn from 'cross-spawn'; +import readline from 'readline'; + +export interface SpawnLoggedOptions { + /** + * The name of the command to prefix the logs with + */ + name?: string; + /** + * Whether to pipe the command's stdout and stderr to the console + */ + verbose?: boolean; + /** + * The working directory where the command will run inside + */ + cwd: string; + /** + * The environment variables to use for the spawned process + */ + env?: NodeJS.ProcessEnv; +} + +export class SpawnLogger { + /** + * Available colors for PID prefixes + */ + COLORS = [ + chalk.cyan, + chalk.green, + chalk.yellow, + chalk.blue, + chalk.magenta, + chalk.greenBright, + chalk.yellowBright, + chalk.blueBright, + chalk.magentaBright, + chalk.cyanBright, + ] as const; + + /** + * Round-robin color index + */ + colorIndex = 0; + + /** + * Get the next color in round-robin fashion + */ + getNextColor() { + const color = this.COLORS[this.colorIndex]; + this.colorIndex = (this.colorIndex + 1) % this.COLORS.length; + return color; + } + + /** + * Spawns a command and logs the output + * @param command - The command to execute (e.g., 'pnpm', 'npm') + * @param args - The command arguments + * @param options - The options for the spawn command + * @param options.name - The name of the command to prefix its output logs with. Defaults to launched process id. + * @param options.verbose - Whether to pipe the command's stdout and stderr to the console. + * @param options.cwd - The working directory where the command will run. + * @param options.env - The environment variables to use for the spawned process (no other environment variables will be used). + */ + async spawn( + command: string, + args: Array, + options: SpawnLoggedOptions, + ): Promise { + const { name, verbose, cwd, env } = options; + + const child = spawn(command, args, { + cwd, + env, + }); + + const colorFn = this.getNextColor(); + const prefix = colorFn(name ? `[${name}]` : `[${child.pid}]`); + + const commandStr = [command, ...args].join(' '); + console.log( + `\n${prefix} ${chalk.dim('→')} ${chalk.dim(cwd)}\n` + + `${prefix} ${chalk.cyan('$')} ${chalk.bold(commandStr)}\n`, + ); + + const exitCode = await new Promise((resolve, reject) => { + if (verbose) { + const [stdout, stderr] = [ + readline.createInterface({ input: child.stdout! }), + readline.createInterface({ input: child.stderr! }), + ]; + + stdout.on('line', line => { + console.log(`${prefix} ${chalk.dim(line)}`); + }); + + stderr.on('line', line => { + console.error(`${prefix} ${chalk.reset(line)}`); + }); + + child.on('exit', function () { + stdout.close(); + stderr.close(); + }); + } + + child.on('exit', code => resolve(code)); + child.on('error', reject); + child.on('disconnect', reject); + }); + + if (exitCode) { + console.log( + `${prefix} ${chalk.dim('→')} ${chalk.red( + `finished with exit code ${exitCode}`, + )}\n`, + ); + + throw new Error(`Command failed with exit code ${exitCode}`); + } else { + console.log( + `${prefix} ${chalk.dim('→')} ${chalk.dim('finished successfully')}\n`, + ); + } + } +} + +const defaultInstance = new SpawnLogger(); + +export async function spawnLogged( + command: string, + args: Array, + options: SpawnLoggedOptions, +): Promise { + return defaultInstance.spawn(command, args, options); +}