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
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import { isPackageNameSafeForAnalytics } from '../analytics/analytics';
import { EventCustomDimension } from '../analytics/analytics-parameters';
import { getProjectByCwd, getSchematicDefaults } from '../utilities/config';
import { assertIsError } from '../utilities/error';
import { formatFiles } from '../utilities/formatting';
import { memoize } from '../utilities/memoize';
import { formatFiles } from '../utilities/prettier';
import { isTTY } from '../utilities/tty';
import {
CommandModule,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import * as semver from 'semver';
import { subscribeToWorkflow } from '../../../command-builder/utilities/schematic-workflow';
import { colors } from '../../../utilities/color';
import { assertIsError } from '../../../utilities/error';
import { formatFiles } from '../../../utilities/formatting';
import { writeErrorToLogFile } from '../../../utilities/log-file';
import { formatFiles } from '../../../utilities/prettier';
import { askChoices } from '../../../utilities/prompt';
import { isTTY } from '../../../utilities/tty';
import { coerceVersionNumber } from './cli-version';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { dirname, join } from 'node:path';
import { promisify } from 'node:util';

const execFileAsync = promisify(execFile);
let prettierCliPath: string | null | undefined;
let formatter: { name: string; cliPath: string } | null | undefined;

Comment thread
cexbrayat marked this conversation as resolved.
/**
* Conservative upper bound for the length of a single spawned command line.
Expand Down Expand Up @@ -60,7 +60,7 @@ export function batchFilesByArgumentLength(
}

/**
* Formats files using Prettier.
* Formats files using the workspace's Prettier installation, falling back to Oxfmt.
* @param cwd The current working directory.
* @param files The files to format.
*/
Expand All @@ -69,40 +69,46 @@ export async function formatFiles(cwd: string, files: Set<string>): Promise<void
return;
}

if (prettierCliPath === undefined) {
try {
const prettierPath = createRequire(cwd + '/').resolve('prettier/package.json');
const prettierPackageJson = JSON.parse(await readFile(prettierPath, 'utf-8')) as {
bin: string;
};
prettierCliPath = join(dirname(prettierPath), prettierPackageJson.bin);
} catch {
// Prettier is not installed.
prettierCliPath = null;
if (formatter === undefined) {
const workspaceRequire = createRequire(cwd + '/');
formatter = null;
for (const name of ['prettier', 'oxfmt']) {
try {
const packageJsonPath = workspaceRequire.resolve(`${name}/package.json`);
const { bin } = JSON.parse(await readFile(packageJsonPath, 'utf-8')) as {
bin: string | Record<string, string>;
};
formatter = {
name,
cliPath: join(dirname(packageJsonPath), typeof bin === 'string' ? bin : bin[name]),
};
break;
} catch {
continue;
}
}
}

if (!prettierCliPath) {
if (!formatter) {
return;
}

const baseArgs = [
prettierCliPath,
'--write',
'--no-error-on-unmatched-pattern',
'--ignore-unknown',
];
const { cliPath: formatterCliPath } = formatter;
const baseArgs = [formatterCliPath, '--write', '--no-error-on-unmatched-pattern'];
if (formatter.name === 'prettier') {
baseArgs.push('--ignore-unknown');
}
Comment thread
cexbrayat marked this conversation as resolved.
// `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
// command-line limit; it and `formatterCliPath` 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
// Spawn the formatter 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
// the formatter 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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.dev/license
*/

import { batchFilesByArgumentLength } from './prettier';
import { batchFilesByArgumentLength } from './formatting';

describe('batchFilesByArgumentLength', () => {
it('returns no batches when there are no files', () => {
Expand Down
33 changes: 33 additions & 0 deletions tests/e2e/tests/generate/formatting.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import assert from 'node:assert/strict';
import { expectFileToMatch, readFile, writeFile } from '../../utils/fs';
import { installPackage, uninstallPackage } from '../../utils/packages';
import { ng } from '../../utils/process';

export default async function () {
const untouched = 'export const untouched={value:"unchanged"};';
try {
await writeFile('src/untouched.ts', untouched);
await writeFile('.prettierrc', JSON.stringify({ singleQuote: true, semi: false }));
await ng('generate', 'service', 'prettier-only', '--skip-tests');
await expectFileToMatch('src/app/prettier-only.ts', "from '@angular/core'\n");

// Distinct configurations make the selected formatter observable when both are installed.
await installPackage('oxfmt');
await writeFile('.oxfmtrc.json', JSON.stringify({ singleQuote: false, semi: false }));
await ng('generate', 'service', 'both-formatters', '--skip-tests');
await expectFileToMatch('src/app/both-formatters.ts', "from '@angular/core'\n");

await uninstallPackage('prettier');
await ng('generate', 'service', 'oxfmt-only', '--skip-tests');
await expectFileToMatch('src/app/oxfmt-only.ts', 'from "@angular/core"\n');

await uninstallPackage('oxfmt');
await ng('generate', 'service', 'no-formatter', '--skip-tests');
await expectFileToMatch('src/app/no-formatter.ts', "from '@angular/core';\n");

assert.equal(await readFile('src/untouched.ts'), untouched);
} finally {
await uninstallPackage('oxfmt');
await installPackage('prettier');
}
}
Comment thread
cexbrayat marked this conversation as resolved.
Loading