Skip to content

Commit e748423

Browse files
committed
refactor(fmt): always use workers
1 parent c7d449b commit e748423

11 files changed

Lines changed: 75 additions & 216 deletions

File tree

‎packages/rstack/src/fmt/cli.ts‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import type { FmtMode, FmtRunResult } from './types.ts';
1010
interface ParsedFmtCLIArgs {
1111
mode: FmtMode;
1212
patterns: string[];
13-
parallel: boolean;
1413
maxWorkers?: number;
1514
help: boolean;
1615
}
@@ -26,7 +25,6 @@ ${color.cyan('Options')}:
2625
--write Write formatted files in place (default)
2726
--check Check whether files are formatted
2827
--list-different Print paths of unformatted files
29-
--no-parallel Disable worker parallelism
3028
--parallel-workers <count> Number of parallel workers
3129
-h, --help Display this help message`;
3230

@@ -55,8 +53,6 @@ const parseFmtCLIArgs = (args: string[]): ParsedFmtCLIArgs => {
5553
check: { type: 'boolean' },
5654
'list-different': { type: 'boolean' },
5755
listDifferent: { type: 'boolean' },
58-
'no-parallel': { type: 'boolean' },
59-
noParallel: { type: 'boolean' },
6056
'parallel-workers': { type: 'string' },
6157
parallelWorkers: { type: 'string' },
6258
help: { type: 'boolean', short: 'h' },
@@ -72,17 +68,11 @@ const parseFmtCLIArgs = (args: string[]): ParsedFmtCLIArgs => {
7268
}
7369

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

78-
if (noParallel && maxWorkers !== undefined) {
79-
throw new Error('The --parallel-workers and --no-parallel options cannot be used together.');
80-
}
81-
8273
return {
8374
mode,
8475
patterns: positionals,
85-
parallel: !noParallel,
8676
maxWorkers,
8777
help: values.help ?? false,
8878
};
@@ -126,7 +116,7 @@ const logFmtResult = (result: FmtRunResult, mode: FmtMode, cwd: string): void =>
126116
};
127117

128118
const runFmtCLI = async (args: string[]): Promise<void> => {
129-
const { help, maxWorkers, mode, parallel, patterns } = parseFmtCLIArgs(args);
119+
const { help, maxWorkers, mode, patterns } = parseFmtCLIArgs(args);
130120
if (help) {
131121
console.log(fmtHelpMessage);
132122
return;
@@ -151,7 +141,6 @@ const runFmtCLI = async (args: string[]): Promise<void> => {
151141
files,
152142
mode,
153143
cache: false,
154-
parallel,
155144
maxWorkers,
156145
});
157146

‎packages/rstack/src/fmt/parallel.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import WorkTank from 'worktank';
66
type FmtWorkerMethods = typeof import('./worker.ts');
77

88
interface FmtWorker {
9-
formatFile: FmtWorkerMethods['formatFileSerial'];
9+
formatFile: FmtWorkerMethods['formatFile'];
1010
terminate: () => void;
1111
}
1212

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

4848
return {
49-
formatFile: (file, shouldWrite) => pool.exec('formatFileSerial', [file, shouldWrite]),
49+
formatFile: (file, shouldWrite) => pool.exec('formatFile', [file, shouldWrite]),
5050
terminate: pool.terminate,
5151
};
5252
};

‎packages/rstack/src/fmt/runner.ts‎

Lines changed: 3 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import type {
55
FmtRunResult,
66
RunFmtFilesOptions,
77
} from './types.ts';
8-
import { formatFileSerial } from './serial.ts';
98

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

39-
/** Processes files sequentially while preserving input order. */
40-
const runFmtFilesSerial = async (
41-
files: FmtFileRequest[],
42-
shouldWrite: boolean,
43-
): Promise<FmtFileResult[]> => {
44-
const results: FmtFileResult[] = [];
45-
46-
for (const file of files) {
47-
results.push(await runFmtFile(file, shouldWrite, formatFileSerial));
48-
}
49-
50-
return results;
51-
};
52-
53-
/** Processes files concurrently while preserving input order. */
54-
const runFmtFilesParallel = async (
38+
/** Processes files in workers while preserving input order. */
39+
const runFmtFilesWithWorkers = async (
5540
files: FmtFileRequest[],
5641
shouldWrite: boolean,
5742
maxWorkers?: number,
@@ -66,16 +51,6 @@ const runFmtFilesParallel = async (
6651
}
6752
};
6853

69-
/** Checks whether every request can be cloned for a worker. */
70-
const canRunFmtFilesParallel = (files: FmtFileRequest[]): boolean => {
71-
try {
72-
structuredClone(files);
73-
return true;
74-
} catch {
75-
return false;
76-
}
77-
};
78-
7954
/** Maps file results to the Prettier-compatible CLI exit code. */
8055
const getFmtExitCode = (files: FmtFileResult[]): FmtExitCode => {
8156
let exitCode: FmtExitCode = 0;
@@ -96,15 +71,12 @@ const getFmtExitCode = (files: FmtFileResult[]): FmtExitCode => {
9671
const runFmtFiles = async ({
9772
files,
9873
mode,
99-
parallel,
10074
maxWorkers,
10175
}: RunFmtFilesOptions): Promise<FmtRunResult> => {
10276
const startTime = performance.now();
10377
const shouldWrite = mode === 'write';
10478
const results =
105-
parallel && files.length > 1 && canRunFmtFilesParallel(files)
106-
? await runFmtFilesParallel(files, shouldWrite, maxWorkers)
107-
: await runFmtFilesSerial(files, shouldWrite);
79+
files.length === 0 ? [] : await runFmtFilesWithWorkers(files, shouldWrite, maxWorkers);
10880

10981
return {
11082
files: results,

‎packages/rstack/src/fmt/serial.ts‎

Lines changed: 0 additions & 29 deletions
This file was deleted.

‎packages/rstack/src/fmt/types.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,7 @@ interface RunFmtFilesOptions {
8080
mode: FmtMode;
8181
/** Persistent cache support is added in a later implementation step. */
8282
cache: false;
83-
/** Whether cloneable file requests should run in worker threads. */
84-
parallel: boolean;
85-
/** Maximum worker count when parallel execution is enabled. */
83+
/** Maximum number of formatting workers. */
8684
maxWorkers?: number;
8785
}
8886

‎packages/rstack/src/fmt/worker.ts‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,32 @@
1-
import { formatFileSerial } from './serial.ts';
1+
// Derived from @prettier/cli, see THIRD_PARTY_NOTICES.md
2+
3+
import { readFile, writeFile } from 'atomically';
4+
import { format } from 'prettier';
5+
import { getPrettierPlugins } from './prettierPlugins.ts';
6+
import type { FmtFileRequest } from './types.ts';
7+
8+
const formatFile = async (
9+
{ path, options }: FmtFileRequest,
10+
shouldWrite: boolean,
11+
): Promise<boolean> => {
12+
const source = await readFile(path, 'utf8');
13+
const formatted = await format(source, {
14+
...options,
15+
plugins: await getPrettierPlugins(options),
16+
});
17+
18+
if (source === formatted) {
19+
return false;
20+
}
21+
22+
if (shouldWrite) {
23+
await writeFile(path, formatted, 'utf8');
24+
}
25+
26+
return true;
27+
};
228

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

6-
export { formatFileSerial, initializeFmtWorker };
32+
export { formatFile, initializeFmtWorker };

‎packages/rstack/tests/cli/fmt/index.test.ts‎

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,7 @@ test('does not sort package.json by default', () => {
113113
);
114114
});
115115

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

130-
const result = runFmt([...options, 'package.json', 'packages/example/package.json']);
127+
const result = runFmt(['package.json', 'packages/example/package.json']);
131128

132129
expect(result.status).toBe(0);
133130
expect(result.stderr).toBe('');
134131
expect(readProjectFile('package.json')).toBe(sortedPackageJson);
135132
expect(readProjectFile('packages/example/package.json')).toBe(sortedPackageJson);
136133
});
137134

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

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

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

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

287-
const result = runFmt([...options, '*.fixture']);
278+
const result = runFmt(['*.fixture']);
288279

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

296-
test('formats mixed plugin overrides in parallel', () => {
287+
test('formats mixed plugin overrides in workers', () => {
297288
writeProjectFile(
298289
'rstack.config.ts',
299290
`import { define } from 'rstack';

