From a14d0631390f1142d142518bc79d6300dfed1e17 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Wed, 2 Oct 2019 16:09:35 +0200 Subject: [PATCH 1/2] Add TypeScript rules to STYLEGUIDE --- STYLEGUIDE.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/STYLEGUIDE.md b/STYLEGUIDE.md index 152a0e2c48871..261bca10fd720 100644 --- a/STYLEGUIDE.md +++ b/STYLEGUIDE.md @@ -141,6 +141,39 @@ function addBar(foos, foo) { } ``` +### Avoid `any` whenever possible + +Since TypeScript 3.0 and the introduction of the +[`unknown` type](https://mariusschulz.com/blog/the-unknown-type-in-typescript) there are rarely any +reasons to use `any` as a type. Nearly all places of former `any` usage can be replace by either a +generic or `unknown` (in cases the type is really not known). + +You should always prefer using those mechanisms over using `any`, since they are stricter typed and +less likely to introduce bugs in the future due to insufficient types. + +If you’re not having `any` in your plugin or are starting a new plugin, you should enable the +[`@typescript-eslint/no-explicit-any`](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-explicit-any.md) +linting rule for your plugin via the [`.eslintrc.js`](https://github.com/elastic/kibana/blob/master/.eslintrc.js) config. + +### Avoid non-null assertions + +You should try avoiding non-null assertions (`!.`) wherever possible. By using them you tell +TypeScript, that something is not null even though by it’s type it could be. Usage of non-null +assertions is most often a side-effect of you actually checked that the variable is not null `null` +but TypeScript doesn’t correctly carry on that information till the usage of the variable. + +In most cases it’s possible to replace the non-null assertion by structuring your code/checks slightly different +or using [user defined type guards](https://www.typescriptlang.org/docs/handbook/advanced-types.html#user-defined-type-guards) +to properly tell TypeScript what type a variable has. + +Using non-null assertion increases the risk for future bugs. In case the condition under which we assumed that the +variable can’t be null has changed (potentially even due to changes in compeltely different files), the non-null +assertion would now wrongly disable proper type checking for us. + +If you’re not using non-null assertions in your plugin or are starting a new plugin, consider enabling the +[`@typescript-eslint/no-non-null-assertion`](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-non-null-assertion.md) +linting rule for you plugin in the [`.eslintrc.js`](https://github.com/elastic/kibana/blob/master/.eslintrc.js) config. + ### Return/throw early from functions To avoid deep nesting of if-statements, always return a function's value as early From 0ed7bf95dc8eab23aa3df2888d897745c7e9b6d2 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Thu, 3 Oct 2019 09:43:10 +0200 Subject: [PATCH 2/2] Update STYLEGUIDE.md Co-Authored-By: Court Ewing --- STYLEGUIDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/STYLEGUIDE.md b/STYLEGUIDE.md index 261bca10fd720..5fd3ef5e8ff4b 100644 --- a/STYLEGUIDE.md +++ b/STYLEGUIDE.md @@ -159,7 +159,7 @@ linting rule for your plugin via the [`.eslintrc.js`](https://github.com/elastic You should try avoiding non-null assertions (`!.`) wherever possible. By using them you tell TypeScript, that something is not null even though by it’s type it could be. Usage of non-null -assertions is most often a side-effect of you actually checked that the variable is not null `null` +assertions is most often a side-effect of you actually checked that the variable is not `null` but TypeScript doesn’t correctly carry on that information till the usage of the variable. In most cases it’s possible to replace the non-null assertion by structuring your code/checks slightly different