diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md index 25b69346a9e7..c8ac47b64f5c 100644 --- a/CODING_STANDARDS.md +++ b/CODING_STANDARDS.md @@ -12,12 +12,12 @@ ES6 or TypeScript. ### General #### Write useful comments -Comments that explain what some block of code does are nice; they can tell you something in less +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. +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: @@ -38,6 +38,11 @@ if (!$attrs['tabindex']) { } ``` +In TypeScript code, use JsDoc-style comments for descriptions (on classes, members, etc.) and +use `//` style comments for everything else (explanations, background info, etc.). + +In SCSS code, always use `//` style comments. + #### Prefer more focused, granular components vs. complex, configurable components. For example, rather than doing this: @@ -55,22 +60,22 @@ do this: ``` #### Prefer small, focused modules -Keeping modules to a single responsibility makes the code easier to test, consume, and maintain. -ES6 modules offer a straightforward way to organize code into logical, granular units. +Keeping modules to a single responsibility makes the code easier to test, consume, and maintain. +ES6 modules offer a straightforward way to organize code into logical, granular units. Ideally, individual files are 200 - 300 lines of code. -As a rule of thumb, once a file draws near 400 lines (barring abnormally long constants / comments), -start considering how to refactor into smaller pieces. +As a rule of thumb, once a file draws near 400 lines (barring abnormally long constants / comments), +start considering how to refactor into smaller pieces. #### Less is more -Once a feature is released, it never goes away. We should avoid adding features that don't offer -high user value for price we pay both in maintenance, complexity, and payload size. When in doubt, -leave it out. +Once a feature is released, it never goes away. We should avoid adding features that don't offer +high user value for price we pay both in maintenance, complexity, and payload size. When in doubt, +leave it out. -This applies especially so to providing two different APIs to accomplish the same thing. Always +This applies especially to providing two different APIs to accomplish the same thing. Always prefer sticking to a _single_ API for accomplishing something. -### 100 column limit +### 100 column limit All code and docs in the repo should be 100 columns or fewer. This applies to TypeScript, SCSS, HTML, bash scripts, and markdown files. @@ -81,7 +86,7 @@ Avoid `any` where possible. If you find yourself using `any`, consider whether a appropriate in your case. For methods and properties that are part of a component's public API, all types must be explicitly -specified because our documentation tooling cannot currently infer types in places where TypeScript +specified because our documentation tooling cannot currently infer types in places where TypeScript can. #### Fluent APIs @@ -99,9 +104,9 @@ class ConfigBuilder { * Omit the `public` keyword as it is the default behavior. * Use `private` when appropriate and possible, prefixing the name with an underscore. * Use `protected` when appropriate and possible with no prefix. -* Prefix *library-internal* properties and methods with an underscore without using the `private` +* Prefix *library-internal* properties and methods with an underscore without using the `private` keyword. This is necessary for anything that must be public (to be used by Angular), but should not -be part of the user-facing API. This typically applies to symbols used in template expressions, +be part of the user-facing API. This typically applies to symbols used in template expressions, `@ViewChildren` / `@ContentChildren` properties, host bindings, and `@Input` / `@Output` properties (when using an alias). @@ -114,14 +119,14 @@ All public APIs must have user-facing comments. These are extracted and shown in on [material.angular.io](https://material.angular.io). Private and internal APIs should have JsDoc when they are not obvious. Ultimately it is the purview -of the code reviwer as to what is "obvious", but the rule of thumb is that *most* classes, -properties, and methods should have a JsDoc description. +of the code reviwer 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: ```ts /** 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: @@ -145,7 +150,7 @@ Boolean properties and return values should use "Whether..." as opposed to "True ##### General * Prefer writing out words instead of using abbreviations. -* Prefer *exact* names over short names (within reason). E.g., `labelPosition` is better than +* Prefer *exact* names over short names (within reason). E.g., `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. @@ -163,31 +168,52 @@ class UniqueSelectionDispatcher { } Avoid suffixing a class with "Service", as it communicates nothing about what the class does. Try to think of the class name as a person's job title. +Classes that correspond to a directive with an `md-` prefix should also be prefixed with `Md`. +CDK classes should not have a prefix. + ##### Methods -The name of a method should capture the action that is performed *by* that method. +The name of a method should capture the action that is performed *by* that method rather than +describing when the method will be called. For example, + +```ts +/** AVOID: does not describe what the function does. */ +handleClick() { + // ... +} + +/** PREFER: describes the action performed by the function. */ +activateRipple() { + // ... +} +``` + +#### Inheritance + +Avoid using inheritence to apply reusable behaviors to multiple components. This limits how many +behaviors can be composed. ### Angular #### Host bindings -Prefer using the `host` object in the directive configuration instead of `@HostBinding` and -`@HostListener`. We do this because TypeScript preserves the type information of methods with +Prefer using the `host` object in the directive configuration instead of `@HostBinding` and +`@HostListener`. We do this because TypeScript preserves the type information of methods with decorators, and when one of the arguments for the method is a native `Event` type, this preserved -type information can lead to runtime errors in non-browser environments (e.g., server-side -pre-rendering). +type information can lead to runtime errors in non-browser environments (e.g., server-side +pre-rendering). ### CSS #### Be cautious with use of `display: flex` -* The [baseline calculation for flex elements](http://www.w3.org/TR/css-flexbox-1/#flex-baselines) -is different than other display values, making it difficult to align flex elements with standard +* The [baseline calculation for flex elements](http://www.w3.org/TR/css-flexbox-1/#flex-baselines) +is different than other display values, making it difficult to align flex elements with standard elements like input and button. * Component outermost elements are never flex (block or inline-block) * Don't use `display: flex` on elements that will contain projected content. #### Use lowest specificity possible -Always prioritize lower specificity over other factors. Most style definitions should consist of a -single element or css selector plus necessary state modifiers. **Avoid SCSS nesting for the sake of +Always prioritize lower specificity over other factors. Most style definitions should consist of a +single element or css selector plus necessary state modifiers. **Avoid SCSS nesting for the sake of code organization.** This will allow users to much more easily override styles. For example, rather than doing this: @@ -224,12 +250,12 @@ do this: The end-user of a component should be the one to decide how much margin a component has around it. #### Prefer styling the host element vs. elements inside the template (where possible). -This makes it easier to override styles when necessary. For example, rather than +This makes it easier to override styles when necessary. For example, rather than ```scss the-host-element { // ... - + .some-child-element { color: red; } @@ -267,4 +293,4 @@ When it is not super obvious, include a brief description of what a class repres // Portion of the floating panel that sits, invisibly, on top of the input. .mat-datepicker-input-mask { } -``` +```