Skip to content

Commit

Permalink
Fix small typos in the root md files (elastic#134609)
Browse files Browse the repository at this point in the history
* FAQ.md
* STYLEGUIDE.mdx
* TYPESCRIPT.md
  • Loading branch information
PhilippeOberti authored Jun 23, 2022
1 parent 761850e commit 8638f12
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 37 deletions.
2 changes: 1 addition & 1 deletion FAQ.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
**Q:** Where do I go for support?
**A:** Please join us at [discuss.elastic.co](https://discuss.elastic.co) with questions. Your problem might be a bug, but it might just be a misunderstanding, or a feature we could improve. We're also available on Freenode in #kibana

**Q:** Ok, we talked about it and its definitely a bug
**Q:** Ok, we talked about it, and it's definitely a bug
**A:** Doh, ok, let's get that fixed. File an issue on [github.com/elastic/kibana](https://github.com/elastic/kibana). I'd recommend reading the beginning of the CONTRIBUTING.md, just so you know how we'll handle the issue.

### Kibana 3 Migration
Expand Down
56 changes: 32 additions & 24 deletions STYLEGUIDE.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,18 @@ and some JavaScript code (check `.eslintrc.js`) is using Prettier to format code
can run `node scripts/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.
Whenever possible we are trying to use Prettier and linting, instead of maintaining a set of 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.
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.
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>
Expand All @@ -73,8 +73,8 @@ buttons.map(btn => (
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.
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.
### How to generate ids?
Expand Down Expand Up @@ -143,7 +143,8 @@ API routes must start with the `/api/` path segment, and should be followed by t
### 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.
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:_
Expand Down Expand Up @@ -202,7 +203,7 @@ function addBar(foos, foo) {
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
reasons to use `any` as a type. Nearly all places of former `any` usage can be replaced 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
Expand All @@ -215,16 +216,16 @@ linting rule for your plugin via the [`.eslintrc.js`](https://github.com/elastic
### 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`
but TypeScript doesnt correctly carry on that information till the usage of the variable.
TypeScript that something is not `null`, even though by its 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`
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
variable can’t be `null` has changed (potentially even due to changes in completely 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
Expand Down Expand Up @@ -266,7 +267,7 @@ function doStuff(val) {
### Use object destructuring
This helps avoid temporary references and helps prevent typo-related bugs.
This helps avoid temporary references and helps prevent typo-related bugs:
```js
// best
Expand Down Expand Up @@ -307,8 +308,8 @@ const second = arr[1];
### 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.
use these_, give them a variable name, so they can be understood and changed
easily:
```js
// good
Expand Down Expand Up @@ -378,7 +379,9 @@ import inSibling from '../foo/child';
#### Avoid export \* in top level index.ts files
The exports in `common/index.ts`, `public/index.ts` and `server/index.ts` dictate a plugin's public API. The public API should be carefully controlled, and using `export *` makes it very easy for a developer working on internal changes to export a new public API unintentionally.
The exports in `common/index.ts`, `public/index.ts` and `server/index.ts` dictate a plugin's public API.
The public API should be carefully controlled, and using `export *` makes it very easy for a developer
working on internal changes to export a new public API unintentionally:
```js
// good
Expand All @@ -399,7 +402,7 @@ by other modules. Even things as simple as a single value should be a module.
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.
involved. Instead, structure the logic for maximum readability:
```js
// good, a situation where only 1 ternary is needed
Expand Down Expand Up @@ -466,7 +469,7 @@ 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.
For the sake of expressiveness, and so you can be mix dynamic and explicit arguments:
```js
// good
Expand All @@ -483,7 +486,7 @@ function something(foo) {
### Default argument syntax
Always use the default argument syntax for optional arguments.
Always use the default argument syntax for optional arguments:
```js
// good
Expand All @@ -500,7 +503,7 @@ function foo(options) {
}
```
And put your optional arguments at the end.
And put your optional arguments at the end:
```js
// good
Expand All @@ -518,7 +521,7 @@ function foo(options = {}, bar) {
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.
from the business logic of the calling code:
```js
// good
Expand Down Expand Up @@ -621,9 +624,13 @@ by your code until the circular dependencies on these have been solved.
## SASS files
When writing a new component, create a sibling SASS file of the same name and import directly into the **top** of the JS/TS component file. Doing so ensures the styles are never separated or lost on import and allows for better modularization (smaller individual plugin asset footprint).
When writing a new component, create a sibling SASS file of the same name and import
directly into the **top** of the JS/TS component file.
Doing so ensures the styles are never separated or lost on import and allows
for better modularization (smaller individual plugin asset footprint).
All SASS (.scss) files will automatically build with the [EUI](https://elastic.github.io/eui/#/guidelines/sass) & Kibana invisibles (SASS variables, mixins, functions) from the [`globals_[theme].scss` file](src/core/public/core_app/styles/_globals_v8light.scss).
All SASS (.scss) files will automatically build with the [EUI](https://elastic.github.io/eui/#/guidelines/sass)
& Kibana invisibles (SASS variables, mixins, functions) from the [`globals_[theme].scss` file](src/core/public/core_app/styles/_globals_v8light.scss).
While the styles for this component will only be loaded if the component exists on the page,
the styles **will** be global and so it is recommended to use a three letter prefix on your
Expand Down Expand Up @@ -656,11 +663,12 @@ The following style guide rules are specific for working with the React framewor
### Prefer reactDirective over react-component
When using `ngReact` to embed your react components inside Angular HTML, prefer the
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.
Using `react-component` means adding a bunch of components into angular, while `reactDirective` keeps them isolated,
and is also a more succinct syntax:
**Good:**
Expand Down
24 changes: 12 additions & 12 deletions TYPESCRIPT.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

To convert existing code over to TypeScript:
1. rename the file from `.js` to either `.ts` (if there is no html or jsx in the file) or `.tsx` (if there is).
2. Ensure eslint is running and installed in the IDE of your choice. There will usually be some linter errors after the file rename.
2. Ensure eslint is running and installed in the IDE of your choice. There will usually be some linter errors after the file rename.
3. Auto-fix what you can. This will save you a lot of time! VSCode can be set to auto fix eslint errors when files are saved.

### How to fix common TypeScript errors
Expand All @@ -14,7 +14,7 @@ The first thing that will probably happen when you convert a `.js` file in our s
#### EUI component is missing types

1. Check https://github.com/elastic/eui/issues/256 to see if they know it’s missing, if it’s not on there, add it.
2. Temporarily get around the issue by adding the missing type in the `typings/@elastic/eui/index.d.ts` file. Bonus points if you write a PR yourself to the EUI repo to add the types, but having them available back in Kibana will take some time, as a new EUI release will need to be generated, then that new release pointed to in Kibana. Best, to make forward progress, to do a temporary workaround.
2. Temporarily get around the issue by adding the missing type in the `typings/@elastic/eui/index.d.ts` file. Bonus points if you write a PR yourself to the EUI repo to add the types, but having them available back in Kibana will take some time, as a new EUI release will need to be generated, then that new release pointed to in Kibana. Best, to make forward progress, to do a temporary workaround.

```ts
// typings/@elastic/eui/index.d.ts
Expand Down Expand Up @@ -61,8 +61,8 @@ declare module '@elastic/eui' {

1. Open up the file and see how easy it would be to convert to TypeScript.
2. If it's very straightforward, go for it.
3. If it's not and you wish to stay focused on your own PR, get around the error by adding a type definition file in the same folder as the dependency, with the same name.
4. Minimally you will need to type what you are using in your PR. No need to go crazy to fully type the thing or you might be there for a while depending on what's available.
3. If it's not, and you wish to stay focused on your own PR, get around the error by adding a type definition file in the same folder as the dependency, with the same name.
4. Minimally you will need to type what you are using in your PR.

For example:

Expand Down Expand Up @@ -107,15 +107,15 @@ Use the version number that we have installed in package.json. This may not alwa

If that happens, just pick the closest one.

If yarn doesn't find the module it may not have types. For example, our `rison_node` package doesn't have types. In this case you have a few options:
If yarn doesn't find the module it may not have types. For example, our `rison_node` package doesn't have types. In this case you have a few options:

1. Contribute types into the DefinitelyTyped repo itself, or
2. Create a top level `types` folder and point to that in the tsconfig. For example, Infra team already handled this for `rison_node` and added: `x-pack/legacy/plugins/infra/types/rison_node.d.ts`. Other code uses it too so we will need to pull it up. Or,
2. Create a top level `types` folder and point to that in the tsconfig. For example, Infra team already handled this for `rison_node` and added: `x-pack/legacy/plugins/infra/types/rison_node.d.ts`. Other code uses it too, so we will need to pull it up. Or,
3. Add a `// @ts-ignore` line above the import. This should be used minimally, the above options are better. However, sometimes you have to resort to this method.

### TypeScripting react files

React has it's own concept of runtime types via `proptypes`. TypeScript gives you compile time types so I prefer those.
React has its own concept of runtime types via `proptypes`. TypeScript gives you compile time types so I prefer those.

Before:
```jsx
Expand Down Expand Up @@ -159,11 +159,11 @@ interface State {
}
```

Note that the name of `Props` and `State` doesn't matter, the order does. If you are exporting those interfaces to be used elsewhere, you probably should give them more fleshed out names, such as `ButtonProps` and `ButtonState`.
Note that the name of `Props` and `State` doesn't matter, the order does. If you are exporting those interfaces to be used elsewhere, you probably should give them more fleshed out names, such as `ButtonProps` and `ButtonState`.

### Typing functions

In react proptypes, we often will use `PropTypes.func`. In TypeScript, a function is `() => void`, or you can more fully flesh it out, for example:
In react proptypes, we often will use `PropTypes.func`. In TypeScript, a function is `() => void`, or you can more fully flesh it out, for example:

- `(inputParamName: string) => string`
- `(newLanguage: string) => void`
Expand All @@ -186,7 +186,7 @@ function ({ title, description }: {title: string, description: string}) {
...
}

or, use an interface
// or use an interface

interface Options {
title: string;
Expand All @@ -200,9 +200,9 @@ function ({ title, description }: Options) {

## Use `any` as little as possible

Using any is sometimes valid, but should rarely be used, even if to make quicker progress. Even `Unknown` is better than using `any` if you aren't sure of an input parameter.
Using any is sometimes valid, but should rarely be used, even if to make quicker progress. Even `unknown` is better than using `any` if you aren't sure of an input parameter.

If you use a variable that isn't initially defined, you should give it a type or it will be `any` by default (and strangely this isn't a warning, even though I think it should be)
If you use a variable that isn't initially defined, you should give it a type, or it will be `any` by default (and strangely this isn't a warning, even though I think it should be)

Before - `color` will be type `any`:
```js
Expand Down

0 comments on commit 8638f12

Please sign in to comment.