From 8638f12f4f183dd835caf08e1037af6686c6fae7 Mon Sep 17 00:00:00 2001 From: Philippe Oberti Date: Thu, 23 Jun 2022 09:36:11 -0500 Subject: [PATCH] Fix small typos in the root md files (#134609) * FAQ.md * STYLEGUIDE.mdx * TYPESCRIPT.md --- FAQ.md | 2 +- STYLEGUIDE.mdx | 56 ++++++++++++++++++++++++++++---------------------- TYPESCRIPT.md | 24 +++++++++++----------- 3 files changed, 45 insertions(+), 37 deletions(-) diff --git a/FAQ.md b/FAQ.md index 26c6df401d150..8a1b1aece9a42 100644 --- a/FAQ.md +++ b/FAQ.md @@ -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 diff --git a/STYLEGUIDE.mdx b/STYLEGUIDE.mdx index 64278c56ffda5..b06cfa44a4973 100644 --- a/STYLEGUIDE.mdx +++ b/STYLEGUIDE.mdx @@ -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 @@ -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? @@ -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:_ @@ -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 @@ -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 doesn’t 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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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:** diff --git a/TYPESCRIPT.md b/TYPESCRIPT.md index ae23768558f9d..9edd11ff4f1b0 100644 --- a/TYPESCRIPT.md +++ b/TYPESCRIPT.md @@ -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 @@ -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 @@ -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: @@ -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 @@ -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` @@ -186,7 +186,7 @@ function ({ title, description }: {title: string, description: string}) { ... } -or, use an interface +// or use an interface interface Options { title: string; @@ -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