255 lines
7.5 KiB
Markdown
255 lines
7.5 KiB
Markdown
# Angular Framework Coding Standards
|
|
|
|
The coding practices in this doc apply only to development on Angular itself, not applications
|
|
built _with_ Angular. (Though you can follow them too if you really want).
|
|
|
|
## Code style
|
|
|
|
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.
|
|
|
|
## Code practices
|
|
|
|
### Write useful comments
|
|
|
|
Comments that explain what some block of code does are nice; they can tell you something in less
|
|
time than it would take to follow through the code itself.
|
|
|
|
Comments that explain why some block of code exists at all, or does something the way it does,
|
|
are _invaluable_. The "why" is difficult, or sometimes impossible, to track down without seeking out
|
|
the original author. When collaborators are in the same room, this hurts productivity.
|
|
When collaborators are in different timezones, this can be devastating to productivity.
|
|
|
|
For example, this is a not-very-useful comment:
|
|
```typescript
|
|
// Set default tabindex.
|
|
if (!attributes['tabindex']) {
|
|
element.setAttribute('tabindex', '-1');
|
|
}
|
|
```
|
|
|
|
While this is much more useful:
|
|
```typescript
|
|
// Unless the user specifies otherwise, the calendar should not be a tab stop.
|
|
// This prevents ngAria from overzealously adding a tabindex to anything with an ng-model.
|
|
if (!attributes['tabindex']) {
|
|
element.setAttribute('tabindex', '-1');
|
|
}
|
|
```
|
|
|
|
In TypeScript code, use JsDoc-style comments for descriptions (on classes, members, etc.) and
|
|
use `//` style comments for everything else (explanations, background info, etc.).
|
|
|
|
### API Design
|
|
|
|
#### Boolean arguments
|
|
|
|
Generally avoid adding boolean arguments to a method in cases where that argument means
|
|
"do something extra". In these cases, prefer breaking the behavior up into different functions.
|
|
|
|
```typescript
|
|
// AVOID
|
|
function getTargetElement(createIfNotFound = false) {
|
|
// ...
|
|
}
|
|
```
|
|
|
|
```typescript
|
|
// PREFER
|
|
function getExistingTargetElement() {
|
|
// ...
|
|
}
|
|
|
|
function createTargetElement() {
|
|
// ...
|
|
}
|
|
```
|
|
|
|
You can ignore this guidance when necessary for performance reasons in framework code.
|
|
|
|
#### Optional arguments
|
|
|
|
Use optional function arguments only when such an argument makes sense for an API or when required
|
|
for performance. Don't use optional arguments merely for convenience in implementation.
|
|
|
|
### TypeScript
|
|
|
|
#### Typing
|
|
|
|
Avoid `any` where possible. If you find yourself using `any`, consider whether a generic or
|
|
`unknown` may be appropriate in your case.
|
|
|
|
#### Getters and Setters
|
|
|
|
Getters and setters introduce openings for side-effects, add more complexity for code readers,
|
|
and generate additional code when targeting older browsers.
|
|
|
|
* Only use getters and setters for `@Input` properties or when otherwise required for API
|
|
compatibility.
|
|
* Avoid long or complex getters and setters. If the logic of an accessor would take more than
|
|
three lines, introduce a new method to contain the logic.
|
|
* A getter should immediately precede its corresponding setter.
|
|
* Decorators such as `@Input` should be applied to the getter and not the setter.
|
|
* Always use a `readonly` property instead of a getter (with no setter) when possible.
|
|
|
|
```typescript
|
|
/** YES */
|
|
readonly active: boolean;
|
|
|
|
/** NO */
|
|
get active(): boolean {
|
|
// Using a getter solely to make the property read-only.
|
|
return this._active;
|
|
}
|
|
```
|
|
|
|
#### Iteration
|
|
|
|
Prefer `for` or `for of` to `Array.prototype.forEach`. The `forEach` API makes debugging harder
|
|
and may increase overhead from unnecessary function invocations (though modern browsers do usually
|
|
optimize this well).
|
|
|
|
#### JsDoc comments
|
|
|
|
All public APIs must have user-facing comments. These are extracted for API documentation and shown
|
|
in IDEs.
|
|
|
|
Private and internal APIs should have JsDoc when they are not obvious. Ultimately it is the purview
|
|
of the code reviewer as to what is "obvious", but the rule of thumb is that *most* classes,
|
|
properties, and methods should have a JsDoc description.
|
|
|
|
Properties should have a concise description of what the property means:
|
|
```typescript
|
|
/** The label position relative to the checkbox. Defaults to 'after' */
|
|
@Input() labelPosition: 'before' | 'after' = 'after';
|
|
```
|
|
|
|
Methods blocks should describe what the function does and provide a description for each parameter
|
|
and the return value:
|
|
```typescript
|
|
/**
|
|
* Opens a modal dialog containing the given component.
|
|
* @param component Type of the component to load into the dialog.
|
|
* @param config Dialog configuration options.
|
|
* @returns Reference to the newly-opened dialog.
|
|
*/
|
|
open<T>(component: ComponentType<T>, config?: MatDialogConfig): MatDialogRef<T> { ... }
|
|
```
|
|
|
|
Boolean properties and return values should use "Whether..." as opposed to "True if...":
|
|
```ts
|
|
/** Whether the button is disabled. */
|
|
disabled: boolean = false;
|
|
```
|
|
|
|
#### Try-Catch
|
|
|
|
Only use `try-catch` blocks when dealing with legitimately unexpected errors. Don't use `try` to
|
|
avoid checking for expected error conditions such as null dereference or out-of-bound array access.
|
|
|
|
Each `try-catch` block **must** include a comment that explains the
|
|
specific error being caught and why it cannot be prevented.
|
|
|
|
##### Variable declarations
|
|
|
|
Prefer `const` wherever possible, only using `let` when a value must change. Avoid `var` unless
|
|
absolutely necessary.
|
|
|
|
##### `readonly`
|
|
|
|
Use `readonly` members wherever possible.
|
|
|
|
#### Naming
|
|
|
|
##### General
|
|
|
|
* Prefer writing out words instead of using abbreviations.
|
|
* Prefer *exact* names over short names (within reason). For example, `labelPosition` is better than
|
|
`align` because the former much more exactly communicates what the property means.
|
|
* Except for `@Input()` properties, use `is` and `has` prefixes for boolean properties / methods.
|
|
|
|
##### Observables
|
|
|
|
Don't suffix observables with `$`.
|
|
|
|
##### Classes
|
|
|
|
Name classes based on their responsibility. Names should capture what the code *does*,
|
|
not how it is used:
|
|
```typescript
|
|
/** NO: */
|
|
class DefaultRouteReuseStrategy { }
|
|
|
|
/** YES: */
|
|
class NonStoringRouteReuseStrategy { }
|
|
```
|
|
|
|
##### Methods
|
|
|
|
The name of a method should capture the action performed *by* that method rather than
|
|
describing when the method will be called. For example:
|
|
|
|
```typescript
|
|
/** AVOID: does not describe what the function does. */
|
|
handleClick() {
|
|
// ...
|
|
}
|
|
|
|
/** PREFER: describes the action performed by the function. */
|
|
activateRipple() {
|
|
// ...
|
|
}
|
|
```
|
|
|
|
##### Test classes and examples
|
|
|
|
Give test classes and examples meaningful, descriptive names.
|
|
|
|
```ts
|
|
/** PREFER: describes the scenario under test. */
|
|
class FormGroupWithCheckboxAndRadios { /* ... */ }
|
|
class InputWithNgModel { /* ... */ }
|
|
|
|
|
|
/** AVOID: does not fully describe the scenario under test. */
|
|
class Comp { /* ... */ }
|
|
class InputComp { /* ... */ }
|
|
```
|
|
|
|
#### RxJS
|
|
|
|
When importing the `of` function to create an `Observable` from a value, alias the imported
|
|
function as `observableOf`.
|
|
|
|
```typescript
|
|
import {of as observableOf} from 'rxjs';
|
|
```
|
|
|
|
#### Testing
|
|
|
|
##### Test names
|
|
|
|
Use descriptive names for jasmine tests. Ideally, test names should read as a sentence, often of
|
|
the form "it should...".
|
|
|
|
```typescript
|
|
/** PREFER: describes the scenario under test. */
|
|
describe('Router', () => {
|
|
describe('with the default route reuse strategy', () => {
|
|
it('should not reuse routes upon location change', () => {
|
|
// ...
|
|
});
|
|
})
|
|
});
|
|
|
|
/** AVOID: does not fully describe the scenario under test. */
|
|
describe('Router', () => {
|
|
describe('default strategy', () => {
|
|
it('should work', () => {
|
|
// ...
|
|
});
|
|
})
|
|
});
|
|
```
|