Skip to content
Open
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
83 changes: 76 additions & 7 deletions packages/angular/cli/src/utilities/prettier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,50 @@ import { promisify } from 'node:util';
const execFileAsync = promisify(execFile);
let prettierCliPath: string | null | undefined;

/**
* Conservative upper bound for the length of a single spawned command line.
* Windows caps `CreateProcess` command lines at 32,767 characters; POSIX `ARG_MAX`
* is larger. Staying below this budget avoids `E2BIG`/`ENAMETOOLONG` when a large
* migration changes thousands of files.
*/
const MAX_COMMAND_LINE_LENGTH = 32_000;

/**
* Groups files into batches whose combined argument length (including `baseLength`
* for the fixed leading arguments) stays under `maxLength`. A file longer on its own
* than the budget still gets its own batch, so files are never dropped.
*/
export function batchFilesByArgumentLength(
files: Iterable<string>,
baseLength: number,
maxLength: number,
): string[][] {
const batches: string[][] = [];
let batch: string[] = [];
let length = baseLength;

for (const file of files) {
// Account for the separator between arguments, plus the surrounding quotes the OS
// adds to paths containing spaces when they are spawned.
const fileLength = file.length + (file.includes(' ') ? 3 : 1);

if (batch.length > 0 && length + fileLength > maxLength) {
batches.push(batch);
batch = [];
length = baseLength;
}

batch.push(file);
length += fileLength;
}

if (batch.length > 0) {
batches.push(batch);
}

return batches;
}

/**
* Formats files using Prettier.
* @param cwd The current working directory.
Expand Down Expand Up @@ -42,12 +86,37 @@ export async function formatFiles(cwd: string, files: Set<string>): Promise<void
return;
}

await execFileAsync(
process.execPath,
[prettierCliPath, '--write', '--no-error-on-unmatched-pattern', '--ignore-unknown', ...files],
{
cwd,
shell: false,
},
const baseArgs = [
prettierCliPath,
'--write',
'--no-error-on-unmatched-pattern',
'--ignore-unknown',
];
// `process.execPath` is spawned as the first argument and also counts toward the
// command-line limit; it and `prettierCliPath` are absolute paths that may contain
// spaces and therefore be quoted.
const baseLength = [process.execPath, ...baseArgs].reduce(
(total, arg) => total + arg.length + (arg.includes(' ') ? 3 : 1),
0,
);

// Spawn Prettier once per batch so repositories with many changed files do not
// overflow the OS command-line length limit. A failure in one batch (e.g. a file
// Prettier cannot parse) must not stop the remaining batches, matching the previous
// single-invocation behavior, so errors are collected and reported together.
const errors: string[] = [];
for (const batch of batchFilesByArgumentLength(files, baseLength, MAX_COMMAND_LINE_LENGTH)) {
try {
await execFileAsync(process.execPath, [...baseArgs, ...batch], {
cwd,
shell: false,
});
} catch (error) {
errors.push(error instanceof Error ? error.message : String(error));
}
}

if (errors.length > 0) {
throw new Error(errors.join('\n'));
}
Comment thread
mdlufy marked this conversation as resolved.
}
67 changes: 67 additions & 0 deletions packages/angular/cli/src/utilities/prettier_spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.dev/license
*/

import { batchFilesByArgumentLength } from './prettier';

describe('batchFilesByArgumentLength', () => {
it('returns no batches when there are no files', () => {
expect(batchFilesByArgumentLength([], 10, 100)).toEqual([]);
});

it('keeps files that fit the budget in a single batch', () => {
const files = ['a.ts', 'b.ts', 'c.ts'];

expect(batchFilesByArgumentLength(files, 0, 1000)).toEqual([files]);
});

it('splits files into multiple batches when the budget is exceeded', () => {
// Each file contributes `length + 1` => 5; a budget of 12 fits two files per batch.
expect(batchFilesByArgumentLength(['aaaa', 'bbbb', 'cccc', 'dddd'], 0, 12)).toEqual([
['aaaa', 'bbbb'],
['cccc', 'dddd'],
]);
});

it('reserves the base argument length in every batch', () => {
// base 8, each file => 5: 8 + 5 fits, 8 + 5 + 5 > 15 so the second file starts a new batch.
expect(batchFilesByArgumentLength(['aaaa', 'bbbb'], 8, 15)).toEqual([['aaaa'], ['bbbb']]);
});
Comment thread
mdlufy marked this conversation as resolved.

it('reserves extra length for files that contain spaces (quoted when spawned)', () => {
// 'a a' contains a space => length + 3 = 6; the others => length + 1 = 2. Budget 10:
// 6 + 2 + 2 fits, adding the fourth file (12) exceeds it.
expect(batchFilesByArgumentLength(['a a', 'b', 'c', 'd'], 0, 10)).toEqual([
['a a', 'b', 'c'],
['d'],
]);
});

it('never drops a file that is longer than the budget on its own', () => {
const files = ['short', 'a-very-long-single-file-name-well-over-budget', 'tiny'];

const batches = batchFilesByArgumentLength(files, 0, 10);

expect(batches.flat()).toEqual(files);
expect(batches).toContain(['a-very-long-single-file-name-well-over-budget']);
});

it('keeps multi-file batches within the budget for a large file set', () => {
const files = Array.from({ length: 5000 }, (_, index) => `path/to/file-${index}.component.ts`);
const baseLength = 40;
const maxLength = 2000;

for (const batch of batchFilesByArgumentLength(files, baseLength, maxLength)) {
const length = batch.reduce((total, file) => total + file.length + 1, baseLength);

// A batch may exceed the budget only when it is a single unavoidable oversized file.
if (batch.length > 1) {
expect(length).toBeLessThanOrEqual(maxLength);
}
}
});
});
Loading