From 17f41012d53b9949eb2a1cc17900919d5c391a2c Mon Sep 17 00:00:00 2001 From: Tetsuji Kato <6571039+iwizsophy@users.noreply.github.com> Date: Tue, 7 Apr 2026 23:12:20 +0900 Subject: [PATCH 1/2] Use generated tarballs instead of pack stdout --- src/cli-internal.ts | 87 ++++++------ tests/cli-nullability.test.ts | 246 ++++++++++++++++++++++++++++++++-- 2 files changed, 278 insertions(+), 55 deletions(-) diff --git a/src/cli-internal.ts b/src/cli-internal.ts index 581b1f5..55fd41e 100644 --- a/src/cli-internal.ts +++ b/src/cli-internal.ts @@ -5,7 +5,15 @@ import { dirname, isAbsolute, join, resolve } from 'path'; import { createReadStream, existsSync, statSync } from 'fs'; -import { mkdir, mkdtemp, writeFile, copyFile, rm, readFile } from 'fs/promises'; +import { + mkdir, + mkdtemp, + writeFile, + copyFile, + rm, + readFile, + readdir, +} from 'fs/promises'; import { createTarPacker, storeReaderToFile, @@ -36,6 +44,34 @@ const readPackageJsonFile = async (packageJsonPath: string): Promise => { return JSON5.parse(content); }; +const readGeneratedTarballPaths = async ( + packDestDir: string +): Promise => { + const entries = await readdir(packDestDir, { withFileTypes: true }); + return entries + .filter((entry) => entry.isFile() && entry.name.endsWith('.tgz')) + .map((entry) => join(packDestDir, entry.name)) + .sort(); +}; + +const resolveGeneratedTarballPath = async (packDestDir: string): Promise => { + const tarballPaths = await readGeneratedTarballPaths(packDestDir); + + if (tarballPaths.length === 1) { + return tarballPaths[0]; + } + + if (tarballPaths.length === 0) { + throw new Error( + `package pack did not produce a .tgz file in ${packDestDir}` + ); + } + + throw new Error( + `package pack produced multiple .tgz files in ${packDestDir}: ${tarballPaths.join(', ')}` + ); +}; + export type PackageManagerName = 'npm' | 'pnpm'; const dependencySectionKeys = new Set([ @@ -347,41 +383,6 @@ export const resolveWorkspaceFilesMerge = async ( }; }; -const readPackResultPath = ( - packageManager: PackageManagerName, - packDestDir: string, - stdout: string -): string => { - if (packageManager === 'pnpm') { - const parsed = JSON5.parse(stdout); - const packResult = Array.isArray(parsed) ? parsed[0] : parsed; - if (!packResult?.filename || typeof packResult.filename !== 'string') { - throw new Error('pnpm pack did not output a valid filename'); - } - - return isAbsolute(packResult.filename) - ? packResult.filename - : join(packDestDir, packResult.filename); - } - - const lines = stdout.trim().split('\n'); - const filename = - lines.find((line) => line.trim().endsWith('.tgz')) || - lines[lines.length - 1]; - if (!filename || !filename.trim().endsWith('.tgz')) { - throw new Error('npm pack did not output a valid .tgz filename'); - } - - return join(packDestDir, filename.trim()); -}; - -/** - * Execute package manager pack and return the generated tarball path - * @param packageManager - Package manager to use - * @param targetDir - Target directory to pack - * @param packDestDir - Directory to store the generated tarball (must exist) - * @returns Path to generated tarball - */ const runPack = async ( packageManager: PackageManagerName, targetDir: string, @@ -389,7 +390,13 @@ const runPack = async ( ): Promise => { const packArgs = packageManager === 'pnpm' - ? ['pack', '--json', '--pack-destination', packDestDir] + ? [ + '--reporter=ndjson', + 'pack', + '--json', + '--pack-destination', + packDestDir, + ] : ['pack', '--pack-destination', packDestDir]; return new Promise((res, rej) => { @@ -411,11 +418,7 @@ const runPack = async ( packProcess.on('close', (code) => { if (code === 0) { - try { - res(readPackResultPath(packageManager, packDestDir, stdout.trim())); - } catch (error: any) { - rej(error); - } + resolveGeneratedTarballPath(packDestDir).then(res).catch(rej); } else { const errorMessage = `${packageManager} pack failed with exit code ${code}`; const fullError = stderr diff --git a/tests/cli-nullability.test.ts b/tests/cli-nullability.test.ts index 14bd89a..e5e981b 100644 --- a/tests/cli-nullability.test.ts +++ b/tests/cli-nullability.test.ts @@ -9,12 +9,47 @@ import { EventEmitter } from 'events'; import { tmpdir } from 'os'; import { join, resolve } from 'path'; -const { getFetchGitMetadataMock, resolveRawPackageJsonObjectMock, spawnMock } = - vi.hoisted(() => ({ - getFetchGitMetadataMock: vi.fn(), - resolveRawPackageJsonObjectMock: vi.fn(), - spawnMock: vi.fn(), - })); +const { + createReadStreamMock, + createTarExtractorMock, + createEntryItemGeneratorMock, + createTarPackerMock, + extractToMock, + getFetchGitMetadataMock, + resolveRawPackageJsonObjectMock, + spawnMock, + storeReaderToFileMock, +} = vi.hoisted(() => ({ + createReadStreamMock: vi.fn(), + createTarExtractorMock: vi.fn(), + createEntryItemGeneratorMock: vi.fn(), + createTarPackerMock: vi.fn(), + extractToMock: vi.fn(), + getFetchGitMetadataMock: vi.fn(), + resolveRawPackageJsonObjectMock: vi.fn(), + spawnMock: vi.fn(), + storeReaderToFileMock: vi.fn(), +})); + +vi.mock('fs', async () => { + const actual = await vi.importActual('fs'); + return { + ...actual, + createReadStream: createReadStreamMock, + }; +}); + +vi.mock('tar-vern', async () => { + const actual = await vi.importActual('tar-vern'); + return { + ...actual, + createTarExtractor: createTarExtractorMock, + createEntryItemGenerator: createEntryItemGeneratorMock, + createTarPacker: createTarPackerMock, + extractTo: extractToMock, + storeReaderToFile: storeReaderToFileMock, + }; +}); vi.mock('../src/analyzer.ts', async () => { const actual = @@ -51,6 +86,65 @@ import { packAssets } from '../src/cli-internal.ts'; import { cliMain } from '../src/cli.ts'; import { createConsoleLogger } from '../src/internal'; +const createMockSpawnProcess = ( + stdoutChunks: string[] = [], + stderrChunks: string[] = [], + exitCode: number | null = 0, + signal: NodeJS.Signals | null = null +) => { + const childProcess = new EventEmitter() as EventEmitter & { + stdout: EventEmitter; + stderr: EventEmitter; + }; + childProcess.stdout = new EventEmitter(); + childProcess.stderr = new EventEmitter(); + + setTimeout(() => { + for (const chunk of stdoutChunks) { + childProcess.stdout.emit('data', chunk); + } + for (const chunk of stderrChunks) { + childProcess.stderr.emit('data', chunk); + } + childProcess.emit('close', exitCode, signal); + }, 0); + + return childProcess; +}; + +const setupPackAssetsTarget = (targetDir: string, outputDir: string) => { + mkdirSync(targetDir, { recursive: true }); + mkdirSync(outputDir, { recursive: true }); + writeFileSync( + join(targetDir, 'package.json'), + JSON.stringify({ name: 'test-package', version: '1.0.0' }, null, 2) + ); + + resolveRawPackageJsonObjectMock.mockResolvedValue({ + metadata: { + name: 'test-package', + version: '1.0.0', + }, + sourceMap: new Map(), + }); +}; + +const mockPackSpawn = ( + tarballNames: string[], + stdoutChunks: string[] = [], + stderrChunks: string[] = [], + exitCode: number | null = 0, + signal: NodeJS.Signals | null = null +) => { + spawnMock.mockImplementation((_command, args: string[]) => { + const packDestDir = args[args.length - 1]; + for (const tarballName of tarballNames) { + writeFileSync(join(packDestDir, tarballName), 'dummy tarball'); + } + return createMockSpawnProcess(stdoutChunks, stderrChunks, exitCode, signal); + }); +}; + describe('CLI nullability regressions', () => { let tempDir: string; @@ -66,8 +160,20 @@ describe('CLI nullability regressions', () => { getFetchGitMetadataMock.mockReset(); resolveRawPackageJsonObjectMock.mockReset(); spawnMock.mockReset(); + createReadStreamMock.mockReset(); + createTarExtractorMock.mockReset(); + createEntryItemGeneratorMock.mockReset(); + createTarPackerMock.mockReset(); + extractToMock.mockReset(); + storeReaderToFileMock.mockReset(); getFetchGitMetadataMock.mockReturnValue(async () => ({})); + createReadStreamMock.mockReturnValue(new EventEmitter() as any); + createTarExtractorMock.mockReturnValue({} as any); + createEntryItemGeneratorMock.mockReturnValue((async function* () {})()); + createTarPackerMock.mockReturnValue({} as any); + extractToMock.mockResolvedValue(undefined); + storeReaderToFileMock.mockResolvedValue(undefined); }); it('should fail when package.json readme source directory is unknown', async () => { @@ -102,17 +208,131 @@ describe('CLI nullability regressions', () => { ); }); + it('should use the sole generated tarball instead of stdout filename logs', async () => { + const targetDir = join(tempDir, 'source'); + const outputDir = join(tempDir, 'output'); + setupPackAssetsTarget(targetDir, outputDir); + + mockPackSpawn( + ['test-package-1.0.0.tgz'], + [ + '> test-package@1.0.0 prepack C:\\temp\\source\n', + '{"filename":"fake-from-script.tgz"}\n', + '{\n', + ' "name": "test-package",\n', + ' "version": "1.0.0",\n', + ' "filename": "test-package-1.0.0.tgz"\n', + '}\n', + ] + ); + + const result = await packAssets( + targetDir, + outputDir, + true, + true, + new Set(['version']), + undefined, + true, + '^', + createConsoleLogger(), + true, + 'pnpm' + ); + + expect(result).toMatchObject({ + packageFileName: 'test-package-1.0.0.tgz', + metadata: { + name: 'test-package', + version: '1.0.0', + }, + }); + + expect(spawnMock).toHaveBeenCalledTimes(1); + const [command, args, options] = spawnMock.mock.calls[0]; + expect(command).toBe('pnpm'); + expect(args).toEqual([ + '--reporter=ndjson', + 'pack', + '--json', + '--pack-destination', + expect.any(String), + ]); + const packDestDir = args[args.length - 1]; + expect(options).toMatchObject({ + cwd: targetDir, + stdio: ['ignore', 'pipe', 'pipe'], + }); + expect(createReadStreamMock).toHaveBeenCalledWith( + join(packDestDir, 'test-package-1.0.0.tgz') + ); + expect(storeReaderToFileMock).toHaveBeenCalledTimes(1); + }); + + it('should fail when pack destination does not contain a tarball', async () => { + const targetDir = join(tempDir, 'source'); + const outputDir = join(tempDir, 'output'); + setupPackAssetsTarget(targetDir, outputDir); + + mockPackSpawn([], ['{"filename":"missing-from-stdout.tgz"}\n']); + + await expect( + packAssets( + targetDir, + outputDir, + true, + true, + new Set(['version']), + undefined, + true, + '^', + createConsoleLogger(), + true, + 'pnpm' + ) + ).rejects.toThrow(/did not produce a \.tgz file/); + + expect(createReadStreamMock).not.toHaveBeenCalled(); + expect(storeReaderToFileMock).not.toHaveBeenCalled(); + }); + + it('should fail when pack destination contains multiple tarballs', async () => { + const targetDir = join(tempDir, 'source'); + const outputDir = join(tempDir, 'output'); + setupPackAssetsTarget(targetDir, outputDir); + + mockPackSpawn( + ['first-package.tgz', 'second-package.tgz'], + ['{"filename":"test-package-1.0.0.tgz"}\n'] + ); + + await expect( + packAssets( + targetDir, + outputDir, + true, + true, + new Set(['version']), + undefined, + true, + '^', + createConsoleLogger(), + true, + 'pnpm' + ) + ).rejects.toThrow(/produced multiple \.tgz files/); + + expect(createReadStreamMock).not.toHaveBeenCalled(); + expect(storeReaderToFileMock).not.toHaveBeenCalled(); + }); + it('should treat npm publish termination without an exit code as failure', async () => { const tarballPath = join(tempDir, 'package.tgz'); writeFileSync(tarballPath, 'dummy tarball'); - spawnMock.mockImplementation(() => { - const publishProcess = new EventEmitter(); - setTimeout(() => { - publishProcess.emit('close', null, 'SIGTERM'); - }, 0); - return publishProcess; - }); + spawnMock.mockImplementation(() => + createMockSpawnProcess([], [], null, 'SIGTERM') + ); const errors: string[] = []; const logger = { From f81bf185c1eab4e092e3ad20f2c6e431ece6bfdd Mon Sep 17 00:00:00 2001 From: Tetsuji Kato <6571039+iwizsophy@users.noreply.github.com> Date: Wed, 8 Apr 2026 11:54:49 +0900 Subject: [PATCH 2/2] Remove --reporter=ndjson and --json flags from pnpm pack args --- src/cli-internal.ts | 12 ++++-------- tests/cli-nullability.test.ts | 8 +------- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/src/cli-internal.ts b/src/cli-internal.ts index 55fd41e..fd0e844 100644 --- a/src/cli-internal.ts +++ b/src/cli-internal.ts @@ -54,7 +54,9 @@ const readGeneratedTarballPaths = async ( .sort(); }; -const resolveGeneratedTarballPath = async (packDestDir: string): Promise => { +const resolveGeneratedTarballPath = async ( + packDestDir: string +): Promise => { const tarballPaths = await readGeneratedTarballPaths(packDestDir); if (tarballPaths.length === 1) { @@ -390,13 +392,7 @@ const runPack = async ( ): Promise => { const packArgs = packageManager === 'pnpm' - ? [ - '--reporter=ndjson', - 'pack', - '--json', - '--pack-destination', - packDestDir, - ] + ? ['pack', '--pack-destination', packDestDir] : ['pack', '--pack-destination', packDestDir]; return new Promise((res, rej) => { diff --git a/tests/cli-nullability.test.ts b/tests/cli-nullability.test.ts index e5e981b..acf8fd4 100644 --- a/tests/cli-nullability.test.ts +++ b/tests/cli-nullability.test.ts @@ -251,13 +251,7 @@ describe('CLI nullability regressions', () => { expect(spawnMock).toHaveBeenCalledTimes(1); const [command, args, options] = spawnMock.mock.calls[0]; expect(command).toBe('pnpm'); - expect(args).toEqual([ - '--reporter=ndjson', - 'pack', - '--json', - '--pack-destination', - expect.any(String), - ]); + expect(args).toEqual(['pack', '--pack-destination', expect.any(String)]); const packDestDir = args[args.length - 1]; expect(options).toMatchObject({ cwd: targetDir,