-
Notifications
You must be signed in to change notification settings - Fork 902
Conventions
We are in the process of migrating Deck from Angular 1 to React. Please submit all new code in React and Typescript.
Imports should be ordered as follows:
-
'angular'
should always be the first thingimport
ed (TS, TSX) orrequire
d as aconst
(JS) - any third-party imports should follow
'angular'
- a blank line
- a single import from
@spinnaker/core
, if applicable - a blank line
- any imports from the current module
- a blank line
- any .less files, which should be
import
ed, notrequire
d.
When importing, there should be a space between curly brackets, e.g. { module }
, not {module}
. Like most things in the codebase (including the import styles above), you'll see a mix of both styles, as we often do not settle on a convention until we've written quite a bit of code, and we often do not update existing code unless we're already in a file making changes.
All files that have a close relation should reside in the same file structure. This includes html, js/ts/tsx, less/css, and unit test files.
Example
We have a module that is responsible for viewing a cluster. This would probably be called the cluster module. The file structure would look something like this:
app/scripts/modules/core ├── cluster │ ├── cluster.module.ts │ ├── cluster.service.ts │ ├── cluster.service.spec.ts │ ├── clusterToggles.directive.html │ ├── clusterToggles.component.ts │ └── index.ts
[moduleName].[artifactType].js
Artifact Types:
- controller
- service
- component
- directive (deprecated - favor use of components)
- provider
- values
- constants
Examples
pipeline.controller.ts
pipeline.service.ts
pipelineSelector.component.ts
feedback.modal.controller.ts (slight deviation from other examples)
Name the tests the same as the file under test and append .spec.ts
Example
File under test:
pipeline.controller.ts
Test name:
pipeline.controller.spec.ts
React component files should have the same name as the component, e.g. ExecutionGroup.tsx
, LoadBalancerPod.tsx
[moduleName].[templateType].html
Template Types:
- component: if the template is included in a component via
templateUrl
- modal: if the template is used in a modal
- view: if the template is used in a ui-router's
ng-view
Examples
pipeline.component.html
pipeline.modal.html
The angular module names should loosely follow the file naming rules for the JS/TS files.
[namespace].[module].[artifactType]
Namespace:
- spinnaker.core: core Spinnaker code
- spinnaker.aws: AWS-specific code
- spinnaker.gce: GCE-specific code
Example
File Name: pipeline.controller.ts
import { module } from 'angular';
export class PipelineController {
// ...
}
export const PIPELINE_CONTROLLER = 'spinnaker.core.pipeline.controller';
module(PIPELINE_CONTROLLER, [])
.controller('pipelineController', PipelineController)
Deck uses LESS to generate CSS. If you need to customize the CSS for a component, create a LESS file with the same name as the component, e.g. pipeline.component.less
. Favor BEM for class names, and restrict the CSS to the component:
@import "../less/imports/commonImports.less";
pipeline {
display: block;
background-color: @light_blue_background;
h3 {
font-size: 90%;
}
.pipeline-stage {
padding: 10px;
}
.pipeline-stage-active {
font-weight: 600;
}
}
With the introduction of webpack to the deck application we now have 2 module systems in play:
- The angular module system which is used for Dependency Injection (DI).
- The CommonsJS style that is used for file system level dependency resolution.
These 2 systems play nicely together as long as you follow a few conventions.
Prefer ES2015 features when possible; avoid using lodash for things like iteration, map, filter unless it results in cleaner, easier to read code.
ES2015 features we strongly favor:
-
let
: for block scoped variables -
const
: for constant variables - arrow functions
- object destructuring, e.g.
let { bar, baz } = foo; // where foo is an object with "bar" and "baz" as fields
- spread/rest operators
- default params for functions
- string templates for multi-line strings or simple substitutions
Relying on a promise to setState
can introduce a memory leak if the react component unmounts while the promise is pending.
This can be prevented by wrapping the promise in a RxJS observable and taking values until a separate notifier observable emits.
import * as React from 'react';
import { Observable, Subject } from 'rxjs';
import { ApplicationReader, IApplicationSummary } from 'core/application';
interface IExampleComponentState {
applications: IApplicationSummary[];
}
export default class ExampleComponent extends React.Component<{}, IExampleComponentState> {
public state: IExampleComponentState = {
applications: [],
};
// Notifier observable
private destroy$ = new Subject();
public componentDidMount(): void {
Observable.fromPromise(ApplicationReader.listApplications())
.takeUntil(this.destroy$)
.subscribe(
applications => this.setState({ applications }),
err => {
// error handling
},
);
}
public componentWillUnmount(): void {
// emits on the notifier observable, completing the promise observable.
this.destroy$.next();
}
public render() {
const { applications } = this.state;
return <p>Applications: {applications.map(({ name }) => name).join(',')}</p>;
}
}