github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-23 17:28:26 -04:00
Code Issues Projects Releases Packages Wiki Activity

Style guide cleanup (#46464) (#46559)

Browse source 
* Remove angular styleguide

* Remove SASS reference only file

It's still linked in the STYLEGUIDE.md document

* Cleanup HTML styleguide

* More cleanup

* Cleanup API style guide

* Remove architecture style guide

* Merge into STYLEGUIDE.md

* add attribution back on

* Change doclink to style guide

* Change wording and remove more rules

* Update STYLEGUIDE.md

Fix typos

Co-Authored-By: Stacey Gammon <gammon@elastic.co>
This commit is contained in:
Tim Roes 2019-09-25 13:19:15 +02:00 • committed by GitHub
parent 4d5f62542e
commit 12a76df1b1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 685 additions and 1362 deletions
Download patch file Download diff file Expand all files Collapse all files

698
STYLEGUIDE.md
View file

@ -3,25 +3,695 @@
This guide applies to all development within the Kibana project and is
recommended for the development of all Kibana plugins.
- [JavaScript](style_guides/js_style_guide.md)
- [Angular](style_guides/angular_style_guide.md)
- [React](style_guides/react_style_guide.md)
- [SASS](https://elastic.github.io/eui/#/guidelines/sass)
- [HTML](style_guides/html_style_guide.md)
- [API](style_guides/api_style_guide.md)
- [Architecture](style_guides/architecture_style_guide.md)
- [Accessibility](style_guides/accessibility_guide.md)
Besides the content in this style guide, the following style guides may also apply
to all development within the Kibana project. Please make sure to also read them:
## Filenames
- [Accessibility style guide](style_guides/accessibility_guide.md)
- [SASS style guide](https://elastic.github.io/eui/#/guidelines/sass)
## General
### Filenames
All filenames should use `snake_case`.
**Right:** `src/kibana/index_patterns/index_pattern.js`
**Wrong:** `src/kibana/IndexPatterns/IndexPattern.js`
### Do not comment out code
We use a version management system. If a line of code is no longer needed,
remove it, don't simply comment it out.
### Prettier and Linting
We are gradually moving the Kibana code base over to Prettier. All TypeScript code
and some JavaScript code (check `.eslintrc.js`) is using Prettier to format code. You
can run `node script/eslint --fix` to fix linting issues and apply Prettier formatting.
We recommend you to enable running ESLint via your IDE.
Whenever possible we are trying to use Prettier and linting over written style guide rules.
Consider every linting rule and every Prettier rule to be also part of our style guide
and disable them only in exceptional cases and ideally leave a comment why they are
disabled at that specific place.
## HTML
This part contains style guide rules around general (framework agnostic) HTML usage.
### Camel case `id` and `data-test-subj`
Use camel case for the values of attributes such as `id` and `data-test-subj` selectors.
```html
<button
id="veryImportantButton"
data-test-subj="clickMeButton"
>
Click me
</button>
```
The only exception is in cases where you're dynamically creating the value, and you need to use
hyphens as delimiters:
```jsx
buttons.map(btn => (
<button
id={`veryImportantButton-${btn.id}`}
data-test-subj={`clickMeButton-${btn.id}`}
>
{btn.label}
</button>
)
```
### Capitalization in HTML and CSS should always match
It's important that when you write CSS/SASS selectors using classes, IDs, and attributes
(keeping in mind that we should _never_ use IDs and attributes in our selectors), that the
capitalization in the CSS matches that used in the HTML. HTML and CSS follow different case sensitivity rules, and we can avoid subtle gotchas by ensuring we use the
same capitalization in both of them.
## API endpoints
The following style guide rules are targeting development of server side API endpoints.
### Paths
API routes must start with the `/api/` path segment, and should be followed by the plugin id if applicable:
**Right:** `/api/marvel/nodes`
**Wrong:** `/marvel/api/nodes`
### snake_case
Kibana uses `snake_case` for the entire API, just like Elasticsearch. All urls, paths, query string parameters, values, and bodies should be `snake_case` formatted.
*Right:*
- `src/kibana/index_patterns/index_pattern.js`
```
POST /api/kibana/index_patterns
{
"id": "...",
"time_field_name": "...",
"fields": [
...
]
}
```
*Wrong:*
- `src/kibana/IndexPatterns/IndexPattern.js`
## TypeScript/JavaScript
## TypeScript vs JavaScript
The following style guide rules apply for working with TypeScript/JavaScript files.
Whenever possible, write code in TypeScript instead of javascript, especially if it's new code. Check out [TYPESCRIPT.md](TYPESCRIPT.md) for help with this process.
### TypeScript vs. JavaScript
Whenever possible, write code in TypeScript instead of JavaScript, especially if it's new code.
Check out [TYPESCRIPT.md](TYPESCRIPT.md) for help with this process.
### Prefer modern JavaScript/TypeScript syntax
You should prefer modern language features in a lot of cases, e.g.:
* Prefer `class` over `prototype` inheritance
* Prefer template strings over string concatenation
### Avoid mutability and state
Wherever possible, do not rely on mutable state. This means you should not
reassign variables, modify object properties, or push values to arrays.
Instead, create new variables, and shallow copies of objects and arrays:
```js
// good
function addBar(foos, foo) {
const newFoo = {...foo, name: 'bar'};
return [...foos, newFoo];
}
// bad
function addBar(foos, foo) {
foo.name = 'bar';
foos.push(foo);
}
```
### Return/throw early from functions
To avoid deep nesting of if-statements, always return a function's value as early
as possible. And where possible, do any assertions first:
```js
// good
function doStuff(val) {
if (val > 100) {
throw new Error('Too big');
}
if (val < 0) {
return false;
}
// ... stuff
}
// bad
function doStuff(val) {
if (val >= 0) {
if (val < 100) {
// ... stuff
} else {
throw new Error('Too big');
}
} else {
return false;
}
}
```
### Use object destructuring
This helps avoid temporary references and helps prevent typo-related bugs.
```js
// best
function fullName({ first, last }) {
return `${first} ${last}`;
}
// good
function fullName(user) {
const { first, last } = user;
return `${first} ${last}`;
}
// bad
function fullName(user) {
const first = user.first;
const last = user.last;
return `${first} ${last}`;
}
```
### Use array destructuring
Directly accessing array values via index should be avoided, but if it is
necessary, use array destructuring:
```js
const arr = [1, 2, 3];
// good
const [first, second] = arr;
// bad
const first = arr[0];
const second = arr[1];
```
### Declare one variable per line, wherever it makes the most sense
This makes it easier to re-order the lines. However, declari variables
wherever it makes sense (not necessairly on the top of a function).
```js
// good
const keys = ['foo', 'bar'];
const values = [23, 42];
// bad
const keys = ['foo', 'bar'],
values = [23, 42];
```
### Prefix private class methods with an underscore `JS only`
Identifying private class methods makes it easier to differentiate a class's public and internal
APIs, and makes private methods easier to mark as `private` when the code is migrated to TypeScript.
```js
// good
class BankAccount {
addFunds() {}
_calculateInterest() {}
}
```
If using TypeScript you should use the `private` modifier instead.
### Magic numbers/strings
These are numbers (or other values) simply used in line in your code. *Do not
use these*, give them a variable name so they can be understood and changed
easily.
```js
// good
const minWidth = 300;
if (width < minWidth) {
...
}
// bad
if (width < 300) {
...
}
```
### Modules
Module dependencies should be written using native ES2015 syntax wherever
possible (which is almost everywhere):
```js
// good
import { mapValues } from 'lodash';
export mapValues;
// bad
const _ = require('lodash');
module.exports = _.mapValues;
// worse
define(['lodash'], function (_) {
...
});
```
In those extremely rare cases where you're writing server-side JavaScript in a
file that does not pass run through webpack, then use CommonJS modules.
In those even rarer cases where you're writing client-side code that does not
run through webpack, then do not use a module loader at all.
#### Import only top-level modules
The files inside a module are implementation details of that module. They
should never be imported directly. Instead, you must only import the top-level
API that's exported by the module itself.
Without a clear mechanism in place in JS to encapsulate protected code, we make
a broad assumption that anything beyond the root of a module is an
implementation detail of that module.
On the other hand, a module should be able to import parent and sibling
modules.
```js
// good
import foo from 'foo';
import child from './child';
import parent from '../';
import ancestor from '../../../';
import sibling from '../foo';
// bad
import inFoo from 'foo/child';
import inSibling from '../foo/child';
```
### Global definitions
Don't do this. Everything should be wrapped in a module that can be depended on
by other modules. Even things as simple as a single value should be a module.
### Function definitions
Use function declarations over function expressions, so that their names will
show up in stack traces, making errors easier to debug.
Also, keep function definitions above other code instead of relying on function
hoisting.
```js
// good
function myFunc() {
...
}
// bad
const myFunc = function () {
...
};
```
### Arrow functions
If you must use a function expression, then use an arrow function:
```js
// good
[1, 2, 3].map((n) => {
const m = doSomething(n);
return m - n;
});
// bad
[1, 2, 3].map(function (n) {
const m = doSomething(n);
return m - n;
});
```
If your arrow function is only returning an object literal, then wrap the
object in parentheses rather than using an explicit return:
```js
// good
() => ({
foo: 'bar'
})
// bad
() => {
return {
foo: 'bar'
};
}
```
### Object / Array iterations, transformations and operations
Use native methods to iterate and transform arrays and objects where possible.
Avoid `for` and `while` loops as they introduce the possibility of infinite
loops and break out of our preferred convention of declarative programming.
Use descriptive variable names in the closures.
Use a utility library as needed and where it will make code more
comprehensible.
```js
// best
const userNames = users.map(user => user.name);
// ok
import { pluck } from 'lodash';
const userNames = pluck(users, 'name');
// bad
const userNames = [];
for (let i = 0; i < users.length; i++) {
userNames.push(users[i].name);
}
```
### Use the spread operator (`...`) for copying arrays
This helps with expressiveness and readability.
```js
const arr = [1, 2, 3];
// good
const arrCopy = [...arr];
// bad
const arrCopy = arr.slice();
```
### Only use ternary operators for small, simple code
And *never* use multiple ternaries together, because they make it more
difficult to reason about how different values flow through the conditions
involved. Instead, structure the logic for maximum readability.
```js
// good, a situation where only 1 ternary is needed
const foo = (a === b) ? 1 : 2;
// bad
const foo = (a === b) ? 1 : (a === c) ? 2 : 3;
```
### Do not extend built-in prototypes
Do not extend the prototype of native JavaScript objects. Your future self will
be forever grateful.
```js
// bad
Array.prototype.empty = function () {
return !this.length;
}
```
### Use descriptive conditions
Any non-trivial conditions should be converted to functions or assigned to
descriptively named variables. By breaking up logic into smaller,
self-contained blocks, it becomes easier to reason about the higher-level
logic. Additionally, these blocks become good candidates for extraction into
their own modules, with unit-tests.
```js
// best
function isShape(thing) {
return thing instanceof Shape;
}
function notSquare(thing) {
return !(thing instanceof Square);
}
if (isShape(thing) && notSquare(thing)) {
...
}
// good
const isShape = thing instanceof Shape;
const notSquare = !(thing instanceof Square);
if (isShape && notSquare) {
...
}
// bad
if (thing instanceof Shape && !(thing instanceof Square)) {
...
}
```
### Name regular expressions
```js
// good
const validPassword = /^(?=.*\d).{4,}$/;
if (password.length >= 4 && validPassword.test(password)) {
console.log('password is valid');
}
// bad
if (password.length >= 4 && /^(?=.*\d).{4,}$/.test(password)) {
console.log('losing');
}
```
### Write small functions
Keep your functions short. A good function fits on a slide that the people in
the last row of a big room can comfortably read. So don't count on them having
perfect vision and limit yourself to ~15 lines of code per function.
### Use "rest" syntax rather than built-in `arguments`
For expressiveness sake, and so you can be mix dynamic and explicit arguments.
```js
// good
function something(foo, ...args) {
...
}
// bad
function something(foo) {
const args = Array.from(arguments).slice(1);
...
}
```
### Default argument syntax
Always use the default argument syntax for optional arguments.
```js
// good
function foo(options = {}) {
...
}
// bad
function foo(options) {
if (typeof options === 'undefined') {
options = {};
}
...
}
```
And put your optional arguments at the end.
```js
// good
function foo(bar, options = {}) {
...
}
// bad
function foo(options = {}, bar) {
...
}
```
### Use thunks to create closures, where possible
For trivial examples (like the one that follows), thunks will seem like
overkill, but they encourage isolating the implementation details of a closure
from the business logic of the calling code.
```js
// good
function connectHandler(client, callback) {
return () => client.connect(callback);
}
setTimeout(connectHandler(client, afterConnect), 1000);
// not as good
setTimeout(() => {
client.connect(afterConnect);
}, 1000);
// bad
setTimeout(() => {
client.connect(() => {
...
});
}, 1000);
```
### Use slashes for comments
Use slashes for both single line and multi line comments. Try to write
comments that explain higher level mechanisms or clarify difficult
segments of your code. *Don't use comments to restate trivial things*.
*Exception:* Comment blocks describing a function and its arguments
(docblock) should start with `/**`, contain a single `*` at the beginning of
each line, and end with `*/`.
```js
// good
// 'ID_SOMETHING=VALUE' -> ['ID_SOMETHING=VALUE', 'SOMETHING', 'VALUE']
const matches = item.match(/ID_([^\n]+)=([^\n]+)/));
/**
* Fetches a user from...
* @param {string} id - id of the user
* @return {Promise}
*/
function loadUser(id) {
// This function has a nasty side effect where a failure to increment a
// redis counter used for statistics will cause an exception. This needs
// to be fixed in a later iteration.
...
}
const isSessionValid = (session.expires < Date.now());
if (isSessionValid) {
...
}
// bad
// Execute a regex
const matches = item.match(/ID_([^\n]+)=([^\n]+)/));
// Usage: loadUser(5, function() { ... })
function loadUser(id, cb) {
// ...
}
// Check if the session is valid
const isSessionValid = (session.expires < Date.now());
// If the session is valid
if (isSessionValid) {
...
}
```
### Do not alias `this`
Try not to rely on `this` at all, but if you must, then use arrow functions
instead of aliasing it.
```js
// good
class Users {
add(user) {
return createUser(user)
.then(response => this.users.push(response.user));
}
}
// bad
class Users {
add(user) {
const self = this;
return createUser(user).then(function (response) {
self.users.push(response.user);
});
}
}
```
### Getters and Setters
Feel free to use getters that are free from [side effects][sideeffect], like
providing a length property for a collection class.
Do not use setters, they cause more problems than they can solve.
[sideeffect]: http://en.wikipedia.org/wiki/Side_effect_(computer_science)
## React
The following style guide rules are specific for working with the React framework.
### Prefer reactDirective over react-component
When using `ngReact` to embed your react components inside Angular HTML, prefer the
`reactDirective` service over the `react-component` directive.
You can read more about these two ngReact methods [here](https://github.com/ngReact/ngReact#features).
Using `react-component` means adding a bunch of components into angular, while `reactDirective` keeps them isolated, and is also a more succinct syntax.
**Good:**
```html
<hello-component fname="person.fname" lname="person.lname" watch-depth="reference"></hello-component>
```
**Bad:**
```html
<react-component name="HelloComponent" props="person" watch-depth="reference" />
```
### Action function names and prop function names
Name action functions in the form of a strong verb and passed properties in the form of on<Subject><Change>. E.g:
```jsx
<sort-button onClick={action.sort}/>
<pagerButton onPageNext={action.turnToNextPage} />
```
## Attribution
Parts of the JavaScript style guide were initially forked from the
[node style guide](https://github.com/felixge/node-style-guide) created by [Felix Geisendörfer](http://felixge.de/) which is
licensed under the [CC BY-SA 3.0](http://creativecommons.org/licenses/by-sa/3.0/)
license.

2
docs/developer/pr-review.asciidoc
View file

@ -75,7 +75,7 @@ Reviewers are not simply evaluating the code itself, they are also evaluating th
Having a relatively consistent codebase is an important part of us building a sustainable project. With dozens of active contributors at any given time, we rely on automation to help ensure consistency - we enforce a comprehensive set of linting rules through CI. We're also rolling out prettier to make this even more automatic.
For things that can't be easily automated, we maintain various link:https://github.com/elastic/kibana/tree/master/style_guides[styleguides] that authors should adhere to and reviewers should keep in mind when they review a pull request.
For things that can't be easily automated, we maintain a link:https://github.com/elastic/kibana/blob/master/STYLEGUIDE.md[style guide] that authors should adhere to and reviewers should keep in mind when they review a pull request.
Beyond that, we're into subjective territory. Statements like "this isn't very readable" are hardly helpful since they can't be qualified, but that doesn't mean a reviewer should outright ignore code that is hard to understand due to how it is written. There isn't one definitively "best" way to write any particular code, so pursuing such shouldn't be our goal. Instead, reviewers and authors alike must accept that there are likely many different appropriate ways to accomplish the same thing with code, and so long as the contribution is utilizing one of those ways, then we're in good shape.

65
style_guides/angular_style_guide.md
View file

@ -1,65 +0,0 @@
# Angular Style Guide
Kibana is written in Angular, and uses several utility methods to make using
Angular easier.
## Defining modules
Angular modules are defined using a custom require module named `ui/modules`.
It is used as follows:
```js
const app = require('ui/modules').get('app/namespace');
```
`app` above is a reference to an Angular module, and can be used to define
controllers, providers and anything else used in Angular. While you can use
this module to create/get any module with ui/modules, we generally use the
"kibana" module for everything.
## Promises
A more robust version of Angular's `$q` service is available as `Promise`. It
can be used in the same way as `$q`, but it comes packaged with several utility
methods that provide many of the same useful utilities as Bluebird.
```js
app.service('CustomService', (Promise, someFunc) => {
new Promise((resolve, reject) => {
...
});
const promisedFunc = Promise.cast(someFunc);
return Promise.resolve('value');
});
```
### Routes
Angular routes are defined using a custom require module named `routes` that
removes much of the required boilerplate.
```js
import routes from 'ui/routes';
routes.when('/my/object/route/:id?', {
// angular route code goes here
});
```
## Private modules
A service called `Private` is available to load any function as an angular
module without needing to define it as such. It is used as follows:
```js
import PrivateExternalClass from 'path/to/some/class';
app.controller('myController', function($scope, otherDeps, Private) {
const ExternalClass = Private(PrivateExternalClass);
...
});
```
**Note:** Use this sparingly. Whenever possible, your modules should not be
coupled to the angular application itself.

32
style_guides/api_style_guide.md
View file

@ -1,32 +0,0 @@
# API Style Guide
## Paths
API routes must start with the `/api/` path segment, and should be followed by the plugin id if applicable:
*Right:* `/api/marvel/nodes`
*Wrong:* `/marvel/api/nodes`
## Versions
Kibana won't be supporting multiple API versions, so API's should not define a version.
*Right:* `/api/kibana/index_patterns`
*Wrong:* `/api/kibana/v1/index_patterns`
## snake_case
Kibana uses `snake_case` for the entire API, just like Elasticsearch. All urls, paths, query string parameters, values, and bodies should be `snake_case` formatted.
*Right:*
```
POST /api/kibana/index_patterns
{
"id": "...",
"time_field_name": "...",
"fields": [
...
]
}
```

70
style_guides/architecture_style_guide.md
View file

@ -1,70 +0,0 @@
# Architecture Style Guide
## Plugin Architecture
These are a collection of architectural styles to follow when building Kibana plugins. This includes applications meant to be accessed from the Kibana sidebar, as applications are simply one type of plugin.
### File and Folder Structure
Kibana plugins, both in core and those developed independent of Kibana, should follow this basic file and folder structure:
```
plugin_root:
.
├── common/
├── public/
├── server/
└── index.js
```
<dl>
<dt>index.js</dt>
<dd>The entry point to your application, this file is loaded when your code is being used in Kibana. This module should simply export a function that takes the <code>kibana</code> server object and initializes your application. This is where you define things like the <em>id</em>, any <em>uiExports</em>, dependencies on other plugins and applications, any configuration, any initialization code, and the location of the <em>public</em> folder.</dd>
<dt>public</dt>
<dd>This folder is where you place client-side code for your application. Anything that is run in the browser belongs in here.</dd>
<dd><strong>NOTE</strong>: An alias for this folder is created at <code>plugins/{id}</code>, where <code>id</code> is what you defined in the plugin entry point. If your application's <code>id</code> is <code>utile</code>, and you were trying to import a module from <code>public/lib/formatter.js</code>, you could import the module as <code>plugins/utile/lib/formatter</code>.
</dd>
<dt>server</dt>
<dd>This folder is where server code belongs. Things like custom routes, data models, or any other code that should only be executed on the server should go here.</dd>
<dt>common</dt>
<dd>This folder is where code that is useful on both the client and the server belongs. A consistent example of this is constants, but this could apply to helper modules as well.</dd>
<dd><strong>NOTE</strong>: If you'd like to avoid adding <code>../common</code> to your public code, you could use <em>webpackShims</em> to resolve the path without traversing backwards.</dd>
</dl>
## Subdirectories
As code gets more complex, it becomes important to organize it into subdirectories. Each subdirectory should contain an `index.js` file that exposes the contents of that directory.
```
plugin_root:
.
├── common/
├── public/
├───── component_one/
├──────── component_one.js
├──────── component_one.html
├──────── component_one_helper.js
├──────── index.js
├───── index.js
├── server/
└── index.js
```
```
public/component_one/index.js:
import './component_one';
```
```
public/index.js (consumer of component_one):
import './component_one';
```
NOTE: There is currently a Webpack plugin that allows import statements to resolve in multiple ways. The statement `import './component_one'` in the `public/index.js` file above would successfully resolve to both `/public/component_one/component_one.js` and `/public/component_one/index.js`. If there is both a named file and an `index.js` file, Webpack will resolve to the `index.js` file. This functionality will be removed in the future, and when that happens, Webpack will only resolve to the `index.js` file.

84
style_guides/html_style_guide.md
View file

@ -1,84 +0,0 @@
# HTML Style Guide
## Camel case ID and other attribute values
Use camel case for the values of attributes such as IDs and data test subject selectors.
```html
<button
id="veryImportantButton"
data-test-subj="clickMeButton"
>
Click me
</button>
```
The only exception is in cases where you're dynamically creating the value, and you need to use
hyphens as delimiters:
```html
<button
ng-repeat="button in buttons"
id="{{ 'veryImportantButton-' + button.id }}"
data-test-subj="{{ 'clickMeButton-' + button.id }}"
>
{{ button.label }}
</button>
```
## Capitalization in HTML and CSS should always match
It's important that when you write CSS selectors using classes, IDs, and attributes
(keeping in mind that we should _never_ use IDs and attributes in our selectors), that the
capitalization in the CSS matches that used in the HTML. [HTML and CSS follow different case sensitivity rules](http://reference.sitepoint.com/css/casesensitivity), and we can avoid subtle gotchas by ensuring we use the
same capitalization in both of them.
## Multiple attribute values
When an element has multiple attributes, each attribute including the first should be on its own line with a single indent.
This makes the attributes easier to scan and compare across similar elements.
The closing bracket should be on its own line. This allows attributes to be shuffled and edited without having to move the bracket around. It also makes it easier to scan vertically and match opening and closing brackets. This format
is inspired by the positioning of the opening and closing parentheses in [Pug/Jade](https://pugjs.org/language/attributes.html#multiline-attributes).
```html
<div
attribute1="value1"
attribute2="value2"
attribute3="value3"
>
Hello
</div>
```
If the element doesn't have children, add the closing tag on the same line as the opening tag's closing bracket.
```html
<div
attribute1="value1"
attribute2="value2"
attribute3="value3"
></div>
```
## Nested elements belong on multiple lines
Putting nested elements on multiple lines makes it easy to scan and identify tags, attributes, and text
nodes, and to distinguish elements from one another. This is especially useful if there are multiple
similar elements which appear sequentially in the markup.
### Do
```html
<div>
<span>
hi
</span>
</div>
```
### Don't
```html
<div><span>hi</span></div>
```

873
style_guides/js_style_guide.md
View file

@ -1,873 +0,0 @@
# JavaScript Style Guide
## Attribution
This JavaScript guide forked from the [node style guide](https://github.com/felixge/node-style-guide) created by [Felix Geisendörfer](http://felixge.de/) and is
licensed under the [CC BY-SA 3.0](http://creativecommons.org/licenses/by-sa/3.0/)
license.
## Prettier
We are gradually moving the Kibana code base over to Prettier.
Prettier is set up to run with ESLint, and to add new code paths to Prettier,
see `.eslintrc.js` in the root of the Kibana repo. This also means that if you
get ESLint errors related to Prettier, run `node scripts/eslint --fix` from the
root of the Kibana repo to fix these.
## 2 Spaces for indention
Use 2 spaces for indenting your code and swear an oath to never mix tabs and
spaces - a special kind of hell is awaiting you otherwise.
## Newlines
Use UNIX-style newlines (`\n`), and a newline character as the last character
of a file. Windows-style newlines (`\r\n`) are forbidden inside any repository.
## No trailing whitespace
Just like you brush your teeth after every meal, you clean up any trailing
whitespace in your JS files before committing. Otherwise the rotten smell of
careless neglect will eventually drive away contributors and/or co-workers.
## Use Semicolons
According to [scientific research][hnsemicolons], the usage of semicolons is
a core value of our community. Consider the points of [the opposition][], but
be a traditionalist when it comes to abusing error correction mechanisms for
cheap syntactic pleasures.
[the opposition]: http://blog.izs.me/post/2353458699/an-open-letter-to-javascript-leaders-regarding
[hnsemicolons]: http://news.ycombinator.com/item?id=1547647
## 100 characters per line
You should limit your lines to 100 chars. Prettier will check for that line width where enabled.
## Use `const` for variables
Your variable references should rarely be mutable, so use `const` for almost
everything. If you absolutely *must* mutate a reference, use `let`.
```js
// good
const foo = 'bar';
// if absolutely necessary, OK
let foo;
// bad
var foo = 'bar';
```
## Use single quotes for fixed strings
Use single quotes, unless you are writing JSON.
```js
// good
const foo = 'bar';
// bad
const foo = "bar";
```
## Use template strings to interpolate variables into strings
```js
// good
const foo = `Hello, ${name}`;
// bad
const foo = 'Hello, ' + name;
```
## Use template strings to avoid escaping single quotes
Because readability is paramount.
```js
// good
const foo = `You won't believe this.`;
// bad
const foo = 'You won\'t believe this.';
const foo = "You won't believe this.";
```
## Use object destructuring
This helps avoid temporary references and helps prevent typo-related bugs.
```js
// best
function fullName({ first, last }) {
return `${first} ${last}`;
}
// good
function fullName(user) {
const { first, last } = user;
return `${first} ${last}`;
}
// bad
function fullName(user) {
const first = user.first;
const last = user.last;
return `${first} ${last}`;
}
```
## Use array destructuring
Directly accessing array values via index should be avoided, but if it is
necessary, use array destructuring:
```js
const arr = [1, 2, 3];
// good
const [first, second] = arr;
// bad
const first = arr[0];
const second = arr[1];
```
## Opening braces go on the same line
Your opening braces go on the same line as the statement.
```js
// good
if (true) {
console.log('winning');
}
// bad
if (true)
{
console.log('losing');
}
```
Also, notice the use of whitespace before and after the condition statement.
## Always use braces for conditionals and loops
```js
// good
if (err) {
return cb(err);
}
// bad
if (err) cb(err);
// bad
if (err)
return cb(err);
```
## Declare one variable per line, wherever it makes the most sense
This makes it easier to re-order the lines. However, ignore
[Crockford][crockfordconvention] when it comes to declaring variables deeper
inside a function, just put the declarations wherever they make sense.
```js
// good
const keys = ['foo', 'bar'];
const values = [23, 42];
// bad
const keys = ['foo', 'bar'],
values = [23, 42];
```
[crockfordconvention]: http://javascript.crockford.com/code.html
## Use lowerCamelCase for variables, properties and function names
Variables, properties and function names should use `lowerCamelCase`. They
should also be descriptive. Single character variables and uncommon
abbreviations should generally be avoided.
```js
// good
const adminUser = getAdmin();
// bad
const admin_user = getAdmin();
```
## Use UpperCamelCase for class names (constructors)
Class names should be capitalized using `UpperCamelCase`.
```js
// good
class BankAccount {}
// bad
class bank_account {}
class bankAccount {}
```
## Prefix private class methods with an underscore
Identifying private class methods makes it easier to differentiate a class's public and internal
APIs, and makes private methods easier to mark as `private` when the code is migrated to TypeScript.
```js
// good
class BankAccount {
addFunds() {}
_calculateInterest() {}
}
```
## Magic numbers/strings
These are numbers (or other values) simply used in line in your code. *Do not
use these*, give them a variable name so they can be understood and changed
easily.
```js
// good
const minWidth = 300;
if (width < minWidth) {
...
}
// bad
if (width < 300) {
...
}
```
## Object properties and functions
Use object method shorthand syntax for functions on objects:
```js
// good
const foo = {
bar() {
...
}
};
// bad
const foo = {
bar: function () {
...
}
};
```
Use property value shorthand syntax for properties that share a name with a
variable. And put them at the beginning:
```js
const bar = true;
// good
const foo = {
bar
};
// bad
const foo = {
bar: bar
};
// also bad (bar should be first)
const foo = {
baz: false,
bar
};
```
## Modules
Module dependencies should be written using native ES2015 syntax wherever
possible (which is almost everywhere):
```js
// good
import { mapValues } from 'lodash';
export mapValues;
// bad
const _ = require('lodash');
module.exports = _.mapValues;
// worse
define(['lodash'], function (_) {
...
});
```
In those extremely rare cases where you're writing server-side JavaScript in a
file that does not pass run through webpack, then use CommonJS modules.
In those even rarer cases where you're writing client-side code that does not
run through webpack, then do not use a module loader at all.
## Import only top-level modules
The files inside a module are implementation details of that module. They
should never be imported directly. Instead, you must only import the top-level
API that's exported by the module itself.
Without a clear mechanism in place in JS to encapsulate protected code, we make
a broad assumption that anything beyond the root of a module is an
implementation detail of that module.
On the other hand, a module should be able to import parent and sibling
modules.
```js
// good
import foo from 'foo';
import child from './child';
import parent from '../';
import ancestor from '../../../';
import sibling from '../foo';
// bad
import inFoo from 'foo/child';
import inSibling from '../foo/child';
```
## Use named exports only
Favor named exports over default exports. See [#8641](https://github.com/elastic/kibana/issues/8641) for more background on this decision.
```js
// good
import { foo } from 'foo';
export foo;
// bad
import myDefaultModule from 'foo/child';
export default myDefaultModule;
```
## Global definitions
Don't do this. Everything should be wrapped in a module that can be depended on
by other modules. Even things as simple as a single value should be a module.
## Function definitions
Use function declarations over function expressions, so that their names will
show up in stack traces, making errors easier to debug.
Also, keep function definitions above other code instead of relying on function
hoisting.
```js
// good
function myFunc() {
...
}
// bad
const myFunc = function () {
...
};
```
## Arrow functions
If you must use a function expression, then use an arrow function:
```js
// good
[1, 2, 3].map((n) => {
const m = doSomething(n);
return m - n;
});
// bad
[1, 2, 3].map(function (n) {
const m = doSomething(n);
return m - n;
});
```
If your function body does not include braces and only accepts one argument,
then omit the argument parentheses:
```js
// good
[1, 2, 3].map(n => n + 1);
// bad
[1, 2, 3].map((n) => n + 1);
// bad
[1, 2, 3].map(n => {
return n + 1;
});
```
If your arrow function is only returning an object literal, then wrap the
object in parentheses rather than using an explicit return:
```js
// good
() => ({
foo: 'bar'
})
// bad
() => {
return {
foo: 'bar'
};
}
```
## Object / Array creation
Use trailing commas and put *short* declarations on a single line. Only quote
keys when your interpreter complains:
```js
// good
const a = ['hello', 'world'];
const b = {
good: 'code',
'is generally': 'pretty'
};
// bad
const a = [
'hello', 'world'
];
const b = {'good': 'code'
, is generally: 'pretty'
};
```
## Object / Array iterations, transformations and operations
Use native methods to iterate and transform arrays and objects where possible.
Avoid `for` and `while` loops as they introduce the possibility of infinite
loops and break out of our preferred convention of declarative programming.
Use descriptive variable names in the closures.
Use a utility library as needed and where it will make code more
comprehensible.
```js
// best
const userNames = users.map(user => user.name);
// ok
import { pluck } from 'lodash';
const userNames = pluck(users, 'name');
// bad
const userNames = [];
for (let i = 0; i < users.length; i++) {
userNames.push(users[i].name);
}
```
## Use the spread operator (...) for copying arrays
This helps with expressiveness and readability.
```js
const arr = [1, 2, 3];
// good
const arrCopy = [...arr];
// bad
const arrCopy = arr.slice();
```
## Use the === operator
Programming is not about remembering [stupid rules][comparisonoperators]. Use
the triple equality operator as it will work just as expected.
```js
const a = 0;
// good
if (a !== '') {
console.log('winning');
}
// bad
if (a == '') {
console.log('losing');
}
```
[comparisonoperators]: https://developer.mozilla.org/en/JavaScript/Reference/Operators/Comparison_Operators
## Only use ternary operators for small, simple code
And *never* use multiple ternaries together, because they make it more
difficult to reason about how different values flow through the conditions
involved. Instead, structure the logic for maximum readability.
```js
// good, a situation where only 1 ternary is needed
const foo = (a === b) ? 1 : 2;
// bad
const foo = (a === b) ? 1 : (a === c) ? 2 : 3;
```
## Do not extend built-in prototypes
Do not extend the prototype of native JavaScript objects. Your future self will
be forever grateful.
```js
// bad
Array.prototype.empty = function () {
return !this.length;
}
```
## Use descriptive conditions
Any non-trivial conditions should be converted to functions or assigned to
descriptively named variables. By breaking up logic into smaller,
self-contained blocks, it becomes easier to reason about the higher-level
logic. Additionally, these blocks become good candidates for extraction into
their own modules, with unit-tests.
```js
// best
function isShape(thing) {
return thing instanceof Shape;
}
function notSquare(thing) {
return !(thing instanceof Square);
}
if (isShape(thing) && notSquare(thing)) {
...
}
// good
const isShape = thing instanceof Shape;
const notSquare = !(thing instanceof Square);
if (isShape && notSquare) {
...
}
// bad
if (thing instanceof Shape && !(thing instanceof Square)) {
...
}
```
## Name regular expressions
```js
// good
const validPassword = /^(?=.*\d).{4,}$/;
if (password.length >= 4 && validPassword.test(password)) {
console.log('password is valid');
}
// bad
if (password.length >= 4 && /^(?=.*\d).{4,}$/.test(password)) {
console.log('losing');
}
```
## Write small functions
Keep your functions short. A good function fits on a slide that the people in
the last row of a big room can comfortably read. So don't count on them having
perfect vision and limit yourself to ~15 lines of code per function.
## Use "rest" syntax rather than built-in `arguments`
For expressiveness sake, and so you can be mix dynamic and explicit arguments.
```js
// good
function something(foo, ...args) {
...
}
// bad
function something(foo) {
const args = Array.from(arguments).slice(1);
...
}
```
## Default argument syntax
Always use the default argument syntax for optional arguments.
```js
// good
function foo(options = {}) {
...
}
// bad
function foo(options) {
if (typeof options === 'undefined') {
options = {};
}
...
}
```
And put your optional arguments at the end.
```js
// good
function foo(bar, options = {}) {
...
}
// bad
function foo(options = {}, bar) {
...
}
```
## Return/throw early from functions
To avoid deep nesting of if-statements, always return a function's value as early
as possible. And where possible, do any assertions first:
```js
// good
function doStuff(val) {
if (val > 100) {
throw new Error('Too big');
}
if (val < 0) {
return false;
}
// ... stuff
}
// bad
function doStuff(val) {
if (val >= 0) {
if (val < 100) {
// ... stuff
} else {
throw new Error('Too big');
}
} else {
return false;
}
}
```
## Chaining operations
When using a chaining syntax, indent the subsequent chained operations.
Also, if the chain is long, each method should be on a new line.
```js
// good
$http.get('/info')
.then(({ data }) => this.transformInfo(data))
.then((transformed) => $http.post('/new-info', transformed))
.then(({ data }) => console.log(data));
// bad
$http.get('/info')
.then(({ data }) => this.transformInfo(data))
.then((transformed) => $http.post('/new-info', transformed))
.then(({ data }) => console.log(data));
```
## Avoid mutability and state
Wherever possible, do not rely on mutable state. This means you should not
reassign variables, modify object properties, or push values to arrays.
Instead, create new variables, and shallow copies of objects and arrays:
```js
// good
function addBar(foos, foo) {
const newFoo = {...foo, name: 'bar'};
return [...foos, newFoo];
}
// bad
function addBar(foos, foo) {
foo.name = 'bar';
foos.push(foo);
}
```
## Use thunks to create closures, where possible
For trivial examples (like the one that follows), thunks will seem like
overkill, but they encourage isolating the implementation details of a closure
from the business logic of the calling code.
```js
// good
function connectHandler(client, callback) {
return () => client.connect(callback);
}
setTimeout(connectHandler(client, afterConnect), 1000);
// not as good
setTimeout(() => {
client.connect(afterConnect);
}, 1000);
// bad
setTimeout(() => {
client.connect(() => {
...
});
}, 1000);
```
## Use slashes for comments
Use slashes for both single line and multi line comments. Try to write
comments that explain higher level mechanisms or clarify difficult
segments of your code. *Don't use comments to restate trivial things*.
*Exception:* Comment blocks describing a function and its arguments
(docblock) should start with `/**`, contain a single `*` at the beginning of
each line, and end with `*/`.
```js
// good
// 'ID_SOMETHING=VALUE' -> ['ID_SOMETHING=VALUE', 'SOMETHING', 'VALUE']
const matches = item.match(/ID_([^\n]+)=([^\n]+)/));
/**
* Fetches a user from...
* @param {string} id - id of the user
* @return {Promise}
*/
function loadUser(id) {
// This function has a nasty side effect where a failure to increment a
// redis counter used for statistics will cause an exception. This needs
// to be fixed in a later iteration.
...
}
const isSessionValid = (session.expires < Date.now());
if (isSessionValid) {
...
}
// bad
// Execute a regex
const matches = item.match(/ID_([^\n]+)=([^\n]+)/));
// Usage: loadUser(5, function() { ... })
function loadUser(id, cb) {
// ...
}
// Check if the session is valid
const isSessionValid = (session.expires < Date.now());
// If the session is valid
if (isSessionValid) {
...
}
```
## Do not comment out code
We use a version management system. If a line of code is no longer needed,
remove it, don't simply comment it out.
## Classes/Constructors and Inheritance
If you must use a constructor, then use the native `class` syntax. *Never* use
third party "class" utilities, and never mutate prototypes.
```js
// best (no local state at all)
function addUser(users, user) {
return [...users, user];
}
const users = addUser([], { name: 'foo' });
// good
class Users {
add(user) {
...
}
}
const users = new Users();
users.add({ name: 'foo' });
// bad
function Users() {
...
}
Users.prototype.add = function () {
...
};
const users = new Users();
users.add({ name: 'foo' });
```
## Do not alias `this`
Try not to rely on `this` at all, but if you must, then use arrow functions
instead of aliasing it.
```js
// good
class Users {
add(user) {
return createUser(user)
.then(response => this.users.push(response.user));
}
}
// bad
class Users {
add(user) {
const self = this;
return createUser(user).then(function (response) {
self.users.push(response.user);
});
}
}
```
## Getters and Setters
Feel free to use getters that are free from [side effects][sideeffect], like
providing a length property for a collection class.
Do not use setters, they cause more problems than they can solve.
[sideeffect]: http://en.wikipedia.org/wiki/Side_effect_(computer_science)

219
style_guides/react_style_guide.md
View file

@ -1,219 +0,0 @@
# React Style Guide
### Prefer Stateless functional components where possible.
Stateless function components are more concise, and there are plans for react to increase performance of them.
Good:
```
export function KuiButton(props) {
return <button className="kuiButton" {...props} />
};
```
Bad:
```
export class KuiButton extends React.Component {
render() {
return <button className="kuiButton" {...this.props} />
}
}
```
### When state is involved, use ES6 style React Classes over ES5.
Good:
```
export class ClickCounter extends React.Component {
state = { clickCount: 0 };
onClick = () => {
this.setState(prevState => ({
clickCount: prevState.clickCount + 1
}));
}
render() {
return <button className="kuiButton" onClick={this.onClick} />
}
}
```
Bad:
```
export const ClickCounter = React.createClass({
getInitialState() {
return {
clickCount: 0
};
},
onClick() {
this.setState(prevState => ({
clickCount: prevState.clickCount + 1
}));
},
render() {
return <button className="kuiButton" onClick={this.onClick} />
}
});
```
### When a state change involves the previous state or props, pass setState a function instead of an object.
https://facebook.github.io/react/docs/react-component.html#setstate
Good:
```
this.setState((prevState, props) => ({
clickCount: prevState.clickCount + props.incrementValue
}));
```
Bad:
```
this.setState({ clickCount: this.state.clickCount + this.props.incrementValue });
```
Because this.props and this.state may be updated asynchronously, you should not rely on their values for calculating the next state.
- https://facebook.github.io/react/docs/state-and-lifecycle.html#state-updates-may-be-asynchronous
This will be even more important when the fibers-based implementation is released:
- https://github.com/acdlite/react-fiber-architecture
- https://www.youtube.com/watch?v=ZCuYPiUIONs
### Prefer reactDirective over react-component
When using ngReact to embed your react components inside angular html, prefer
reactDirective over react-component. You can read more about these two ngReact methods [here](https://github.com/ngReact/ngReact#features).
Using `react-component` means adding a bunch of components into angular, while `reactDirective` keeps them isolated,
and is also a more succinct syntax.
Good:
```
<hello-component fname="person.fname" lname="person.lname" watch-depth="reference"></hello-component>
```
Bad:
```
<react-component name="HelloComponent" props="person" watch-depth="reference" />
```
### Prefix ui_framework elements with kui, but not their file names.
Good:
```
button.js:
export function KuiButton(props) {
return <button className="kuiButton" {...props} />
};
```
Bad:
```
button.js:
export function Button(props) {
return <button className="kuiButton" {...props} />
};
```
The filenames leave it off because snake casing already increases file name length.
### Action function names and prop function names
Name action functions in the form of a strong verb and passed properties in the form of on<Subject><Change>. E.g:
```
<sort-button onClick={action.sort}/>
<pagerButton onPageNext={action.turnToNextPage} />
```
### Avoid creating a function and passing that as a property, in render functions.
Best (relies on [stage 2 proposal](https://github.com/tc39/proposal-class-public-fields)):
```
export class ClickCounter extends React.Component {
state = { clickCount: 0 };
// This syntax ensures `this` is bound within handleClick
onClick = () => {
this.setState(prevState => { clickCount: prevState.clickCount + 1 });
}
render() {
return <button className="kuiButton" onClick={this.onClick} />
}
}
```
Good:
```
export class ClickCounter extends React.Component {
constructor() {
this.state = { clickCount: 0 };
this.onClick = this.onClick.bind(this);
}
onClick() {
this.setState(prevState => { clickCount: prevState.clickCount + 1 });
}
render() {
return <button className="kuiButton" onClick={this.onClick} />
}
}
```
Bad:
```
export class ClickCounter extends React.Component {
state = { clickCount: 0 };
onClick() {
this.setState(prevState => { clickCount: prevState.clickCount + 1 });
}
render() {
return <button className="kuiButton" onClick={() => this.onClick()} />
}
}
```
Also Bad:
```
render() {
return <button className="kuiButton" onClick={this.onClick.bind(this)} />
}
```
Background: https://facebook.github.io/react/docs/handling-events.html
There is also an eslint rule we should be able to turn on for this.
### Never mutate state directly
Good:
```
this.setState(prevState => { clickCount: prevState.clickCount + 1 });
```
Bad:
```
this.state.clickCount += 1;
```
### Prefer primitives over objects when storing in state.
Good:
```
this.setState({
currentPage: 0,
selectedIds: []
});
```
Discouraged:
```
this.setState({
pager: new Pager(),
selectedIds: new SelectedIds()
});
```
### Favor spread operators
```
render() {
return <button className="kuiButton" {...this.props} />
}
```
```
export function Button({ className, ...rest }) {
const classNames = classNames('KuiButton', className);
return <button className={classNames} {...rest} />
};
```
## General Guidelines
### Prefer pure functions when possible
Pure functions are easier to understand. We don't want to have to think about side effects or mutated state. When invoking a pure function, all we have to think about is what goes in and what comes out.

4
style_guides/scss_style_guide.md
View file

@ -1,4 +0,0 @@
# SCSS Style Guide
Our style guide is maintained in [EUI Sass Guidelines](https://elastic.github.io/eui/#/guidelines/sass).