Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 1 addition & 12 deletions packages/rstack/src/fmt/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import type { FmtMode, FmtRunResult } from './types.ts';
interface ParsedFmtCLIArgs {
mode: FmtMode;
patterns: string[];
parallel: boolean;
maxWorkers?: number;
help: boolean;
}
Expand All @@ -26,7 +25,6 @@ ${color.cyan('Options')}:
--write Write formatted files in place (default)
--check Check whether files are formatted
--list-different Print paths of unformatted files
--no-parallel Disable worker parallelism
--parallel-workers <count> Number of parallel workers
-h, --help Display this help message`;

Expand Down Expand Up @@ -55,8 +53,6 @@ const parseFmtCLIArgs = (args: string[]): ParsedFmtCLIArgs => {
check: { type: 'boolean' },
'list-different': { type: 'boolean' },
listDifferent: { type: 'boolean' },
'no-parallel': { type: 'boolean' },
noParallel: { type: 'boolean' },
'parallel-workers': { type: 'string' },
parallelWorkers: { type: 'string' },
help: { type: 'boolean', short: 'h' },
Expand All @@ -72,17 +68,11 @@ const parseFmtCLIArgs = (args: string[]): ParsedFmtCLIArgs => {
}

const mode = values.check ? 'check' : listDifferent ? 'list-different' : 'write';
const noParallel = values['no-parallel'] || values.noParallel;
const maxWorkers = parseMaxWorkers(values['parallel-workers'], values.parallelWorkers);

if (noParallel && maxWorkers !== undefined) {
throw new Error('The --parallel-workers and --no-parallel options cannot be used together.');
}

return {
mode,
patterns: positionals,
parallel: !noParallel,
maxWorkers,
help: values.help ?? false,
};
Expand Down Expand Up @@ -126,7 +116,7 @@ const logFmtResult = (result: FmtRunResult, mode: FmtMode, cwd: string): void =>
};

const runFmtCLI = async (args: string[]): Promise<void> => {
const { help, maxWorkers, mode, parallel, patterns } = parseFmtCLIArgs(args);
const { help, maxWorkers, mode, patterns } = parseFmtCLIArgs(args);
if (help) {
console.log(fmtHelpMessage);
return;
Expand All @@ -151,7 +141,6 @@ const runFmtCLI = async (args: string[]): Promise<void> => {
files,
mode,
cache: false,
parallel,
maxWorkers,
});

Expand Down
4 changes: 2 additions & 2 deletions packages/rstack/src/fmt/parallel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import WorkTank from 'worktank';
type FmtWorkerMethods = typeof import('./worker.ts');

interface FmtWorker {
formatFile: FmtWorkerMethods['formatFileSerial'];
formatFile: FmtWorkerMethods['formatFile'];
terminate: () => void;
}

Expand Down Expand Up @@ -46,7 +46,7 @@ const createFmtWorker = async (fileCount: number, maxWorkers?: number): Promise<
}

return {
formatFile: (file, shouldWrite) => pool.exec('formatFileSerial', [file, shouldWrite]),
formatFile: (file, shouldWrite) => pool.exec('formatFile', [file, shouldWrite]),
terminate: pool.terminate,
};
};
Expand Down
34 changes: 3 additions & 31 deletions packages/rstack/src/fmt/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import type {
FmtRunResult,
RunFmtFilesOptions,
} from './types.ts';
import { formatFileSerial } from './serial.ts';

/** Formats one file and reports whether its contents differ. */
type FormatFile = (file: FmtFileRequest, shouldWrite: boolean) => Promise<boolean>;
Expand Down Expand Up @@ -36,22 +35,8 @@ const runFmtFile = async (
}
};

/** Processes files sequentially while preserving input order. */
const runFmtFilesSerial = async (
files: FmtFileRequest[],
shouldWrite: boolean,
): Promise<FmtFileResult[]> => {
const results: FmtFileResult[] = [];

for (const file of files) {
results.push(await runFmtFile(file, shouldWrite, formatFileSerial));
}

return results;
};

/** Processes files concurrently while preserving input order. */
const runFmtFilesParallel = async (
/** Processes files in workers while preserving input order. */
const runFmtFilesWithWorkers = async (
files: FmtFileRequest[],
shouldWrite: boolean,
maxWorkers?: number,
Expand All @@ -66,16 +51,6 @@ const runFmtFilesParallel = async (
}
};

/** Checks whether every request can be cloned for a worker. */
const canRunFmtFilesParallel = (files: FmtFileRequest[]): boolean => {
try {
structuredClone(files);
return true;
} catch {
return false;
}
};

/** Maps file results to the Prettier-compatible CLI exit code. */
const getFmtExitCode = (files: FmtFileResult[]): FmtExitCode => {
let exitCode: FmtExitCode = 0;
Expand All @@ -96,15 +71,12 @@ const getFmtExitCode = (files: FmtFileResult[]): FmtExitCode => {
const runFmtFiles = async ({
files,
mode,
parallel,
maxWorkers,
}: RunFmtFilesOptions): Promise<FmtRunResult> => {
const startTime = performance.now();
const shouldWrite = mode === 'write';
const results =
parallel && files.length > 1 && canRunFmtFilesParallel(files)
? await runFmtFilesParallel(files, shouldWrite, maxWorkers)
: await runFmtFilesSerial(files, shouldWrite);
files.length === 0 ? [] : await runFmtFilesWithWorkers(files, shouldWrite, maxWorkers);

return {
files: results,
Expand Down
29 changes: 0 additions & 29 deletions packages/rstack/src/fmt/serial.ts

This file was deleted.

4 changes: 1 addition & 3 deletions packages/rstack/src/fmt/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,7 @@ interface RunFmtFilesOptions {
mode: FmtMode;
/** Persistent cache support is added in a later implementation step. */
cache: false;
/** Whether cloneable file requests should run in worker threads. */
parallel: boolean;
/** Maximum worker count when parallel execution is enabled. */
/** Maximum number of formatting workers. */
maxWorkers?: number;
}

Expand Down
30 changes: 28 additions & 2 deletions packages/rstack/src/fmt/worker.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,32 @@
import { formatFileSerial } from './serial.ts';
// Derived from @prettier/cli, see THIRD_PARTY_NOTICES.md

import { readFile, writeFile } from 'atomically';
import { format } from 'prettier';
import { getPrettierPlugins } from './prettierPlugins.ts';
import type { FmtFileRequest } from './types.ts';

const formatFile = async (
{ path, options }: FmtFileRequest,
shouldWrite: boolean,
): Promise<boolean> => {
const source = await readFile(path, 'utf8');
const formatted = await format(source, {
...options,
plugins: await getPrettierPlugins(options),
});

if (source === formatted) {
return false;
}

if (shouldWrite) {
await writeFile(path, formatted, 'utf8');
}

return true;
};

/** Confirms that the worker module and its runtime dependencies are ready. */
const initializeFmtWorker = (): true => true;

export { formatFileSerial, initializeFmtWorker };
export { formatFile, initializeFmtWorker };
23 changes: 7 additions & 16 deletions packages/rstack/tests/cli/fmt/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,7 @@ test('does not sort package.json by default', () => {
);
});

test.each([
['parallel execution', []],
['serial execution', ['--no-parallel']],
] as const)('sorts package.json with %s', (_, options) => {
test('sorts package.json with workers', () => {
writeProjectFile(
'rstack.config.ts',
`import { define } from 'rstack';
Expand All @@ -127,22 +124,19 @@ define.fmt({ sortPackageJson: true });
writeProjectFile('package.json', packageJsonSource);
writeProjectFile('packages/example/package.json', packageJsonSource);