‎packages/rstack/tests/fmt/cli.test.ts‎

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ test('uses write mode by default', () => {
55
expect(parseFmtCLIArgs([])).toEqual({
66
mode: 'write',
77
patterns: [],
8-
parallel: true,
98
maxWorkers: undefined,
109
help: false,
1110
});
@@ -20,17 +19,6 @@ test.each([
2019
expect(parseFmtCLIArgs([option])).toEqual({
2120
mode,
2221
patterns: [],
23-
parallel: true,
24-
maxWorkers: undefined,
25-
help: false,
26-
});
27-
});
28-
29-
test.each(['--no-parallel', '--noParallel'])('disables parallel execution with %s', (option) => {
30-
expect(parseFmtCLIArgs([option])).toEqual({
31-
mode: 'write',
32-
patterns: [],
33-
parallel: false,
3422
maxWorkers: undefined,
3523
help: false,
3624
});
@@ -42,7 +30,6 @@ test.each(['--parallel-workers', '--parallelWorkers'])(
4230
expect(parseFmtCLIArgs([option, '3'])).toEqual({
4331
mode: 'write',
4432
patterns: [],
45-
parallel: true,
4633
maxWorkers: 3,
4734
help: false,
4835
});
@@ -62,22 +49,12 @@ test('prefers the kebab-case parallel worker option', () => {
6249
expect(parseFmtCLIArgs(['--parallel-workers', '2', '--parallelWorkers', '3']).maxWorkers).toBe(2);
6350
});
6451

65-
test.each([
66-
['--no-parallel', '--parallel-workers'],
67-
['--noParallel', '--parallelWorkers'],
68-
])('rejects conflicting parallel options: %s and %s', (noParallel, maxWorkersOption) => {
69-
expect(() => parseFmtCLIArgs([noParallel, maxWorkersOption, '2'])).toThrow(
70-
'The --parallel-workers and --no-parallel options cannot be used together.',
71-
);
72-
});
73-
7452
test('preserves file paths and globs', () => {
7553
const patterns = ['src/file with spaces.ts', 'src/**/*.{js,ts}', '!src/generated/**'];
7654

7755
expect(parseFmtCLIArgs([patterns[0], '--check', ...patterns.slice(1)])).toEqual({
7856
mode: 'check',
7957
patterns,
80-
parallel: true,
8158
maxWorkers: undefined,
8259
help: false,
8360
});
@@ -87,7 +64,6 @@ test('treats arguments after the terminator as paths', () => {
8764
expect(parseFmtCLIArgs(['--check', '--', '--write', '--help'])).toEqual({
8865
mode: 'check',
8966
patterns: ['--write', '--help'],
90-
parallel: true,
9167
maxWorkers: undefined,
9268
help: false,
9369
});
@@ -102,7 +78,6 @@ test('provides command help', () => {
10278
expect(fmtHelpMessage).toContain('--write');
10379
expect(fmtHelpMessage).toContain('--check');
10480
expect(fmtHelpMessage).toContain('--list-different');
105-
expect(fmtHelpMessage).toContain('--no-parallel');
10681
expect(fmtHelpMessage).toContain('--parallel-workers <count>');
10782
expect(fmtHelpMessage).toContain('-h, --help');
10883
});
@@ -119,6 +94,9 @@ test.each([
11994
);
12095
});
12196

122-
test.each(['--unknown', '--no-cache'])('rejects unsupported option %s', (option) => {
123-
expect(() => parseFmtCLIArgs([option])).toThrow();
124-
});
97+
test.each(['--unknown', '--no-cache', '--no-parallel', '--noParallel'])(
98+
'rejects unsupported option %s',
99+
(option) => {
100+
expect(() => parseFmtCLIArgs([option])).toThrow();
101+
},
102+
);

0 commit comments

Comments
 (0)