From 9036d0572b50ac41a9e60eefea6e45860c24ac67 Mon Sep 17 00:00:00 2001 From: PhilippeOberti Date: Thu, 16 Jun 2022 16:46:11 -0500 Subject: [PATCH 1/3] Fix small typos in the root md files --- FAQ.md | 2 +- STYLEGUIDE.mdx | 58 ++++++++++++--------- TYPESCRIPT.md | 134 +++++++++++++++++++++++++++++++------------------ 3 files changed, 118 insertions(+), 76 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..465dfd03345d3 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 @@ -599,7 +602,7 @@ 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) +[sideeffect] http://en.wikipedia.org/wiki/Side_effect_(computer_science) ### Avoid circular dependencies @@ -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..c75a7889a1049 100644 --- a/TYPESCRIPT.md +++ b/TYPESCRIPT.md @@ -3,18 +3,25 @@ ### Converting existing code 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. -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. +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 -The first thing that will probably happen when you convert a `.js` file in our system to `.ts` is that some imports will be lacking types. +The first thing that will probably happen when you convert a `.js` file in our system to `.ts` is that some imports will +be lacking types. #### 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 @@ -22,32 +29,38 @@ The first thing that will probably happen when you convert a `.js` file in our s declare module '@elastic/eui' { // Add your types here export const EuiPopoverTitle: React.FC; - ... +... } ``` ```ts // you can now import it in -import { EuiPopoverTitle } from '@elastic/eui'; +import {EuiPopoverTitle} from '@elastic/eui'; ``` Some background on the differences between module declaration and augmentation: -In TypeScript module declarations can not be merged, which means each module can only be declared once. But it is possible to augment previously declared modules. The documentation about the distinction between module declaration and augmentation is sparse. The observed rules for `declare module '...' {}` in a `.d.ts` file seem to be: +In TypeScript module declarations can not be merged, which means each module can only be declared once. But it is +possible to augment previously declared modules. The documentation about the distinction between module declaration and +augmentation is sparse. The observed rules for `declare module '...' {}` in a `.d.ts` file seem to be: * it is treated as a module declaration when the file itself is not a module * it is treated as a module augmentation when the file itself is module -A `.d.ts` file is treated as a module if it contains any top-level `import` or `export` statements. That means that in order to write a module declaration the `import`s must be contained within the `declare` block and none must be located on the topmost level. Conversely, to write a module augmentation there must be at least one top-level `import` or `export` and the `declare` block must not contain any `import` statements. +A `.d.ts` file is treated as a module if it contains any top-level `import` or `export` statements. That means that in +order to write a module declaration the `import`s must be contained within the `declare` block and none must be located +on the topmost level. Conversely, to write a module augmentation there must be at least one top-level `import` +or `export` and the `declare` block must not contain any `import` statements. -Since `@elastic/eui` already ships with a module declaration, any local additions must be performed using module augmentation, e.g. +Since `@elastic/eui` already ships with a module declaration, any local additions must be performed using module +augmentation, e.g. ```typescript // file `typings/@elastic/eui/index.d.ts` -import { CommonProps } from '@elastic/eui'; -import { FC } from 'react'; +import {CommonProps} from '@elastic/eui'; +import {FC} from 'react'; declare module '@elastic/eui' { export type EuiNewComponentProps = CommonProps & { @@ -61,23 +74,26 @@ 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: metadata.js: + ```js export let metadata = null; export function __newPlatformInit__(legacyMetadata) { - ... +... } ``` documentation_links.js: + ```js -import { metadata } from './metadata'; +import {metadata} from './metadata'; export const DOC_LINK_VERSION = metadata.branch; ``` @@ -85,6 +101,7 @@ export const DOC_LINK_VERSION = metadata.branch; To TypeScript `documentation_links.js` you'll need to add a type definition for `metadata.js` metadata.d.ts + ``` declare interface Metadata { public branch: string; @@ -101,43 +118,50 @@ export { metadata }; `yarn add -D @types/markdown-it@8.4.1` -Use the version number that we have installed in package.json. This may not always work, and you might get something like: +Use the version number that we have installed in package.json. This may not always work, and you might get something +like: `Please choose a version of "@types/markdown-it" from this list:` 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, -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. +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 import PropTypes from 'prop-types'; - export class Button extends Component { - state = { - buttonWasClicked = false - }; +export class Button extends Component { + state = { + buttonWasClicked = false + }; - render() { - return - } - } + render() { + return + } +} - Button.proptypes = { +Button.proptypes = { text: PropTypes.string, - } +} ``` After: + ```tsx interface Props { @@ -148,22 +172,24 @@ interface State { buttonWasClicked: boolean; } - export class Button extends Component { - state = { - buttonWasClicked = false - }; +export class Button extends Component { + state = { + buttonWasClicked = false + }; - render() { - return - } - } + render() { + return + } +} ``` -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` @@ -174,37 +200,42 @@ In react proptypes, we often will use `PropTypes.func`. In TypeScript, a functi Especially since we often use the spread operator, this syntax is a little different and probably worth calling out. Before: + ```js -function ({ title, description }) { - ... +function ({title, description}) { +... } ``` After: + ```ts -function ({ title, description }: {title: string, description: string}) { - ... +function ({title, description}: { title: string, description: string }) { +... } -or, use an interface +// or use an interface interface Options { title: string; description: string; } -function ({ title, description }: Options) { - ... +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 let color; @@ -216,6 +247,7 @@ if (danger) { ``` After - `color` will be type `string`: + ```ts let color: string; @@ -226,7 +258,8 @@ if (danger) { } ``` -Another quirk, default `Map\WeakMap\Set` constructors use any-based type signature like `Map\WeakMap\Set`. That means that TS won't complain about the piece of code below: +Another quirk, default `Map\WeakMap\Set` constructors use any-based type signature +like `Map\WeakMap\Set`. That means that TS won't complain about the piece of code below: ```ts const anyMap = new Map(); @@ -240,6 +273,7 @@ anySet.add('2'); ``` So we should explicitly define types for default constructors whenever possible: + ```ts const typedMap = new Map(); typedMap.set('1', 2); From f43f87c326141c70afcb2d911af3aa31369e1888 Mon Sep 17 00:00:00 2001 From: PhilippeOberti Date: Wed, 22 Jun 2022 14:13:45 -0500 Subject: [PATCH 2/3] Undo auto-formatting from webstorm --- STYLEGUIDE.mdx | 2 +- TYPESCRIPT.md | 128 ++++++++++++++++++------------------------------- 2 files changed, 48 insertions(+), 82 deletions(-) diff --git a/STYLEGUIDE.mdx b/STYLEGUIDE.mdx index 465dfd03345d3..b06cfa44a4973 100644 --- a/STYLEGUIDE.mdx +++ b/STYLEGUIDE.mdx @@ -602,7 +602,7 @@ 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) +[sideeffect]: http://en.wikipedia.org/wiki/Side_effect_(computer_science) ### Avoid circular dependencies diff --git a/TYPESCRIPT.md b/TYPESCRIPT.md index c75a7889a1049..d4ebfacd6ae5b 100644 --- a/TYPESCRIPT.md +++ b/TYPESCRIPT.md @@ -3,25 +3,18 @@ ### Converting existing code 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. -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. +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 -The first thing that will probably happen when you convert a `.js` file in our system to `.ts` is that some imports will -be lacking types. +The first thing that will probably happen when you convert a `.js` file in our system to `.ts` is that some imports will be lacking types. #### 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 @@ -29,38 +22,32 @@ be lacking types. declare module '@elastic/eui' { // Add your types here export const EuiPopoverTitle: React.FC; -... + ... } ``` ```ts // you can now import it in -import {EuiPopoverTitle} from '@elastic/eui'; +import { EuiPopoverTitle } from '@elastic/eui'; ``` Some background on the differences between module declaration and augmentation: -In TypeScript module declarations can not be merged, which means each module can only be declared once. But it is -possible to augment previously declared modules. The documentation about the distinction between module declaration and -augmentation is sparse. The observed rules for `declare module '...' {}` in a `.d.ts` file seem to be: +In TypeScript module declarations can not be merged, which means each module can only be declared once. But it is possible to augment previously declared modules. The documentation about the distinction between module declaration and augmentation is sparse. The observed rules for `declare module '...' {}` in a `.d.ts` file seem to be: * it is treated as a module declaration when the file itself is not a module * it is treated as a module augmentation when the file itself is module -A `.d.ts` file is treated as a module if it contains any top-level `import` or `export` statements. That means that in -order to write a module declaration the `import`s must be contained within the `declare` block and none must be located -on the topmost level. Conversely, to write a module augmentation there must be at least one top-level `import` -or `export` and the `declare` block must not contain any `import` statements. +A `.d.ts` file is treated as a module if it contains any top-level `import` or `export` statements. That means that in order to write a module declaration the `import`s must be contained within the `declare` block and none must be located on the topmost level. Conversely, to write a module augmentation there must be at least one top-level `import` or `export` and the `declare` block must not contain any `import` statements. -Since `@elastic/eui` already ships with a module declaration, any local additions must be performed using module -augmentation, e.g. +Since `@elastic/eui` already ships with a module declaration, any local additions must be performed using module augmentation, e.g. ```typescript // file `typings/@elastic/eui/index.d.ts` -import {CommonProps} from '@elastic/eui'; -import {FC} from 'react'; +import { CommonProps } from '@elastic/eui'; +import { FC } from 'react'; declare module '@elastic/eui' { export type EuiNewComponentProps = CommonProps & { @@ -74,26 +61,23 @@ 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. +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: metadata.js: - ```js export let metadata = null; export function __newPlatformInit__(legacyMetadata) { -... + ... } ``` documentation_links.js: - ```js -import {metadata} from './metadata'; +import { metadata } from './metadata'; export const DOC_LINK_VERSION = metadata.branch; ``` @@ -101,7 +85,6 @@ export const DOC_LINK_VERSION = metadata.branch; To TypeScript `documentation_links.js` you'll need to add a type definition for `metadata.js` metadata.d.ts - ``` declare interface Metadata { public branch: string; @@ -118,50 +101,43 @@ export { metadata }; `yarn add -D @types/markdown-it@8.4.1` -Use the version number that we have installed in package.json. This may not always work, and you might get something -like: +Use the version number that we have installed in package.json. This may not always work, and you might get something like: `Please choose a version of "@types/markdown-it" from this list:` 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, -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. +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 its own concept of runtime types via `proptypes`. TypeScript gives you compile time types so I prefer those. Before: - ```jsx import PropTypes from 'prop-types'; -export class Button extends Component { - state = { - buttonWasClicked = false - }; + export class Button extends Component { + state = { + buttonWasClicked = false + }; - render() { - return - } -} + render() { + return + } + } -Button.proptypes = { + Button.proptypes = { text: PropTypes.string, -} + } ``` After: - ```tsx interface Props { @@ -172,24 +148,22 @@ interface State { buttonWasClicked: boolean; } -export class Button extends Component { - state = { - buttonWasClicked = false - }; + export class Button extends Component { + state = { + buttonWasClicked = false + }; - render() { - return - } -} + render() { + return + } + } ``` -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` @@ -200,18 +174,16 @@ flesh it out, for example: Especially since we often use the spread operator, this syntax is a little different and probably worth calling out. Before: - ```js -function ({title, description}) { -... +function ({ title, description }) { + ... } ``` After: - ```ts -function ({title, description}: { title: string, description: string }) { -... +function ({ title, description }: {title: string, description: string}) { + ... } // or use an interface @@ -221,21 +193,18 @@ interface Options { description: string; } -function ({title, description}: Options) { -... +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 let color; @@ -247,7 +216,6 @@ if (danger) { ``` After - `color` will be type `string`: - ```ts let color: string; @@ -258,8 +226,7 @@ if (danger) { } ``` -Another quirk, default `Map\WeakMap\Set` constructors use any-based type signature -like `Map\WeakMap\Set`. That means that TS won't complain about the piece of code below: +Another quirk, default `Map\WeakMap\Set` constructors use any-based type signature like `Map\WeakMap\Set`. That means that TS won't complain about the piece of code below: ```ts const anyMap = new Map(); @@ -273,7 +240,6 @@ anySet.add('2'); ``` So we should explicitly define types for default constructors whenever possible: - ```ts const typedMap = new Map(); typedMap.set('1', 2); From 476a28660ce78f1ed31a7dc28f4dcd713ce8a6d2 Mon Sep 17 00:00:00 2001 From: PhilippeOberti Date: Wed, 22 Jun 2022 14:18:40 -0500 Subject: [PATCH 3/3] change Unknow to unknow type --- TYPESCRIPT.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/TYPESCRIPT.md b/TYPESCRIPT.md index d4ebfacd6ae5b..9edd11ff4f1b0 100644 --- a/TYPESCRIPT.md +++ b/TYPESCRIPT.md @@ -200,7 +200,7 @@ 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)