const result = runFmt([...options, 'package.json', 'packages/example/package.json']);
const result = runFmt(['package.json', 'packages/example/package.json']);

expect(result.status).toBe(0);
expect(result.stderr).toBe('');
expect(readProjectFile('package.json')).toBe(sortedPackageJson);
expect(readProjectFile('packages/example/package.json')).toBe(sortedPackageJson);
});

test.each([
['disabling parallel execution', ['--no-parallel']],
['configuring parallel worker count', ['--parallel-workers', '1']],
] as const)('supports %s', (_, options) => {
test('supports configuring the worker count', () => {
writeProjectFile('first.ts', 'const first="first"');
writeProjectFile('second.ts', 'const second="second"');

const result = runFmt([...options, 'first.ts', 'second.ts']);
const result = runFmt(['--parallel-workers', '1', 'first.ts', 'second.ts']);

expect(result.status).toBe(0);
expect(result.stdout).toBe('first.ts\nsecond.ts\n');
Expand Down Expand Up @@ -267,10 +261,7 @@ test('returns exit code 2 for config errors', () => {
expect(result.stderr).toContain('invalid fmt config');
});

test.each([
['parallel execution', []],
['serial execution', ['--no-parallel']],
] as const)('formats with a project-local plugin using %s', (_, options) => {
test('formats with a project-local plugin in workers', () => {
writeProjectFile(
'rstack.config.ts',
`import { define } from 'rstack';
Expand All @@ -284,7 +275,7 @@ define.fmt({
writeProjectFile('first.fixture', '{"first":true}');
writeProjectFile('second.fixture', '{"second":true}');

const result = runFmt([...options, '*.fixture']);
const result = runFmt(['*.fixture']);

expect(result.status).toBe(0);
expect(result.stdout).toBe('first.fixture\nsecond.fixture\n');
Expand All @@ -293,7 +284,7 @@ define.fmt({
expect(readProjectFile('second.fixture')).toBe('{ "second": true }\n');
});

test('formats mixed plugin overrides in parallel', () => {
test('formats mixed plugin overrides in workers', () => {
writeProjectFile(
'rstack.config.ts',
`import { define } from 'rstack';
Expand Down
34 changes: 6 additions & 28 deletions packages/rstack/tests/fmt/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ test('uses write mode by default', () => {
expect(parseFmtCLIArgs([])).toEqual({
mode: 'write',
patterns: [],
parallel: true,
maxWorkers: undefined,
help: false,
});
Expand All @@ -20,17 +19,6 @@ test.each([
expect(parseFmtCLIArgs([option])).toEqual({
mode,
patterns: [],
parallel: true,
maxWorkers: undefined,
help: false,
});
});

test.each(['--no-parallel', '--noParallel'])('disables parallel execution with %s', (option) => {
expect(parseFmtCLIArgs([option])).toEqual({
mode: 'write',
patterns: [],
parallel: false,
maxWorkers: undefined,
help: false,
});
Expand All @@ -42,7 +30,6 @@ test.each(['--parallel-workers', '--parallelWorkers'])(
expect(parseFmtCLIArgs([option, '3'])).toEqual({
mode: 'write',
patterns: [],
parallel: true,
maxWorkers: 3,
help: false,
});
Expand All @@ -62,22 +49,12 @@ test('prefers the kebab-case parallel worker option', () => {
expect(parseFmtCLIArgs(['--parallel-workers', '2', '--parallelWorkers', '3']).maxWorkers).toBe(2);
});

test.each([
['--no-parallel', '--parallel-workers'],
['--noParallel', '--parallelWorkers'],
])('rejects conflicting parallel options: %s and %s', (noParallel, maxWorkersOption) => {
expect(() => parseFmtCLIArgs([noParallel, maxWorkersOption, '2'])).toThrow(
'The --parallel-workers and --no-parallel options cannot be used together.',
);
});

test('preserves file paths and globs', () => {
const patterns = ['src/file with spaces.ts', 'src/**/*.{js,ts}', '!src/generated/**'];

expect(parseFmtCLIArgs([patterns[0], '--check', ...patterns.slice(1)])).toEqual({
mode: 'check',
patterns,
parallel: true,
maxWorkers: undefined,
help: false,
});
Expand All @@ -87,7 +64,6 @@ test('treats arguments after the terminator as paths', () => {
expect(parseFmtCLIArgs(['--check', '--', '--write', '--help'])).toEqual({
mode: 'check',
patterns: ['--write', '--help'],
parallel: true,
maxWorkers: undefined,
help: false,
});
Expand All @@ -102,7 +78,6 @@ test('provides command help', () => {
expect(fmtHelpMessage).toContain('--write');
expect(fmtHelpMessage).toContain('--check');
expect(fmtHelpMessage).toContain('--list-different');
expect(fmtHelpMessage).toContain('--no-parallel');
expect(fmtHelpMessage).toContain('--parallel-workers <count>');
expect(fmtHelpMessage).toContain('-h, --help');
});
Expand All @@ -119,6 +94,9 @@ test.each([
);
});

test.each(['--unknown', '--no-cache'])('rejects unsupported option %s', (option) => {
expect(() => parseFmtCLIArgs([option])).toThrow();
});
test.each(['--unknown', '--no-cache', '--no-parallel', '--noParallel'])(
'rejects unsupported option %s',
(option) => {
expect(() => parseFmtCLIArgs([option])).toThrow();
},
);
Loading