Skip to content

Commit

Permalink
ci: complete migration to prettier formatting (#55580)
Browse files Browse the repository at this point in the history
Format the remaining unformatted files in the repository

PR Close #55580
  • Loading branch information
josephperrott authored and AndrewKushnir committed Apr 29, 2024
1 parent 49d3062 commit fd54415
Show file tree
Hide file tree
Showing 18 changed files with 47 additions and 163 deletions.
1 change: 0 additions & 1 deletion .devcontainer/recommended-devcontainer.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
"devondcarew.bazel-code",
"gkalpak.aio-docs-utils",
"ms-vscode.vscode-typescript-tslint-plugin",
"xaver.clang-format",
// The following extensions are useful when working on angular.io (i.e. inside the `aio/` directory).
//"angular.ng-template",
//"dbaeumer.vscode-eslint",
Expand Down
89 changes: 5 additions & 84 deletions .ng-dev/format.mts
Original file line number Diff line number Diff line change
Expand Up @@ -7,37 +7,8 @@ export const format: FormatConfig = {
'prettier': {
'matchers': [
'**/*.{yaml,yml}',
'adev/**/*.{js,ts}',
'devtools/**/*.{js,ts}',
'integration/**/*.{js,ts}',
'tools/**/*.{js,ts}',
'modules/**/*.{js,ts}',
'scripts/**/*.{js,ts}',
'packages/animations/**/*.{js,ts}',
'packages/bazel/**/*.{js,ts}',
'packages/benchpress/**/*.{js,ts}',
'packages/common/**/*.{js,ts}',
'packages/compiler/**/*.{js,ts}',
'packages/compiler-cli/**/*.{js,ts}',
'packages/core/**/*.{js,ts}',
'packages/docs/**/*.{js,ts}',
'packages/elements/**/*.{js,ts}',
'packages/examples/**/*.{js,ts}',
'packages/forms/**/*.{js,ts}',
'packages/language-service/**/*.{js,ts}',
'packages/localize/**/*.{js,ts}',
'packages/platform-browser/**/*.{js,ts}',
'packages/platform-browser-dynamic/**/*.{js,ts}',
'packages/platform-server/**/*.{js,ts}',
'packages/misc/**/*.{js,ts}',
'packages/private/**/*.{js,ts}',
'packages/router/**/*.{js,ts}',
'packages/service-worker/**/*.{js,ts}',
'packages/upgrade/**/*.{js,ts}',
'packages/zone.js/**/*.{js,ts}',
'**/*.{js,ts}',

// Test cases contain non valid code.
'!packages/compiler-cli/test/compliance/test_cases/**/*.{js,ts}',
// Do not format d.ts files as they are generated
'!**/*.d.ts',
// Both third_party and .yarn are directories containing copied code which should
Expand All @@ -49,60 +20,10 @@ export const format: FormatConfig = {
'!packages/core/src/i18n/locale_en.ts',
'!packages/common/locales/closure-locale.ts',
'!packages/common/src/i18n/currencies.ts',
],
},
'clang-format': {
'matchers': [
'**/*.{js,ts}',
// TODO: burn down format failures and remove aio and integration exceptions.
'!aio/**',
'!integration/**',
// Both third_party and .yarn are directories containing copied code which should
// not be modified.
'!third_party/**',
'!.yarn/**',
// Do not format d.ts files as they are generated
'!**/*.d.ts',
// Do not format generated ng-dev script
'!dev-infra/ng-dev.js',
'!dev-infra/build-worker.js',
// Do not format compliance test-cases since they must match generated code
'!packages/compiler-cli/test/compliance/test_cases/**/*.js',
// Do not format the locale files which are checked-in for Google3, but generated using
// the `generate-locales-tool` from `packages/common/locales`.
'!packages/core/src/i18n/locale_en.ts',
'!packages/common/locales/closure-locale.ts',
'!packages/common/src/i18n/currencies.ts',
// Temporarily disable formatting for adev
'!adev/**',

// Migrated to prettier
'!devtools/**/*.{js,ts}',
'!tools/**/*.{js,ts}',
'!modules/**/*.{js,ts}',
'!scripts/**/*.{js,ts}',
'!packages/animations/**/*.{js,ts}',
'!packages/bazel/**/*.{js,ts}',
'!packages/benchpress/**/*.{js,ts}',
'!packages/common/**/*.{js,ts}',
'!packages/compiler/**/*.{js,ts}',
'!packages/compiler-cli/**/*.{js,ts}',
'!packages/core/**/*.{js,ts}',
'!packages/docs/**/*.{js,ts}',
'!packages/elements/**/*.{js,ts}',
'!packages/examples/**/*.{js,ts}',
'!packages/forms/**/*.{js,ts}',
'!packages/language-service/**/*.{js,ts}',
'!packages/localize/**/*.{js,ts}',
'!packages/platform-browser/**/*.{js,ts}',
'!packages/platform-browser-dynamic/**/*.{js,ts}',
'!packages/platform-server/**/*.{js,ts}',
'!packages/misc/**/*.{js,ts}',
'!packages/private/**/*.{js,ts}',
'!packages/router/**/*.{js,ts}',
'!packages/service-worker/**/*.{js,ts}',
'!packages/upgrade/**/*.{js,ts}',
'!packages/zone.js/**/*.{js,ts}',
// Test cases contain non valid code.
'!packages/compiler-cli/test/compliance/test_cases/**/*.{js,ts}',
// Ignore AIO as its pending removal.
'!aio/**/*.{js,ts}',
],
},
'buildifier': true,
Expand Down
1 change: 0 additions & 1 deletion .vscode/extensions.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,5 @@
"recommendations": [
"BazelBuild.vscode-bazel",
"ms-vscode.vscode-typescript-tslint-plugin",
"xaver.clang-format",
],
}
6 changes: 0 additions & 6 deletions .vscode/recommended-settings.json
Original file line number Diff line number Diff line change
@@ -1,16 +1,10 @@
{
// Format js and ts files on save with `clang-format.executable`
// If `clang-format.executable` is not being used, these two settings should be removed otherwise it will break existing formatting.
// You can instead run `yarn gulp format` to manually format your code.
"[javascript]": {
"editor.formatOnSave": true,
},
"[typescript]": {
"editor.formatOnSave": true,
},
// Please install https://marketplace.visualstudio.com/items?itemName=xaver.clang-format to take advantage of `clang-format` in VSCode.
// (See https://clang.llvm.org/docs/ClangFormat.html for more info `clang-format`.)
"clang-format.executable": "${workspaceRoot}/node_modules/.bin/clang-format",
// Exclude third party modules and build artifacts from the editor watchers/searches.
"files.watcherExclude": {
"**/.git/objects/**": true,
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ To ensure consistency throughout the source code, keep these rules in mind as yo
* All public API methods **must be documented**.
* We follow [Google's JavaScript Style Guide][js-style-guide], but wrap all code at **100 characters**.

An automated formatter is available, see [DEVELOPER.md](docs/DEVELOPER.md#clang-format).
An automated formatter is available, see [DEVELOPER.md](docs/DEVELOPER.md#formatting-your-source-code).


## <a name="commit"></a> Commit Message Format
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ export class HomeComponent {
return;
}

this.filteredLocationList = this.housingLocationList.filter(
(housingLocation) => housingLocation?.city.toLowerCase().includes(text.toLowerCase()),
this.filteredLocationList = this.housingLocationList.filter((housingLocation) =>
housingLocation?.city.toLowerCase().includes(text.toLowerCase()),
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ export class HomeComponent {
return;
}

this.filteredLocationList = this.housingLocationList.filter(
(housingLocation) => housingLocation?.city.toLowerCase().includes(text.toLowerCase()),
this.filteredLocationList = this.housingLocationList.filter((housingLocation) =>
housingLocation?.city.toLowerCase().includes(text.toLowerCase()),
);
}
}
16 changes: 8 additions & 8 deletions browser-providers.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,19 @@ const customLaunchers = {

const sauceAliases = {
'CI_REQUIRED': buildConfiguration('unitTest', 'SL', true),
'CI_OPTIONAL': buildConfiguration('unitTest', 'SL', false)
'CI_OPTIONAL': buildConfiguration('unitTest', 'SL', false),
};

module.exports = {
customLaunchers : customLaunchers,
sauceAliases : sauceAliases,
customLaunchers: customLaunchers,
sauceAliases: sauceAliases,
};

function buildConfiguration(type, target, required) {
return Object.keys(config)
.filter((item) => {
const conf = config[item][type];
return conf.required === required && conf.target === target;
})
.map((item) => target + '_' + item.toUpperCase());
.filter((item) => {
const conf = config[item][type];
return conf.required === required && conf.target === target;
})
.map((item) => target + '_' + item.toUpperCase());
}
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@ export class DirectivePropertyResolver {
propPointer = inputLabels.has(propName)
? inputProps
: outputLabels.has(propName)
? outputProps
: stateProps;
? outputProps
: stateProps;
propPointer[propName] = this.directiveProperties[propName];
});

Expand Down
2 changes: 1 addition & 1 deletion docs/CODING_STANDARDS.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ built _with_ Angular. (Though you can follow them too if you really want).

The [Google JavaScript Style Guide](https://google.github.io/styleguide/jsguide.html) is the
basis for Angular's coding style, with additional guidance here pertaining to TypeScript. The team
uses `clang-format` to automatically format code; automatic formatting is enforced by CI.
uses `prettier` to automatically format code; automatic formatting is enforced by CI.

## Code practices

Expand Down
28 changes: 2 additions & 26 deletions docs/DEVELOPER.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ PRs can only be merged if the code is formatted properly and all tests are passi

<a name="formatting-your-source-code">
<a name="clang-format"></a>
<a name="prettier"></a>

### Testing changes against a local library/project

Expand Down Expand Up @@ -119,39 +120,14 @@ node --preserve-symlinks --preserve-symlinks-main node_modules/@angular/cli/lib/

## Formatting your source code

Angular uses [clang-format](https://clang.llvm.org/docs/ClangFormat.html) to format the source code.
Angular uses [prettier](https://clang.llvm.org/docs/ClangFormat.html) to format the source code.
If the source code is not properly formatted, the CI will fail and the PR cannot be merged.

You can automatically format your code by running:
- `yarn ng-dev format changed [shaOrRef]`: format only files changed since the provided sha/ref. `shaOrRef` defaults to `main`.
- `yarn ng-dev format all`: format _all_ source code
- `yarn ng-dev format files <files..>`: format only provided files

A better way is to set up your IDE to format the changed file on each file save.

### VS Code
1. Install [Clang-Format](https://marketplace.visualstudio.com/items?itemName=xaver.clang-format) extension for VS Code.
2. It will automatically pick up the settings from `.vscode/settings.json`.
If you haven't already, create a `settings.json` file by following the instructions [here](../.vscode/README.md).

### WebStorm / IntelliJ
1. Install the [ClangFormatIJ](https://plugins.jetbrains.com/plugin/8396-clangformatij) plugin
1. Open `Preferences->Tools->clang-format`
1. Find the field named "PATH"
1. Add `<PATH_TO_YOUR_WORKSPACE>/angular/node_modules/clang-format/bin/<OS>/`
where the OS options are: `darwin_x64`, `linux_x64`, and `win32`.

### Vim
1. Install [Vim Clang-Format](https://github.com/rhysd/vim-clang-format).
2. Create a [project-specific `.vimrc`](https://andrew.stwrt.ca/posts/project-specific-vimrc/) in
your Angular directory containing

```vim
let g:clang_format#command = '$ANGULAR_PATH/node_modules/.bin/clang-format'
```

where `$ANGULAR_PATH` is an environment variable of the absolute path of your Angular directory.

## Linting/verifying your Source Code

You can check that your code is properly formatted and adheres to coding style by running:
Expand Down
11 changes: 5 additions & 6 deletions goldens/public-api/manage.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,17 @@ const argv = parser(process.argv.slice(2));
// The command the user would like to run, either 'accept' or 'test'
const USER_COMMAND = argv._[0];
// The shell command to query for all Public API guard tests.
const BAZEL_PUBLIC_API_TARGET_QUERY_CMD =
`yarn -s bazel query --output label 'kind(nodejs_test, ...) intersect attr("tags", "api_guard", ...)'`
const BAZEL_PUBLIC_API_TARGET_QUERY_CMD = `yarn -s bazel query --output label 'kind(nodejs_test, ...) intersect attr("tags", "api_guard", ...)'`;
// Bazel targets for testing Public API goldens
process.stdout.write('Gathering all Public API targets');
const ALL_PUBLIC_API_TESTS = exec(BAZEL_PUBLIC_API_TARGET_QUERY_CMD, {silent: true})
.trim()
.split('\n')
.map(test => test.trim());
.trim()
.split('\n')
.map((test) => test.trim());
process.stdout.clearLine();
process.stdout.cursorTo(0);
// Bazel targets for generating Public API goldens
const ALL_PUBLIC_API_ACCEPTS = ALL_PUBLIC_API_TESTS.map(test => `${test}.accept`);
const ALL_PUBLIC_API_ACCEPTS = ALL_PUBLIC_API_TESTS.map((test) => `${test}.accept`);

/**
* Run the provided bazel commands on each provided target individually.
Expand Down
1 change: 0 additions & 1 deletion gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,4 @@ function loadTask(fileName, taskName) {
return task(gulp);
}


gulp.task('changelog:zonejs', loadTask('changelog-zonejs'));
13 changes: 5 additions & 8 deletions karma-js.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ if (!process.env.KARMA_WEB_TEST_MODE && isBazel && process.env.TEST_TARGET.inclu
process.env.KARMA_WEB_TEST_MODE = 'SL_REQUIRED';
}

module.exports = function(config) {
module.exports = function (config) {
const conf = {
frameworks: ['jasmine'],

Expand Down Expand Up @@ -55,11 +55,7 @@ module.exports = function(config) {

customLaunchers: browserProvidersConf.customLaunchers,

plugins: [
'karma-jasmine',
'karma-chrome-launcher',
'karma-sourcemap-loader',
],
plugins: ['karma-jasmine', 'karma-chrome-launcher', 'karma-sourcemap-loader'],

preprocessors: {
'**/*.js': ['sourcemap'],
Expand Down Expand Up @@ -126,7 +122,7 @@ module.exports = function(config) {
// This slows-down tests/browser restarting and can decrease stability.
// https://github.com/karma-runner/karma-sauce-launcher/blob/59b0c5c877448e064ad56449cd906743721c6b62/src/launcher/launcher.ts#L72-L79.
require('saucelabs').default.prototype.downloadJobAsset = () =>
Promise.resolve('<FAKE-LOGS>');
Promise.resolve('<FAKE-LOGS>');
}
}

Expand Down Expand Up @@ -155,7 +151,8 @@ module.exports = function(config) {
break;
default:
throw new Error(
`Unrecognized process.env.KARMA_WEB_TEST_MODE: ${process.env.KARMA_WEB_TEST_MODE}`);
`Unrecognized process.env.KARMA_WEB_TEST_MODE: ${process.env.KARMA_WEB_TEST_MODE}`,
);
}
} else {
// Run the test locally
Expand Down
12 changes: 6 additions & 6 deletions packages/circular-deps-test.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@
const path = require('path');

module.exports = {
baseDir : '../',
goldenFile : '../goldens/circular-deps/packages.json',
glob : `./**/*.ts`,
baseDir: '../',
goldenFile: '../goldens/circular-deps/packages.json',
glob: `./**/*.ts`,
// Command that will be displayed if the golden needs to be updated.
approveCommand : 'yarn ts-circular-deps:approve',
resolveModule : resolveModule,
ignoreTypeOnlyChecks : true,
approveCommand: 'yarn ts-circular-deps:approve',
resolveModule: resolveModule,
ignoreTypeOnlyChecks: true,
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -727,8 +727,8 @@ export abstract class BackendService {
db instanceof Observable
? db
: typeof (db as any).then === 'function'
? from(db as Promise<any>)
: of(db);
? from(db as Promise<any>)
: of(db);
db$.pipe(first()).subscribe((d: {}) => {
this.db = d;
this.dbReadySubject && this.dbReadySubject.next(true);
Expand Down
4 changes: 2 additions & 2 deletions packages/zone.js/DEVELOPER.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Run tslint:

`yarn lint`

Run format with clang-format:
Run format with prettier:

`yarn format`

Expand All @@ -35,7 +35,7 @@ Before Commit
Please make sure you pass all following checks before commit

- yarn gulp lint (tslint)
- yarn gulp format (clang-format)
- yarn gulp format (prettier)
- yarn promisetest (promise a+ test)
- yarn bazel test //packages/zone.js/... (all tests)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,10 +248,10 @@
return 'function' == typeof value
? value.name || value
: 'string' == typeof value
? value
: null == value
? ''
: '' + value;
? value
: null == value
? ''
: '' + value;
}
/**
*@license
Expand Down

0 comments on commit fd54415

Please sign in to comment.