From 37dcae5a862d5e01ffaad309bef18c543455b55c Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Mon, 26 Jun 2023 01:11:43 -0700 Subject: [PATCH 01/20] add ts guidelines --- .../PROPTYPES_CONVERSION_TABLE.md | 18 + contributingGuides/TS_CHEATSHEET.md | 183 ++++++++++ contributingGuides/TS_STYLE.md | 335 ++++++++++++++++++ 3 files changed, 536 insertions(+) create mode 100644 contributingGuides/PROPTYPES_CONVERSION_TABLE.md create mode 100644 contributingGuides/TS_CHEATSHEET.md create mode 100644 contributingGuides/TS_STYLE.md diff --git a/contributingGuides/PROPTYPES_CONVERSION_TABLE.md b/contributingGuides/PROPTYPES_CONVERSION_TABLE.md new file mode 100644 index 000000000000..9149a6883151 --- /dev/null +++ b/contributingGuides/PROPTYPES_CONVERSION_TABLE.md @@ -0,0 +1,18 @@ +# Expensify PropTypes Conversation Table + +| PropTypes | TypeScript | Instructions | +| -------------------------------------------------------------------- | --------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `PropTypes.any` | `T`, `Record` or `any` | Figure out what would be the correct data type and use it.

If you know that it's a object but isn't possible to determine the internal structure, use `Record`. | +| `PropTypes.array` or `PropTypes.arrayOf(T)` | `T[]` or `Array` | Convert to `T[]` or `Array`, where `T` is the data type of the array.

If `T` isn't a primitive type, create a separate `type` for the object structure of your prop and use it. | +| `PropTypes.bool` | `boolean` | Convert to `boolean`. | +| `PropTypes.func` | `(arg1: Type1, arg2, Type2...) => ReturnType` | Convert to the function signature. | +| `PropTypes.number` | `number` | Convert to `number`. | +| `PropTypes.object`, `PropTypes.shape(T)` or `PropTypes.exact(T)` | `T` | If `T` isn't a primitive type, create a separate `type` for the `T` object structure of your prop and use it.

If you want an object but isn't possible to determine the internal structure, use `Record`. | +| `PropTypes.objectOf(T)` | `Record` | Convert to a `Record` where `T` is the data type of your dictionary.

If `T` isn't a primitive type, create a separate `type` for the object structure and use it. | +| `PropTypes.string` | `string` | Convert to `string`. | +| `PropTypes.node` | `React.ReactNode` | Convert to `React.ReactNode`. `ReactNode` includes `ReactElement` as well as other types such as `strings`, `numbers`, `arrays` of the same, `null`, and `undefined` In other words, anything that can be rendered in React is a `ReactNode`. | +| `PropTypes.element` | `React.ReactElement` | Convert to `React.ReactElement`. | +| `PropTypes.symbol` | `symbol` | Convert to `symbol`. | +| `PropTypes.elementType` | `React.ElementType` | Convert to `React.ElementType`. | +| `PropTypes.instanceOf(T)` | `T` | Convert to `T`. | +| `PropTypes.oneOf([T, U, ...])` or `PropTypes.oneOfType([T, U, ...])` | `T \| U \| ...` | Convert to a union type e.g. `T \| U \| ...`. | diff --git a/contributingGuides/TS_CHEATSHEET.md b/contributingGuides/TS_CHEATSHEET.md new file mode 100644 index 000000000000..e056d36ddb07 --- /dev/null +++ b/contributingGuides/TS_CHEATSHEET.md @@ -0,0 +1,183 @@ +# Expensify TypeScript React Native CheatSheet + +## Table of Contents + +- [1.1 `props.children`](#convension-children) +- [1.2 `forwardRef`](#forwardRef) +- [1.3 Animated styles](#animated-style) +- [1.4 Style Props](#style-props) +- [1.5 Render Prop](#render-prop) +- [1.6 Type Narrowing](#type-narrowing) +- [1.7 Errors in Type Catch Clauses](#try-catch-errors) + +## CheatSheet + +- [1.1](#convension-children) **`props.children`** + + ```tsx + type WrapperComponentProps = { + children?: React.ReactNode; + }; + + function WrapperComponent({ children }: Props) { + return {children}; + } + + function App() { + return ( + + + + ); + } + ``` + +- [1.2](#forwardRef) **`forwardRef`** + + ```ts + import { forwardRef, useRef, ReactNode } from "react"; + import { TextInput, View } from "react-native"; + + export type CustomButtonProps = { + label: string; + children?: ReactNode; + }; + + const CustomTextInput = forwardRef( + (props, ref) => { + return ( + + + {props.children} + + ); + } + ); + + function ParentComponent() { + const ref = useRef; + return ; + } + ``` + +- [1.3](#animated-style) **Animated styles** + + ```ts + import {useRef} from 'react'; + import {Animated, StyleProp, ViewStyle} from 'react-native'; + + type MyComponentProps = { + style?: Animated.WithAnimatedValue>; + }; + + function MyComponent({ style }: Props) { + return ; + } + + function MyComponent() { + const anim = useRef(new Animated.Value(0)).current; + return ; + } + ``` + +- [1.4](#style-props) **Style Props** + + When converting or typing style props, use `StyleProp` type where `T` is the type of styles related to the component your prop is going to apply. + + ```tsx + import { StyleProp, ViewStyle, TextStyle, ImageStyle } from "react-native"; + + type MyComponentProps = { + containerStyle?: StyleProp; + textStyle?: StyleProp; + imageStyle?: StyleProp; + }; + + function MyComponentProps({ containerStyle, textStyle, imageStyle }: MyComponentProps) = { + + Sample Image + + + } + ``` + +- [1.5](#render-prop) **Render Prop** + + ```tsx + type ParentComponentProps = { + children: (label: string) => React.ReactNode; + }; + + function ParentComponent({ children }: Props) { + return children("String being injected"); + } + + function App() { + return ( + + {(label) => ( + + {label} + + )} + + ); + } + ``` + +- [1.6](#type-narrowing) **Type Narrowing** Narrow types down using `typeof` or custom type guards. + + ```ts + type Manager = { + role: "manager"; + team: string; + }; + + type Engineer = { + role: "engineer"; + language: "ts" | "js" | "php"; + }; + + function introduce(employee: Manager | Engineer) { + console.log(employee.team); // TypeScript errors: Property 'team' does not exist on type 'Manager | Engineer'. + + if (employee.role === "manager") { + console.log(`I manage ${employee.team}`); // employee: Manager + } else { + console.log(`I write ${employee.language}`); // employee: Engineer + } + } + ``` + + In the above code, type narrowing is used to determine whether an employee object is a Manager or an Engineer based on the role property, allowing safe access to the `team` property for managers and the `language` property for engineers. + + We can also create a custom type guard function. + + ```ts + function isManager(employee: Manager | Engineer): employee is Manager { + return employee.role === "manager"; + } + + function introduce(employee: Manager | Engineer) { + if (isManager(employee)) { + console.log(`I manage ${employee.team}`); // employee: Manager + } + } + ``` + + In the above code, `employee is Manager` is Type Predicate. It signifies that the return type of `isManager` is a `boolean`, indicating whether a value passed to the function is of a certain type (e.g. `Manager`). + +- [1.7] **Error in Try Catch Clauses** + + Errors in try/catch clauses are typed as unknown, if the developer needs to use the error data they must conditionally check the type of the data first. Use type narrowing + + ```ts + try { + .... + } catch (e) { // `e` is `unknown`. + if (e instanceof Error) { + // you can access properties on Error + console.error(e.message); + } + } + ``` diff --git a/contributingGuides/TS_STYLE.md b/contributingGuides/TS_STYLE.md new file mode 100644 index 000000000000..5b7d5290b079 --- /dev/null +++ b/contributingGuides/TS_STYLE.md @@ -0,0 +1,335 @@ +# Expensify TypeScript Style Guide + +## Other Expensify Resources on TypeScript + +- [Expensify TypeScript React Native CheatSheet](./TS_CHEATSHEET.md) +- [Expensify TypeScript PropTypes Conversion Table](./PROPTYPES_CONVERSION_TABLE.md) + +## Learning Sources + +### Quickest way to learn TypeScript + +- Get up to speed quickly + - [TypeScript playground](https://www.typescriptlang.org/play?q=231#example) + - Go though all examples on the playground. Click on "Example" tab on the top +- Handy Reference + - [TypeScript CheatSheet](https://www.typescriptlang.org/static/TypeScript%20Types-ae199d69aeecf7d4a2704a528d0fd3f9.png) + - [Type](https://www.typescriptlang.org/static/TypeScript%20Types-ae199d69aeecf7d4a2704a528d0fd3f9.png) + - [Control Flow Analysis](https://www.typescriptlang.org/static/TypeScript%20Control%20Flow%20Analysis-8a549253ad8470850b77c4c5c351d457.png) +- TypeScript with React + - [React TypeScript CheatSheet](https://react-typescript-cheatsheet.netlify.app/) + - [List of built-in utility types](https://react-typescript-cheatsheet.netlify.app/docs/basic/troubleshooting/utilities) + - [HOC CheatSheet](https://react-typescript-cheatsheet.netlify.app/docs/hoc/) + +## Table of Contents + +- [1.1 Naming Conventions](#convension-naming-convension) +- [1.2 `d.ts` Extension](#convensions-d-ts-extension) +- [1.3 Type Alias vs. Interface](#convensions-type-alias-vs-interface) +- [1.4 Enum vs. Union Type](#convensions-enum-vs-union-type) +- [1.5 `unknown` vs. `any`](#convensions-unknown-vs-any) +- [1.6 `T[]` vs. `Array`](#convensions-array) +- [1.7 @ts-ignore](#convension-ts-ignore) +- [1.8 Optional chaining and nullish coalescing](#convension-ts-nullish-coalescing) +- [1.9 Type Inference](#convension-type-inference) +- [1.10 JSDoc](#conventions-jsdoc) +- [1.11 `propTypes` and `defaultProps`](#convension-proptypes-and-defaultprops) +- [1.12 Utility Types](#convension-utility-types) +- [1.13 `object` Type](#convension-object-type) +- [1.14 Export Prop Types](#convension-export-prop-types) +- [Communication Items](#communication-items) +- [Migration Guidelines](#migration-guidelines) + +## Exception to Rules + +Most of the rules are enforced in ESLint or checked by TypeScript. If you think your particular situation warrants an exception, post the context in the `#expensify-open-source` Slack channel with your message prefixed with `TS EXCEPTION:`. Internal engineers will assess the case and suggest alternative or grants an exception. When an exception is granted, link the relevant Slack conversation in your pull request. Suppress ESLint or TypeScript warnings/errors with comments if necessary. + +This rule will apply until the migration is done. After the migration, exceptions are assessed and granted by PR reviewers. + +## Conventions + + + +- [1.1](#convension-naming-convension) **Naming Conventions**: Use PascalCase for type names. Do not postfix type aliases with `Type` + + ```ts + // bad + type foo = ...; + type BAR = ...; + type PersonType = ...; + + // good + type Foo = ...; + type Bar = ...; + type Person = ...; + ``` + + + +- [1.2](#convensions-d-ts-extension) **`d.ts` Extension**: Do not use `d.ts` file extension even when a file contains only type declarations. + + > Why? Type errors in `d.ts` files are not checked by TypeScript [^1]. + +[^1]: This is because `skipLibCheck` TypeScript configuration is set to `true` in this project. + + + +- [1.3](#convensions-type-alias-vs-interface) **Type Alias vs. Interface**: Do not use `interface`. Use `type`. + + > Why? In TypeScript, `type` and `interface` can be used interchangeably to declare types. Use `type` for consistency. + + ```ts + // bad + interface Person { + name: string; + } + + // good + type Person = { + name: string; + }; + ``` + + + +- [1.4](#convensions-enum-vs-union-type) **Enum vs. Union Type**: Do not use `enum`. Use union types. + + > Why? Enums come with several [pitfalls](https://blog.logrocket.com/why-typescript-enums-suck/). Most enum use cases can be replaced with union types. + + ```ts + // Most simple form of union type. + type Color = "red" | "green" | "blue"; + function printColors(color: Color) { + console.log(color); + } + + // When the values need to be iterated upon. + import { TupleToUnion } from "type-fest"; + + const COLORS = ["red", "green", "blue"] as const; + type Color = TupleToUnion; // type: 'red' | 'green' | 'blue' + + for (const colors of color) { + printColor(color); + } + + // When the values should be accessed through object keys. (i.e. `COLORS.Red` vs. `"red"`) + import { ValueOf } from "type-fest"; + + const COLORS = { + Red: "red", + Green: "green", + Blue: "blue", + } as const; + type Color = ValueOf; // type: 'red' | 'green' | 'blue' + + printColor(COLORS.Red); + ``` + + + +- [1.5](#convensions-unknown-vs-any) **`unknown` vs. `any`**: Don't use `any`. Use `unknown` if type is not known beforehand + + > Why? `any` type bypasses type checking. `unknown` is type safe as `unknown` type needs to be type narrowed before being used. + + ```ts + const value: unknown = JSON.parse(someJson); + if (typeof value === 'string') {...} + else if (isPerson(value)) {...} + ... + ``` + + + +- [1.6](#convensions-array) **`T[]` vs. `Array`**: Use T[] or readonly T[] for simple types (i.e. types which are just primitive names or type references). Use Array or ReadonlyArray for all other types (union types, intersection types, object types, function types, etc). + + ```ts + // Array + const a: Array = ["a", "b"]; + const b: Array<{ prop: string }> = [{ prop: "a" }]; + const c: Array<() => void> = [() => {}]; + + // T[] + const d: MyType[] = ["a", "b"]; + const e: string[] = ["a", "b"]; + const f: readonly string[] = ["a", "b"]; + ``` + + + +- [1.7](#convension-ts-ignore) **@ts-ignore**: Do not use `@ts-ignore` or its variant `@ts-nocheck` to suppress warnings and errors. Use `@ts-expect-error` during the migration for type errors that should be handled later. + + + +- [1.8](#convension-ts-nullish-coalescing) **Optional chaining and nullish coalescing**: Use optional chaining and nullish coalescing instead of the `get` lodash function. + + ```ts + // Bad + import { get } from "lodash"; + const name = lodashGet(user, "name", "default name"); + + // Good + const name = user?.name ?? "default name"; + ``` + + + +- [1.9](#convension-type-inference) **Type Inference**: When possible, allow the compiler to infer type of variables. + + ```ts + // Bad + const foo: string = "foo"; + const [counter, setCounter] = useState(0); + + // Good + const foo = "foo"; + const [counter, setCounter] = useState(0); + const [username, setUsername] = useState(undefined); // Username is a union type of string and undefined, and its type cannot be interred from the default value of undefined + ``` + + For function return types, default to always typing them unless a function is simple enough to reason about its return type. + + > Why? Explicit return type helps catch errors when implementation of the function changes. It also makes it easy to read code even when TypeScript intellisense is not provided. + + ```ts + function simpleFunction(name: string) { + return `hello, ${name}`; + } + + function complicatedFunction(name: string): boolean { + // ... some complex logic here ... + return foo; + } + ``` + + + +- [1.10](#conventions-jsdoc) **JSDoc**: Omit comments that are redundant with TypeScript. Do not declare types in `@param` or `@return` blocks. Do not write `@implements`, `@enum`, `@private`, `@override` + + ```ts + // bad + /** + * @param {number} age + * @returns {boolean} Whether the person is a legal drinking age or nots + */ + function canDrink(age: number): boolean { + return age >= 21; + } + + // good + /** + * @param age + * @returns Whether the person is a legal drinking age or nots + */ + ``` + + + +- [1.11](#convension-proptypes-and-defaultprops) **`propTypes` and `defaultProps`**: Do not use them. Use object destructing to assign default values if necessary. + + > Refer to [the propTypes Migration Table](./PROPTYPES_CONVERSION_TABLE.md) on how to type props based on existing `propTypes`. + + ```tsx + type GreetingProps = { + greeting: string; + name: string; + }; + + function Greeting({ greeting = "hello", name = "world" }: ComponentProps) { + {`${greeting}, ${name}`}; + } + ``` + + + +- [1.12](#convension-utility-types) **Utility Types**: Use types from [TypeScript utility types](https://www.typescriptlang.org/docs/handbook/utility-types.html) and [`type-fest`](https://github.com/sindresorhus/type-fest) when possible. + + ```ts + type Foo = { + bar: string; + }; + + // good + type ReadOnlyFoo = Readonly; + + // bad + type ReadOnlyFoo = { + readonly [Property in keyof Foo]: Foo[Property]; + }; + ``` + + + +- [1.13](#convension-object-type) **`object`**: Don't use `object` type. + +> Why? `object` refers to "any non-primitive type," not "any object". Typing "any non-primitive value" is not commonly needed. + +```ts +// bad +const foo: object = [1, 2, 3]; // TypeScript does not error +``` + +If you know that the type of data is an object but don't know what properties or values it has beforehand, use `Record`. + + + +- [1.14](#convension-export-prop-types) **Prop Types**: Define and export prop types for components. Use exported prop types instead of grabbing the prop type from a component. + +> Why? Exporting prop types aids reusability. + +```tsx +// MyComponent.tsx +export type MyComponentProps = { + foo: string; +}; + +export default function MyComponent({ foo }: MyComponentProps) { + return {foo}; +} + +// bad +import { ComponentProps } from "React"; +import MyComponent from "./MyComponent"; +type MyComponentProps = ComponentProps; + +// good +import MyComponent, { MyComponentProps } from "./MyComponent"; +``` + +## Communication Items + +> Comment in the `#expensify-open-source` Slack channel if any of the following situations are encountered. Each comment should be prefixed with `TS ATTENTION:`. Internal engineers will access each situation and prescribe solutions to each case. Internal engineers should refer to general solutions to each situation that follows each list item. + +- I think types definitions in a third party library is incomplete or incorrect + + When the library indeed contains incorrect type definitions and it cannot be updated, use module argumentation to correct them. + + ```ts + declare module "external-library-name" { + interface LibraryComponentProps { + // Add or modify typings + additionalProp: string; + } + } + ``` + +## Migration Guidelines + +> This section contains instructions that are applicable during the migration. + +- Found type bugs. Now what? + + If TypeScript migration uncovers a bug that has been “invisible,” there are two options an author of a migration PR can take + + - Fix issues if they are minor. Document each fix in the PR comment + - Suppress a TypeScript error stemming from the bug with `@ts-expect-error`. Create a separate GH issue. Prefix the issue title with `[TS ERROR #]`. Cross-link the migration PR and the created GH issue + + The `@ts-expect-error` annotation tells the TS compiler to ignore any errors in the line that follows it. However, if there's no error in the line, TypeScript will also raise an error. + + ```ts + // @ts-expect-error + const x: number = "This is a string"; // No TS error raised + + // @ts-expect-error + const y: number = 123; // TS error: Unused '@ts-expect-error' directive. + ``` From c218adc03e7df487008269745275ab5797ec836f Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Mon, 26 Jun 2023 01:55:07 -0700 Subject: [PATCH 02/20] fix issues --- contributingGuides/TS_CHEATSHEET.md | 22 +++++++++++--- contributingGuides/TS_STYLE.md | 45 +++++++++++++++-------------- 2 files changed, 42 insertions(+), 25 deletions(-) diff --git a/contributingGuides/TS_CHEATSHEET.md b/contributingGuides/TS_CHEATSHEET.md index e056d36ddb07..c5a6723da459 100644 --- a/contributingGuides/TS_CHEATSHEET.md +++ b/contributingGuides/TS_CHEATSHEET.md @@ -2,17 +2,19 @@ ## Table of Contents -- [1.1 `props.children`](#convension-children) +- [1.1 `props.children`](#children-prop) - [1.2 `forwardRef`](#forwardRef) - [1.3 Animated styles](#animated-style) - [1.4 Style Props](#style-props) - [1.5 Render Prop](#render-prop) - [1.6 Type Narrowing](#type-narrowing) -- [1.7 Errors in Type Catch Clauses](#try-catch-errors) +- [1.7 Errors in Type Catch Clauses](#try-catch-clauses) ## CheatSheet -- [1.1](#convension-children) **`props.children`** + + +- [1.1](#children-prop) **`props.children`** ```tsx type WrapperComponentProps = { @@ -32,6 +34,8 @@ } ``` + + - [1.2](#forwardRef) **`forwardRef`** ```ts @@ -60,6 +64,8 @@ } ``` + + - [1.3](#animated-style) **Animated styles** ```ts @@ -80,6 +86,8 @@ } ``` + + - [1.4](#style-props) **Style Props** When converting or typing style props, use `StyleProp` type where `T` is the type of styles related to the component your prop is going to apply. @@ -101,6 +109,8 @@ } ``` + + - [1.5](#render-prop) **Render Prop** ```tsx @@ -125,6 +135,8 @@ } ``` + + - [1.6](#type-narrowing) **Type Narrowing** Narrow types down using `typeof` or custom type guards. ```ts @@ -167,7 +179,9 @@ In the above code, `employee is Manager` is Type Predicate. It signifies that the return type of `isManager` is a `boolean`, indicating whether a value passed to the function is of a certain type (e.g. `Manager`). -- [1.7] **Error in Try Catch Clauses** + + +- [1.7](#try-catch-clauses) **Error in Try Catch Clauses** Errors in try/catch clauses are typed as unknown, if the developer needs to use the error data they must conditionally check the type of the data first. Use type narrowing diff --git a/contributingGuides/TS_STYLE.md b/contributingGuides/TS_STYLE.md index 5b7d5290b079..9923e6317283 100644 --- a/contributingGuides/TS_STYLE.md +++ b/contributingGuides/TS_STYLE.md @@ -1,11 +1,33 @@ # Expensify TypeScript Style Guide +## Table of Contents + +- [Other Expensify Resources on TypeScript](#other-expensify-resources-on-typescript) +- [Learning Resources](#learning-resources) +- [Exception to Rules](#exception-to-rules) +- [1.1 Naming Conventions](#convension-naming-convension) +- [1.2 `d.ts` Extension](#convensions-d-ts-extension) +- [1.3 Type Alias vs. Interface](#convensions-type-alias-vs-interface) +- [1.4 Enum vs. Union Type](#convensions-enum-vs-union-type) +- [1.5 `unknown` vs. `any`](#convensions-unknown-vs-any) +- [1.6 `T[]` vs. `Array`](#convensions-array) +- [1.7 @ts-ignore](#convension-ts-ignore) +- [1.8 Optional chaining and nullish coalescing](#convension-ts-nullish-coalescing) +- [1.9 Type Inference](#convension-type-inference) +- [1.10 JSDoc](#conventions-jsdoc) +- [1.11 `propTypes` and `defaultProps`](#convension-proptypes-and-defaultprops) +- [1.12 Utility Types](#convension-utility-types) +- [1.13 `object` Type](#convension-object-type) +- [1.14 Export Prop Types](#convension-export-prop-types) +- [Communication Items](#communication-items) +- [Migration Guidelines](#migration-guidelines) + ## Other Expensify Resources on TypeScript - [Expensify TypeScript React Native CheatSheet](./TS_CHEATSHEET.md) - [Expensify TypeScript PropTypes Conversion Table](./PROPTYPES_CONVERSION_TABLE.md) -## Learning Sources +## Learning Resources ### Quickest way to learn TypeScript @@ -21,32 +43,13 @@ - [List of built-in utility types](https://react-typescript-cheatsheet.netlify.app/docs/basic/troubleshooting/utilities) - [HOC CheatSheet](https://react-typescript-cheatsheet.netlify.app/docs/hoc/) -## Table of Contents - -- [1.1 Naming Conventions](#convension-naming-convension) -- [1.2 `d.ts` Extension](#convensions-d-ts-extension) -- [1.3 Type Alias vs. Interface](#convensions-type-alias-vs-interface) -- [1.4 Enum vs. Union Type](#convensions-enum-vs-union-type) -- [1.5 `unknown` vs. `any`](#convensions-unknown-vs-any) -- [1.6 `T[]` vs. `Array`](#convensions-array) -- [1.7 @ts-ignore](#convension-ts-ignore) -- [1.8 Optional chaining and nullish coalescing](#convension-ts-nullish-coalescing) -- [1.9 Type Inference](#convension-type-inference) -- [1.10 JSDoc](#conventions-jsdoc) -- [1.11 `propTypes` and `defaultProps`](#convension-proptypes-and-defaultprops) -- [1.12 Utility Types](#convension-utility-types) -- [1.13 `object` Type](#convension-object-type) -- [1.14 Export Prop Types](#convension-export-prop-types) -- [Communication Items](#communication-items) -- [Migration Guidelines](#migration-guidelines) - ## Exception to Rules Most of the rules are enforced in ESLint or checked by TypeScript. If you think your particular situation warrants an exception, post the context in the `#expensify-open-source` Slack channel with your message prefixed with `TS EXCEPTION:`. Internal engineers will assess the case and suggest alternative or grants an exception. When an exception is granted, link the relevant Slack conversation in your pull request. Suppress ESLint or TypeScript warnings/errors with comments if necessary. This rule will apply until the migration is done. After the migration, exceptions are assessed and granted by PR reviewers. -## Conventions +## Guidelines From b40a302beeae3dedd272730b457c7fe1f65f9c35 Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Mon, 26 Jun 2023 22:26:33 -0700 Subject: [PATCH 03/20] add section on file organization --- contributingGuides/TS_STYLE.md | 129 +++++++++++++++++++++++++-------- 1 file changed, 97 insertions(+), 32 deletions(-) diff --git a/contributingGuides/TS_STYLE.md b/contributingGuides/TS_STYLE.md index 9923e6317283..8fa8827e4762 100644 --- a/contributingGuides/TS_STYLE.md +++ b/contributingGuides/TS_STYLE.md @@ -19,6 +19,7 @@ - [1.12 Utility Types](#convension-utility-types) - [1.13 `object` Type](#convension-object-type) - [1.14 Export Prop Types](#convension-export-prop-types) +- [1.15 File Organization](#convension-file-organization) - [Communication Items](#communication-items) - [Migration Guidelines](#migration-guidelines) @@ -265,39 +266,102 @@ This rule will apply until the migration is done. After the migration, exception - [1.13](#convension-object-type) **`object`**: Don't use `object` type. -> Why? `object` refers to "any non-primitive type," not "any object". Typing "any non-primitive value" is not commonly needed. + > Why? `object` refers to "any non-primitive type," not "any object". Typing "any non-primitive value" is not commonly needed. -```ts -// bad -const foo: object = [1, 2, 3]; // TypeScript does not error -``` + ```ts + // bad + const foo: object = [1, 2, 3]; // TypeScript does not error + ``` + + If you know that the type of data is an object but don't know what properties or values it has beforehand, use `Record`. -If you know that the type of data is an object but don't know what properties or values it has beforehand, use `Record`. + > Even though `string` is specified as a key, `Record` type can still accepts objects whose keys are numbers or symbols. This is because number and + + ```ts + function logObject(object: Record) { + for (const [key, value] of Object.entries(object)) { + console.log(`${key}: ${value}`); + } + } + ``` - [1.14](#convension-export-prop-types) **Prop Types**: Define and export prop types for components. Use exported prop types instead of grabbing the prop type from a component. -> Why? Exporting prop types aids reusability. + > Why? Exporting prop types aids reusability. + + ```tsx + // MyComponent.tsx + export type MyComponentProps = { + foo: string; + }; + + export default function MyComponent({ foo }: MyComponentProps) { + return {foo}; + } -```tsx -// MyComponent.tsx -export type MyComponentProps = { - foo: string; -}; + // bad + import { ComponentProps } from "React"; + import MyComponent from "./MyComponent"; + type MyComponentProps = ComponentProps; -export default function MyComponent({ foo }: MyComponentProps) { - return {foo}; -} + // good + import MyComponent, { MyComponentProps } from "./MyComponent"; + ``` -// bad -import { ComponentProps } from "React"; -import MyComponent from "./MyComponent"; -type MyComponentProps = ComponentProps; + -// good -import MyComponent, { MyComponentProps } from "./MyComponent"; -``` +- [1.15](#convension-file-organization) **File organization**: In modules with platform-specific implementations, create `types.ts` to define shared types. Import and use shared types in each platform specific files. + + > Why? To encourage consistent API across platform-specific implementations. + + ```ts + // types.ts + type GreetingModule = { + sayHello: () => boolean; + sayGoodbye: () => boolean; + }; + + // index.native.ts + import { GreetingModule } from "./types.ts"; + function sayHello() { + console.log("hello from mobile code"); + } + function sayGoodbye() { + console.log("goodbye from mobile code"); + } + const Greeting: GreetingModule = { + sayHello, + sayGoodbye, + }; + export default Greeting; + + // index.ts + import { GreetingModule } from "./types.ts"; + ... + const Greeting: GreetingModule = { + ... + ``` + + ```ts + // types.ts + export type MyComponentProps = { + foo: string; + } + + // index.ios.ts + import { MyComponentProps } from ./types.ts; + + export MyComponentProps; + export default function MyComponent({ foo }: MyComponentProps) {...} + + // index.ts + import { MyComponentProps } from ./types.ts; + + export MyComponentProps; + export default function MyComponent({ foo }: MyComponentProps) {...} + ``` ## Communication Items @@ -305,16 +369,16 @@ import MyComponent, { MyComponentProps } from "./MyComponent"; - I think types definitions in a third party library is incomplete or incorrect - When the library indeed contains incorrect type definitions and it cannot be updated, use module argumentation to correct them. +When the library indeed contains incorrect type definitions and it cannot be updated, use module augmentation to correct them. All module augmentation code should be contained in `global.d.ts`. - ```ts - declare module "external-library-name" { - interface LibraryComponentProps { - // Add or modify typings - additionalProp: string; - } +```ts +declare module "external-library-name" { + interface LibraryComponentProps { + // Add or modify typings + additionalProp: string; } - ``` +} +``` ## Migration Guidelines @@ -325,13 +389,14 @@ import MyComponent, { MyComponentProps } from "./MyComponent"; If TypeScript migration uncovers a bug that has been “invisible,” there are two options an author of a migration PR can take - Fix issues if they are minor. Document each fix in the PR comment - - Suppress a TypeScript error stemming from the bug with `@ts-expect-error`. Create a separate GH issue. Prefix the issue title with `[TS ERROR #]`. Cross-link the migration PR and the created GH issue + - Suppress a TypeScript error stemming from the bug with `@ts-expect-error`. Create a separate GH issue. Prefix the issue title with `[TS ERROR #]`. Cross-link the migration PR and the created GH issue. On the line below `@ts-expect-error`, put down the GH issue number prefixed with `TODO:`. The `@ts-expect-error` annotation tells the TS compiler to ignore any errors in the line that follows it. However, if there's no error in the line, TypeScript will also raise an error. ```ts // @ts-expect-error - const x: number = "This is a string"; // No TS error raised + // TODO: #21647 + const x: number = "123"; // No TS error raised // @ts-expect-error const y: number = 123; // TS error: Unused '@ts-expect-error' directive. From 842bc5fbc29054dacd3297d3f9f150e1451a6432 Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Tue, 27 Jun 2023 16:56:34 -0700 Subject: [PATCH 04/20] address PR reviews --- contributingGuides/TS_CHEATSHEET.md | 8 ++++---- contributingGuides/TS_STYLE.md | 19 ++++++++++++------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/contributingGuides/TS_CHEATSHEET.md b/contributingGuides/TS_CHEATSHEET.md index c5a6723da459..cff733a4985d 100644 --- a/contributingGuides/TS_CHEATSHEET.md +++ b/contributingGuides/TS_CHEATSHEET.md @@ -90,7 +90,7 @@ - [1.4](#style-props) **Style Props** - When converting or typing style props, use `StyleProp` type where `T` is the type of styles related to the component your prop is going to apply. + Use `StyleProp` to type style props. For pass-through style props, use types exported from `react-native` for the type parameter (e.g. `ViewStyle`). ```tsx import { StyleProp, ViewStyle, TextStyle, ImageStyle } from "react-native"; @@ -118,7 +118,7 @@ children: (label: string) => React.ReactNode; }; - function ParentComponent({ children }: Props) { + function ParentComponent({ children }: ParentComponentProps) { return children("String being injected"); } @@ -177,13 +177,13 @@ } ``` - In the above code, `employee is Manager` is Type Predicate. It signifies that the return type of `isManager` is a `boolean`, indicating whether a value passed to the function is of a certain type (e.g. `Manager`). + In the above code, `employee is Manager` is a [type predicate](https://www.typescriptlang.org/docs/handbook/2/narrowing.html#using-type-predicates). It means that the return type of `isManager` is a `boolean` that indicates whether a value passed to the function is of a certain type (e.g. `Manager`). - [1.7](#try-catch-clauses) **Error in Try Catch Clauses** - Errors in try/catch clauses are typed as unknown, if the developer needs to use the error data they must conditionally check the type of the data first. Use type narrowing + Errors in try/catch clauses are inferred as `unknown`. If the error dat needs to be accessed, the type of the error needs to be checked and narrowed down. ```ts try { diff --git a/contributingGuides/TS_STYLE.md b/contributingGuides/TS_STYLE.md index 8fa8827e4762..619fa4a10bb3 100644 --- a/contributingGuides/TS_STYLE.md +++ b/contributingGuides/TS_STYLE.md @@ -132,7 +132,7 @@ This rule will apply until the migration is done. After the migration, exception -- [1.5](#convensions-unknown-vs-any) **`unknown` vs. `any`**: Don't use `any`. Use `unknown` if type is not known beforehand +- [1.5](#convensions-unknown-vs-any) **`unknown` vs. `any`**: Don't use `any`. Use `unknown` if type is not known beforehand. > Why? `any` type bypasses type checking. `unknown` is type safe as `unknown` type needs to be type narrowed before being used. @@ -145,7 +145,7 @@ This rule will apply until the migration is done. After the migration, exception -- [1.6](#convensions-array) **`T[]` vs. `Array`**: Use T[] or readonly T[] for simple types (i.e. types which are just primitive names or type references). Use Array or ReadonlyArray for all other types (union types, intersection types, object types, function types, etc). +- [1.6](#convensions-array) **`T[]` vs. `Array`**: Use `T[]` or `readonly T[]` for simple types (i.e. types which are just primitive names or type references). Use `Array` or `ReadonlyArray` for all other types (union types, intersection types, object types, function types, etc). ```ts // Array @@ -234,13 +234,18 @@ This rule will apply until the migration is done. After the migration, exception > Refer to [the propTypes Migration Table](./PROPTYPES_CONVERSION_TABLE.md) on how to type props based on existing `propTypes`. ```tsx - type GreetingProps = { - greeting: string; - name: string; + type MyComponentProps = { + requiredProp: string; + optionalPropWithDefaultValue?: number; + optionalProp?: boolean; }; - function Greeting({ greeting = "hello", name = "world" }: ComponentProps) { - {`${greeting}, ${name}`}; + function MyComponent({ + requiredProp, + optionalPropWithDefaultValue = 42, + optionalProp, + }: MyComponentProps) { + // component's code } ``` From d89fc4c60be97d18e588868341a2e8eb403f27d5 Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Tue, 27 Jun 2023 21:21:12 -0700 Subject: [PATCH 05/20] add conversion example code --- .../PROPTYPES_CONVERSION_TABLE.md | 113 ++++++++++++++++++ contributingGuides/TS_CHEATSHEET.md | 4 +- contributingGuides/TS_STYLE.md | 63 ++++++---- 3 files changed, 153 insertions(+), 27 deletions(-) diff --git a/contributingGuides/PROPTYPES_CONVERSION_TABLE.md b/contributingGuides/PROPTYPES_CONVERSION_TABLE.md index 9149a6883151..c45e71218df1 100644 --- a/contributingGuides/PROPTYPES_CONVERSION_TABLE.md +++ b/contributingGuides/PROPTYPES_CONVERSION_TABLE.md @@ -1,5 +1,38 @@ # Expensify PropTypes Conversation Table +## Table of Contents + +- [Important Considerations](#important-considerations) + - [Don't Rely on `isRequired`](#dont-rely-on-isrequired) +- [PropTypes Conversion Table](#proptypes-conversion-table) +- [Conversion Example](#conversion-example) + +## Important Considerations + +### Don't Rely on `isRequired` + +Regardless of `isRequired` is present or not on props in `PropTypes`, read through the component implementation to check if props without `isRequired` can actually be optional. The use of `isRequired` is not consistent in the current codebase. Just because `isRequired` is not present, it does not necessarily mean that the prop is optional. + +One trick is to mark the prop in question with optional modifier `?`. See if the "possibly `undefined`" error is raised by TypeScript. If any error is raised, the implementation assumes the prop not to be optional. + +```ts +// Before +const propTypes = { + isVisible: PropTypes.bool.isRequired, + // `confirmText` prop is not marked as required here, theoretically it is optional. + confirmText: PropTypes.string, +}; + +// After +type Props = { + isVisible: boolean; + // Consider it as required unless you have proof that it is indeed an optional prop. + confirmText: string; // vs. confirmText?: string; +}; +``` + +## PropTypes Conversion Table + | PropTypes | TypeScript | Instructions | | -------------------------------------------------------------------- | --------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `PropTypes.any` | `T`, `Record` or `any` | Figure out what would be the correct data type and use it.

If you know that it's a object but isn't possible to determine the internal structure, use `Record`. | @@ -16,3 +49,83 @@ | `PropTypes.elementType` | `React.ElementType` | Convert to `React.ElementType`. | | `PropTypes.instanceOf(T)` | `T` | Convert to `T`. | | `PropTypes.oneOf([T, U, ...])` or `PropTypes.oneOfType([T, U, ...])` | `T \| U \| ...` | Convert to a union type e.g. `T \| U \| ...`. | + +## Conversion Example + +```ts +// Before +const propTypes = { + unknownData: PropTypes.any, + anotherUnknownData: PropTypes.any, + indexes: PropTypes.arrayOf(PropTypes.number), + items: PropTypes.arrayOf( + PropTypes.shape({ + value: PropTypes.string, + label: PropTypes.string, + }) + ), + shouldShowIcon: PropTypes.bool, + onChangeText: PropTypes.func, + count: PropTypes.number, + session: PropTypes.shape({ + authToken: PropTypes.string, + accountID: PropTypes.number, + }), + errors: PropTypes.objectOf(PropTypes.string), + inputs: PropTypes.objectOf( + PropTypes.shape({ + id: PropTypes.string, + label: PropTypes.string, + }) + ), + label: PropTypes.string, + anchor: PropTypes.node, + footer: PropTypes.element, + uniqSymbol: PropTypes.symbol, + icon: PropTypes.elementType, + date: PropTypes.instanceOf(Date), + size: PropTypes.oneOf(["small", "medium", "large"]), +}; + +// After +type Item = { + value: string; + label: string; +}; + +type Session = { + authToken: string; + accountID: number; +}; + +type Input = { + id: string; + label: string; +}; + +type Size = "small" | "medium" | "large"; + +type Props = { + unknownData: string[]; + + // It's not possible to infer the data as it can be anything because of reasons X, Y and Z. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + anotherUnknownData: any; + + indexes: number[]; + items: Item[]; + shouldShowIcon: boolean; + onChangeText: (value: string) => void; + count: number; + session: Session; + errors: Record; + inputs: Record; + label: string; + anchor: React.ReactNode; + footer: React.ReactElement; + uniqSymbol: symbol; + icon: React.ElementType; + date: Date; + size: Size; +}; +``` diff --git a/contributingGuides/TS_CHEATSHEET.md b/contributingGuides/TS_CHEATSHEET.md index cff733a4985d..5322d250f730 100644 --- a/contributingGuides/TS_CHEATSHEET.md +++ b/contributingGuides/TS_CHEATSHEET.md @@ -8,7 +8,7 @@ - [1.4 Style Props](#style-props) - [1.5 Render Prop](#render-prop) - [1.6 Type Narrowing](#type-narrowing) -- [1.7 Errors in Type Catch Clauses](#try-catch-clauses) +- [1.7 Errors in Try-Catch Clauses](#try-catch-clauses) ## CheatSheet @@ -181,7 +181,7 @@ -- [1.7](#try-catch-clauses) **Error in Try Catch Clauses** +- [1.7](#try-catch-clauses) **Error in Try-Catch Clauses** Errors in try/catch clauses are inferred as `unknown`. If the error dat needs to be accessed, the type of the error needs to be checked and narrowed down. diff --git a/contributingGuides/TS_STYLE.md b/contributingGuides/TS_STYLE.md index 619fa4a10bb3..73f6ac1b9bb4 100644 --- a/contributingGuides/TS_STYLE.md +++ b/contributingGuides/TS_STYLE.md @@ -5,21 +5,22 @@ - [Other Expensify Resources on TypeScript](#other-expensify-resources-on-typescript) - [Learning Resources](#learning-resources) - [Exception to Rules](#exception-to-rules) -- [1.1 Naming Conventions](#convension-naming-convension) -- [1.2 `d.ts` Extension](#convensions-d-ts-extension) -- [1.3 Type Alias vs. Interface](#convensions-type-alias-vs-interface) -- [1.4 Enum vs. Union Type](#convensions-enum-vs-union-type) -- [1.5 `unknown` vs. `any`](#convensions-unknown-vs-any) -- [1.6 `T[]` vs. `Array`](#convensions-array) -- [1.7 @ts-ignore](#convension-ts-ignore) -- [1.8 Optional chaining and nullish coalescing](#convension-ts-nullish-coalescing) -- [1.9 Type Inference](#convension-type-inference) -- [1.10 JSDoc](#conventions-jsdoc) -- [1.11 `propTypes` and `defaultProps`](#convension-proptypes-and-defaultprops) -- [1.12 Utility Types](#convension-utility-types) -- [1.13 `object` Type](#convension-object-type) -- [1.14 Export Prop Types](#convension-export-prop-types) -- [1.15 File Organization](#convension-file-organization) +- [Guidelines](#guidelines) + - [1.1 Naming Conventions](#convension-naming-convension) + - [1.2 `d.ts` Extension](#convensions-d-ts-extension) + - [1.3 Type Alias vs. Interface](#convensions-type-alias-vs-interface) + - [1.4 Enum vs. Union Type](#convensions-enum-vs-union-type) + - [1.5 `unknown` vs. `any`](#convensions-unknown-vs-any) + - [1.6 `T[]` vs. `Array`](#convensions-array) + - [1.7 @ts-ignore](#convension-ts-ignore) + - [1.8 Optional chaining and nullish coalescing](#convension-ts-nullish-coalescing) + - [1.9 Type Inference](#convension-type-inference) + - [1.10 JSDoc](#conventions-jsdoc) + - [1.11 `propTypes` and `defaultProps`](#convension-proptypes-and-defaultprops) + - [1.12 Utility Types](#convension-utility-types) + - [1.13 `object` Type](#convension-object-type) + - [1.14 Export Prop Types](#convension-export-prop-types) + - [1.15 File Organization](#convension-file-organization) - [Communication Items](#communication-items) - [Migration Guidelines](#migration-guidelines) @@ -321,34 +322,46 @@ This rule will apply until the migration is done. After the migration, exception > Why? To encourage consistent API across platform-specific implementations. + Utility module example + ```ts // types.ts type GreetingModule = { - sayHello: () => boolean; - sayGoodbye: () => boolean; + getHello: () => string; + getGoodbye: () => string; }; // index.native.ts import { GreetingModule } from "./types.ts"; - function sayHello() { - console.log("hello from mobile code"); + function getHello() { + return "hello from mobile code"; } - function sayGoodbye() { - console.log("goodbye from mobile code"); + function getGoodbye() { + return "goodbye from mobile code"; } const Greeting: GreetingModule = { - sayHello, - sayGoodbye, + getHello, + getGoodbye, }; export default Greeting; // index.ts import { GreetingModule } from "./types.ts"; - ... + function getHello() { + return "hello from other platform code"; + } + function getGoodbye() { + return "goodbye from other platform code"; + } const Greeting: GreetingModule = { - ... + getHello, + getGoodbye, + }; + export default Greeting; ``` + Component module example + ```ts // types.ts export type MyComponentProps = { From 42ef63272f034999ca906571617b7998531bfa33 Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Wed, 28 Jun 2023 00:04:03 -0700 Subject: [PATCH 06/20] add more sections --- contributingGuides/TS_STYLE.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/contributingGuides/TS_STYLE.md b/contributingGuides/TS_STYLE.md index 73f6ac1b9bb4..e31436f65357 100644 --- a/contributingGuides/TS_STYLE.md +++ b/contributingGuides/TS_STYLE.md @@ -5,6 +5,7 @@ - [Other Expensify Resources on TypeScript](#other-expensify-resources-on-typescript) - [Learning Resources](#learning-resources) - [Exception to Rules](#exception-to-rules) +- [General Rules](#general-rules) - [Guidelines](#guidelines) - [1.1 Naming Conventions](#convension-naming-convension) - [1.2 `d.ts` Extension](#convensions-d-ts-extension) @@ -51,6 +52,17 @@ Most of the rules are enforced in ESLint or checked by TypeScript. If you think This rule will apply until the migration is done. After the migration, exceptions are assessed and granted by PR reviewers. +## General Rules + +Strive to type as strictly as possible. + +```ts +type Foo = { + fetchingStatus: "loading" | "success" | "error"; // vs. fetchingStatus: string; + person: { name: string; age: number }; // vs. person: Record; +}; +``` + ## Guidelines @@ -381,6 +393,18 @@ This rule will apply until the migration is done. After the migration, exception export default function MyComponent({ foo }: MyComponentProps) {...} ``` + + +- [1.16] **Reusable Types**: Reusable type definitions, such as models (e.g. Report), must have their own file and be placed under `src/types/`. The type should be exported as a default export. + + ```ts + // src/types/Report.ts + + type Report = {...}; + + export default Report; + ``` + ## Communication Items > Comment in the `#expensify-open-source` Slack channel if any of the following situations are encountered. Each comment should be prefixed with `TS ATTENTION:`. Internal engineers will access each situation and prescribe solutions to each case. Internal engineers should refer to general solutions to each situation that follows each list item. From de56cb2031a9762ad99e33737308fd261f9c7956 Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Sun, 2 Jul 2023 23:57:02 -0700 Subject: [PATCH 07/20] address PR reviews --- contributingGuides/TS_CHEATSHEET.md | 18 ++++ contributingGuides/TS_STYLE.md | 126 +++++++++++++++------------- 2 files changed, 85 insertions(+), 59 deletions(-) diff --git a/contributingGuides/TS_CHEATSHEET.md b/contributingGuides/TS_CHEATSHEET.md index 5322d250f730..fa21abf140b6 100644 --- a/contributingGuides/TS_CHEATSHEET.md +++ b/contributingGuides/TS_CHEATSHEET.md @@ -9,6 +9,7 @@ - [1.5 Render Prop](#render-prop) - [1.6 Type Narrowing](#type-narrowing) - [1.7 Errors in Try-Catch Clauses](#try-catch-clauses) +- [1.8 Const Assertion](#const-assertion) ## CheatSheet @@ -195,3 +196,20 @@ } } ``` + + + +- [1.8](#const-assersion) **Use const assertions for rigorous typing** + + Use `as const` when you want to ensure that the types and values are as exact as possible and prevent unwanted mutations. + + ```ts + const greeting1 = "hello"; // type: string + const greeting2 = "goodbye" as const; // type: "goodbye" + + const person1 = { name: "Alice", age: 20 }; // type: { name: string, age: number } + const person2 = { name: "Bob", age: 30 } as const; // type: { readonly name: "Bob", readonly age; 30 } + + const array1 = ["hello", 1]; // type: (string | number)[] + const array2 = ["goodbye", 2]; // type: readonly ["goodbye", 2] + ``` diff --git a/contributingGuides/TS_STYLE.md b/contributingGuides/TS_STYLE.md index e31436f65357..c113cf44213b 100644 --- a/contributingGuides/TS_STYLE.md +++ b/contributingGuides/TS_STYLE.md @@ -7,22 +7,23 @@ - [Exception to Rules](#exception-to-rules) - [General Rules](#general-rules) - [Guidelines](#guidelines) - - [1.1 Naming Conventions](#convension-naming-convension) - - [1.2 `d.ts` Extension](#convensions-d-ts-extension) - - [1.3 Type Alias vs. Interface](#convensions-type-alias-vs-interface) - - [1.4 Enum vs. Union Type](#convensions-enum-vs-union-type) - - [1.5 `unknown` vs. `any`](#convensions-unknown-vs-any) - - [1.6 `T[]` vs. `Array`](#convensions-array) - - [1.7 @ts-ignore](#convension-ts-ignore) - - [1.8 Optional chaining and nullish coalescing](#convension-ts-nullish-coalescing) - - [1.9 Type Inference](#convension-type-inference) - - [1.10 JSDoc](#conventions-jsdoc) - - [1.11 `propTypes` and `defaultProps`](#convension-proptypes-and-defaultprops) - - [1.12 Utility Types](#convension-utility-types) - - [1.13 `object` Type](#convension-object-type) - - [1.14 Export Prop Types](#convension-export-prop-types) - - [1.15 File Organization](#convension-file-organization) -- [Communication Items](#communication-items) + - [1.1 Naming Conventions](#naming-conventions) + - [1.2 `d.ts` Extension](#d-ts-extension) + - [1.3 Type Alias vs. Interface](#type-alias-vs-interface) + - [1.4 Enum vs. Union Type](#enum-vs-union-type) + - [1.5 `unknown` vs. `any`](#unknown-vs-any) + - [1.6 `T[]` vs. `Array`](#array) + - [1.7 @ts-ignore](#ts-ignore) + - [1.8 Optional chaining and nullish coalescing](#ts-nullish-coalescing) + - [1.9 Type Inference](#type-inference) + - [1.10 JSDoc](#jsdoc) + - [1.11 `propTypes` and `defaultProps`](#proptypes-and-defaultprops) + - [1.12 Utility Types](#utility-types) + - [1.13 `object` Type](#object-type) + - [1.14 Export Prop Types](#export-prop-types) + - [1.15 File Organization](#file-organization) + - [1.16 Reusable Types](#reusable-types) +- [Communication Items](#items) - [Migration Guidelines](#migration-guidelines) ## Other Expensify Resources on TypeScript @@ -65,33 +66,35 @@ type Foo = { ## Guidelines - + -- [1.1](#convension-naming-convension) **Naming Conventions**: Use PascalCase for type names. Do not postfix type aliases with `Type` +- [1.1](#naming-conventions) **Naming Conventions**: Use PascalCase for type names. Do not postfix type aliases with `Type`. Use singular name for union types. eslint: [`@typescript-eslint/naming-convention`](https://typescript-eslint.io/rules/naming-convention/) ```ts // bad type foo = ...; type BAR = ...; type PersonType = ...; + type Colors = 'red' | 'blue' | 'green'; // good type Foo = ...; type Bar = ...; type Person = ...; + type Color = 'red' | 'blue' | 'green'; ``` - + -- [1.2](#convensions-d-ts-extension) **`d.ts` Extension**: Do not use `d.ts` file extension even when a file contains only type declarations. +- [1.2](#d-ts-extension) **`d.ts` Extension**: Do not use `d.ts` file extension even when a file contains only type declarations. > Why? Type errors in `d.ts` files are not checked by TypeScript [^1]. [^1]: This is because `skipLibCheck` TypeScript configuration is set to `true` in this project. - + -- [1.3](#convensions-type-alias-vs-interface) **Type Alias vs. Interface**: Do not use `interface`. Use `type`. +- [1.3](#type-alias-vs-interface) **Type Alias vs. Interface**: Do not use `interface`. Use `type`. eslint: [`@typescript-eslint/consistent-type-definitions`](https://typescript-eslint.io/rules/consistent-type-definitions/) > Why? In TypeScript, `type` and `interface` can be used interchangeably to declare types. Use `type` for consistency. @@ -107,9 +110,9 @@ type Foo = { }; ``` - + -- [1.4](#convensions-enum-vs-union-type) **Enum vs. Union Type**: Do not use `enum`. Use union types. +- [1.4](#enum-vs-union-type) **Enum vs. Union Type**: Do not use `enum`. Use union types. eslint: [`no-restricted-syntax`](https://eslint.org/docs/latest/rules/no-restricted-syntax) > Why? Enums come with several [pitfalls](https://blog.logrocket.com/why-typescript-enums-suck/). Most enum use cases can be replaced with union types. @@ -143,9 +146,9 @@ type Foo = { printColor(COLORS.Red); ``` - + -- [1.5](#convensions-unknown-vs-any) **`unknown` vs. `any`**: Don't use `any`. Use `unknown` if type is not known beforehand. +- [1.5](#unknown-vs-any) **`unknown` vs. `any`**: Don't use `any`. Use `unknown` if type is not known beforehand. eslint: [`@typescript-eslint/no-explicit-any`](https://typescript-eslint.io/rules/no-explicit-any/) > Why? `any` type bypasses type checking. `unknown` is type safe as `unknown` type needs to be type narrowed before being used. @@ -156,9 +159,9 @@ type Foo = { ... ``` - + -- [1.6](#convensions-array) **`T[]` vs. `Array`**: Use `T[]` or `readonly T[]` for simple types (i.e. types which are just primitive names or type references). Use `Array` or `ReadonlyArray` for all other types (union types, intersection types, object types, function types, etc). +- [1.6](#array) **`T[]` vs. `Array`**: Use `T[]` or `readonly T[]` for simple types (i.e. types which are just primitive names or type references). Use `Array` or `ReadonlyArray` for all other types (union types, intersection types, object types, function types, etc). eslint: [`@typescript-eslint/array-type`](https://typescript-eslint.io/rules/array-type/) ```ts // Array @@ -172,26 +175,28 @@ type Foo = { const f: readonly string[] = ["a", "b"]; ``` - + -- [1.7](#convension-ts-ignore) **@ts-ignore**: Do not use `@ts-ignore` or its variant `@ts-nocheck` to suppress warnings and errors. Use `@ts-expect-error` during the migration for type errors that should be handled later. +- [1.7](#ts-ignore) **@ts-ignore**: Do not use `@ts-ignore` or its variant `@ts-nocheck` to suppress warnings and errors. - + > Use `@ts-expect-error` during the migration for type errors that should be handled later. Refer to the [Migration Guidelines](#migration-guidelines) for specific instructions on how to deal with type errors during the migration. eslint: [`@typescript-eslint/ban-ts-comment`](https://typescript-eslint.io/rules/ban-ts-comment/) -- [1.8](#convension-ts-nullish-coalescing) **Optional chaining and nullish coalescing**: Use optional chaining and nullish coalescing instead of the `get` lodash function. + + +- [1.8](#ts-nullish-coalescing) **Optional chaining and nullish coalescing**: Use optional chaining and nullish coalescing instead of the `get` lodash function. eslint: [`no-restricted-imports`](https://eslint.org/docs/latest/rules/no-restricted-imports) ```ts // Bad - import { get } from "lodash"; + import lodashGet from "lodash/get"; const name = lodashGet(user, "name", "default name"); // Good const name = user?.name ?? "default name"; ``` - + -- [1.9](#convension-type-inference) **Type Inference**: When possible, allow the compiler to infer type of variables. +- [1.9](#type-inference) **Type Inference**: When possible, allow the compiler to infer type of variables. ```ts // Bad @@ -219,9 +224,9 @@ type Foo = { } ``` - + -- [1.10](#conventions-jsdoc) **JSDoc**: Omit comments that are redundant with TypeScript. Do not declare types in `@param` or `@return` blocks. Do not write `@implements`, `@enum`, `@private`, `@override` +- [1.10](#jsdoc) **JSDoc**: Omit comments that are redundant with TypeScript. Do not declare types in `@param` or `@return` blocks. Do not write `@implements`, `@enum`, `@private`, `@override`. eslint: [`jsdoc/no-types`](https://github.com/gajus/eslint-plugin-jsdoc/blob/main/.README/rules/no-types.md) ```ts // bad @@ -240,9 +245,9 @@ type Foo = { */ ``` - + -- [1.11](#convension-proptypes-and-defaultprops) **`propTypes` and `defaultProps`**: Do not use them. Use object destructing to assign default values if necessary. +- [1.11](#proptypes-and-defaultprops) **`propTypes` and `defaultProps`**: Do not use them. Use object destructing to assign default values if necessary. > Refer to [the propTypes Migration Table](./PROPTYPES_CONVERSION_TABLE.md) on how to type props based on existing `propTypes`. @@ -262,9 +267,9 @@ type Foo = { } ``` - + -- [1.12](#convension-utility-types) **Utility Types**: Use types from [TypeScript utility types](https://www.typescriptlang.org/docs/handbook/utility-types.html) and [`type-fest`](https://github.com/sindresorhus/type-fest) when possible. +- [1.12](#utility-types) **Utility Types**: Use types from [TypeScript utility types](https://www.typescriptlang.org/docs/handbook/utility-types.html) and [`type-fest`](https://github.com/sindresorhus/type-fest) when possible. ```ts type Foo = { @@ -280,9 +285,9 @@ type Foo = { }; ``` - + -- [1.13](#convension-object-type) **`object`**: Don't use `object` type. +- [1.13](#object-type) **`object`**: Don't use `object` type. eslint: [`@typescript-eslint/ban-types`](https://typescript-eslint.io/rules/ban-types/) > Why? `object` refers to "any non-primitive type," not "any object". Typing "any non-primitive value" is not commonly needed. @@ -293,7 +298,7 @@ type Foo = { If you know that the type of data is an object but don't know what properties or values it has beforehand, use `Record`. - > Even though `string` is specified as a key, `Record` type can still accepts objects whose keys are numbers or symbols. This is because number and + > Even though `string` is specified as a key, `Record` type can still accepts objects whose keys are numbers. This is because numbers are converted to strings when used as an object index. Note that you cannot use [symbols](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol) for `Record`. ```ts function logObject(object: Record) { @@ -303,9 +308,9 @@ type Foo = { } ``` - + -- [1.14](#convension-export-prop-types) **Prop Types**: Define and export prop types for components. Use exported prop types instead of grabbing the prop type from a component. +- [1.14](#export-prop-types) **Prop Types**: Define and export prop types for components. Use exported prop types instead of grabbing the prop type from a component. > Why? Exporting prop types aids reusability. @@ -328,11 +333,11 @@ type Foo = { import MyComponent, { MyComponentProps } from "./MyComponent"; ``` - + -- [1.15](#convension-file-organization) **File organization**: In modules with platform-specific implementations, create `types.ts` to define shared types. Import and use shared types in each platform specific files. +- [1.15](#file-organization) **File organization**: In modules with platform-specific implementations, create `types.ts` to define shared types. Import and use shared types in each platform specific files. - > Why? To encourage consistent API across platform-specific implementations. + > Why? To encourage consistent API across platform-specific implementations. If you're migrating module that doesn't have a default implement (i.e. `index.ts`, e.g. `getPlatform`), refer to [Migration Guidelines](#migration-guidelines) for further information. Utility module example @@ -395,15 +400,15 @@ type Foo = { -- [1.16] **Reusable Types**: Reusable type definitions, such as models (e.g. Report), must have their own file and be placed under `src/types/`. The type should be exported as a default export. +- [1.16](#reusable-types) **Reusable Types**: Reusable type definitions, such as models (e.g. Report), must have their own file and be placed under `src/types/`. The type should be exported as a default export. - ```ts - // src/types/Report.ts +```ts +// src/types/Report.ts - type Report = {...}; +type Report = {...}; - export default Report; - ``` +export default Report; +``` ## Communication Items @@ -426,18 +431,21 @@ declare module "external-library-name" { > This section contains instructions that are applicable during the migration. +- If you're migrating a module that doesn't have a default implementation (i.e. `index.ts`, e.g. `getPlatform`), convert `index.website.js` to `index.ts`. Without `index.ts`, TypeScript cannot get type information where the module is imported. + +- Deprecate the usage of `underscore`. Use corresponding methods from `lodash`. eslint: [`no-restricted-imports`](https://eslint.org/docs/latest/rules/no-restricted-imports) + - Found type bugs. Now what? If TypeScript migration uncovers a bug that has been “invisible,” there are two options an author of a migration PR can take - Fix issues if they are minor. Document each fix in the PR comment - - Suppress a TypeScript error stemming from the bug with `@ts-expect-error`. Create a separate GH issue. Prefix the issue title with `[TS ERROR #]`. Cross-link the migration PR and the created GH issue. On the line below `@ts-expect-error`, put down the GH issue number prefixed with `TODO:`. + - Suppress a TypeScript error stemming from the bug with `@ts-expect-error`. Create a separate GH issue. Prefix the issue title with `[TS ERROR #]`. Cross-link the migration PR and the created GH issue. On the same line as `@ts-expect-error`, put down the GH issue number prefixed with `TODO:`. - The `@ts-expect-error` annotation tells the TS compiler to ignore any errors in the line that follows it. However, if there's no error in the line, TypeScript will also raise an error. + > The `@ts-expect-error` annotation tells the TS compiler to ignore any errors in the line that follows it. However, if there's no error in the line, TypeScript will also raise an error. ```ts - // @ts-expect-error - // TODO: #21647 + // @ts-expect-error TODO: #21647 const x: number = "123"; // No TS error raised // @ts-expect-error From 0a0bf3eaf8840821779eafdae39f694c49eb271a Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Mon, 3 Jul 2023 00:08:02 -0700 Subject: [PATCH 08/20] specific path --- contributingGuides/TS_STYLE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contributingGuides/TS_STYLE.md b/contributingGuides/TS_STYLE.md index c113cf44213b..7523f00e8e0f 100644 --- a/contributingGuides/TS_STYLE.md +++ b/contributingGuides/TS_STYLE.md @@ -416,7 +416,7 @@ export default Report; - I think types definitions in a third party library is incomplete or incorrect -When the library indeed contains incorrect type definitions and it cannot be updated, use module augmentation to correct them. All module augmentation code should be contained in `global.d.ts`. +When the library indeed contains incorrect type definitions and it cannot be updated, use module augmentation to correct them. All module augmentation code should be contained in `/src/global.d.ts`. ```ts declare module "external-library-name" { From bf2005ff30da57dc830fa06b2329a15f64a759ba Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Mon, 3 Jul 2023 00:16:57 -0700 Subject: [PATCH 09/20] add tsx section --- contributingGuides/TS_STYLE.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/contributingGuides/TS_STYLE.md b/contributingGuides/TS_STYLE.md index 7523f00e8e0f..9299542c81fe 100644 --- a/contributingGuides/TS_STYLE.md +++ b/contributingGuides/TS_STYLE.md @@ -23,6 +23,7 @@ - [1.14 Export Prop Types](#export-prop-types) - [1.15 File Organization](#file-organization) - [1.16 Reusable Types](#reusable-types) + - [1.17 `.tsx`](#tsx) - [Communication Items](#items) - [Migration Guidelines](#migration-guidelines) @@ -410,6 +411,12 @@ type Report = {...}; export default Report; ``` + + +- [1.17](#tsx) **tsx** Use `.tsx` extension for files that contain React syntax. + +> Why? It is a widely adopted convention to mark any files that contain React specific syntax with `.jsx` or `.tsx`. + ## Communication Items > Comment in the `#expensify-open-source` Slack channel if any of the following situations are encountered. Each comment should be prefixed with `TS ATTENTION:`. Internal engineers will access each situation and prescribe solutions to each case. Internal engineers should refer to general solutions to each situation that follows each list item. From 6d9bf6d7b5ee64af67961503b781518774f127f9 Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Mon, 3 Jul 2023 21:51:01 -0700 Subject: [PATCH 10/20] add more guidelines on naming convention and prop types --- contributingGuides/TS_STYLE.md | 101 +++++++++++++++++++++++++-------- 1 file changed, 78 insertions(+), 23 deletions(-) diff --git a/contributingGuides/TS_STYLE.md b/contributingGuides/TS_STYLE.md index 9299542c81fe..0c43525cb477 100644 --- a/contributingGuides/TS_STYLE.md +++ b/contributingGuides/TS_STYLE.md @@ -24,6 +24,7 @@ - [1.15 File Organization](#file-organization) - [1.16 Reusable Types](#reusable-types) - [1.17 `.tsx`](#tsx) + - [1.18 No inline prop types](#no-inline-prop-types) - [Communication Items](#items) - [Migration Guidelines](#migration-guidelines) @@ -69,21 +70,56 @@ type Foo = { -- [1.1](#naming-conventions) **Naming Conventions**: Use PascalCase for type names. Do not postfix type aliases with `Type`. Use singular name for union types. eslint: [`@typescript-eslint/naming-convention`](https://typescript-eslint.io/rules/naming-convention/) +- [1.1](#naming-conventions) **Naming Conventions**: Follow naming conventions specified below - ```ts - // bad - type foo = ...; - type BAR = ...; - type PersonType = ...; - type Colors = 'red' | 'blue' | 'green'; + - Use PascalCase for type names. eslint: [`@typescript-eslint/naming-convention`](https://typescript-eslint.io/rules/naming-convention/) - // good - type Foo = ...; - type Bar = ...; - type Person = ...; - type Color = 'red' | 'blue' | 'green'; - ``` + ```ts + // bad + type foo = ...; + type BAR = ...; + + // good + type Foo = ...; + type Bar = ...; + ``` + + - Do not postfix type aliases with `Type`. + + ```ts + // bad + type PersonType = ...; + + // good + type Person = ...; + ``` + + - Use singular name for union types. + + ```ts + // bad + type Colors = "red" | "blue" | "green"; + + // good + type Color = "red" | "blue" | "green"; + ``` + + - For generic type parameters, use `T` if you have only one type parameter. Don't use the `T`, `U`, `V`... sequence. Make type parameter names descriptive, each prefixed with `T`. + + > Prefix each type parameter name to distinguish them from other types. + + ```ts + // bad + type KeyValuePair = { key: K; value: U }; + + type Keys = Array; + + // good + type KeyValuePair = { key: TKey; value: TValue }; + + type Keys = Array; + type Keys = Array; + ``` @@ -311,9 +347,9 @@ type Foo = { -- [1.14](#export-prop-types) **Prop Types**: Define and export prop types for components. Use exported prop types instead of grabbing the prop type from a component. +- [1.14](#export-prop-types) **Prop Types**: Don't use `ComponentProps` to grab a component's prop types. Go to the source file for the component and export prop types from there. Import and use the exported prop types. - > Why? Exporting prop types aids reusability. + > Don't export prop types from component files by default. Only export it when there is a code that needs to access the prop type directly. ```tsx // MyComponent.tsx @@ -403,19 +439,38 @@ type Foo = { - [1.16](#reusable-types) **Reusable Types**: Reusable type definitions, such as models (e.g. Report), must have their own file and be placed under `src/types/`. The type should be exported as a default export. -```ts -// src/types/Report.ts + ```ts + // src/types/Report.ts -type Report = {...}; + type Report = {...}; -export default Report; -``` + export default Report; + ``` + + + +- [1.17](#tsx) **tsx**: Use `.tsx` extension for files that contain React syntax. + + > Why? It is a widely adopted convention to mark any files that contain React specific syntax with `.jsx` or `.tsx`. - + -- [1.17](#tsx) **tsx** Use `.tsx` extension for files that contain React syntax. +- [1.18](#no-inline-prop-types) **No inline prop tpe**: Do not define prop types inline for components that are exported. -> Why? It is a widely adopted convention to mark any files that contain React specific syntax with `.jsx` or `.tsx`. + > Why? Prop types might need to be exported from component files. //TODO: link to the export component types section. If the component is only used inside a file or module and not exported, then inline prop types can be used. + + ```ts + // bad + export default function MyComponent({ foo, bar }: { foo: string, bar: number }){ + // component implementation + }; + + // good + type MyComponentProp = { foo: string, bar: number }; + export default MyComponent({ foo, bar }: MyComponentProp){ + // component implementation + } + ``` ## Communication Items From 0dd70cda8d5519b0601a9087e27e0c335cfce1f8 Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Tue, 4 Jul 2023 10:12:25 -0700 Subject: [PATCH 11/20] fix typos --- contributingGuides/TS_STYLE.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contributingGuides/TS_STYLE.md b/contributingGuides/TS_STYLE.md index 0c43525cb477..2755e6120712 100644 --- a/contributingGuides/TS_STYLE.md +++ b/contributingGuides/TS_STYLE.md @@ -41,7 +41,7 @@ - [TypeScript playground](https://www.typescriptlang.org/play?q=231#example) - Go though all examples on the playground. Click on "Example" tab on the top - Handy Reference - - [TypeScript CheatSheet](https://www.typescriptlang.org/static/TypeScript%20Types-ae199d69aeecf7d4a2704a528d0fd3f9.png) + - [TypeScript CheatSheet](https://www.typescriptlang.org/cheatsheets) - [Type](https://www.typescriptlang.org/static/TypeScript%20Types-ae199d69aeecf7d4a2704a528d0fd3f9.png) - [Control Flow Analysis](https://www.typescriptlang.org/static/TypeScript%20Control%20Flow%20Analysis-8a549253ad8470850b77c4c5c351d457.png) - TypeScript with React @@ -88,7 +88,7 @@ type Foo = { ```ts // bad - type PersonType = ...; + type PersonType = ...; // good type Person = ...; @@ -455,7 +455,7 @@ type Foo = { -- [1.18](#no-inline-prop-types) **No inline prop tpe**: Do not define prop types inline for components that are exported. +- [1.18](#no-inline-prop-types) **No inline prop types**: Do not define prop types inline for components that are exported. > Why? Prop types might need to be exported from component files. //TODO: link to the export component types section. If the component is only used inside a file or module and not exported, then inline prop types can be used. From 593ba9ef3ea924e12438ba8d659c4f603e254629 Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Sun, 9 Jul 2023 23:44:39 -0700 Subject: [PATCH 12/20] fix typos & improve clarity --- contributingGuides/TS_STYLE.md | 76 +++++++++++++++++----------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/contributingGuides/TS_STYLE.md b/contributingGuides/TS_STYLE.md index 2755e6120712..5afe4470bcf6 100644 --- a/contributingGuides/TS_STYLE.md +++ b/contributingGuides/TS_STYLE.md @@ -25,7 +25,7 @@ - [1.16 Reusable Types](#reusable-types) - [1.17 `.tsx`](#tsx) - [1.18 No inline prop types](#no-inline-prop-types) -- [Communication Items](#items) +- [Communication Items](#communication-items) - [Migration Guidelines](#migration-guidelines) ## Other Expensify Resources on TypeScript @@ -75,11 +75,11 @@ type Foo = { - Use PascalCase for type names. eslint: [`@typescript-eslint/naming-convention`](https://typescript-eslint.io/rules/naming-convention/) ```ts - // bad + // BAD type foo = ...; type BAR = ...; - // good + // GOOD type Foo = ...; type Bar = ...; ``` @@ -87,20 +87,20 @@ type Foo = { - Do not postfix type aliases with `Type`. ```ts - // bad + // BAD type PersonType = ...; - // good + // GOOD type Person = ...; ``` - Use singular name for union types. ```ts - // bad + // BAD type Colors = "red" | "blue" | "green"; - // good + // GOOD type Color = "red" | "blue" | "green"; ``` @@ -109,12 +109,12 @@ type Foo = { > Prefix each type parameter name to distinguish them from other types. ```ts - // bad + // BAD type KeyValuePair = { key: K; value: U }; type Keys = Array; - // good + // GOOD type KeyValuePair = { key: TKey; value: TValue }; type Keys = Array; @@ -123,7 +123,7 @@ type Foo = { -- [1.2](#d-ts-extension) **`d.ts` Extension**: Do not use `d.ts` file extension even when a file contains only type declarations. +- [1.2](#d-ts-extension) **`d.ts` Extension**: Do not use `d.ts` file extension even when a file contains only type declarations. Only exception is the `global.d.ts` file in which third party packages can be modified using module augmentation. Refer to the [Communication Items](#communication-items) section to learn more about module augmentation. > Why? Type errors in `d.ts` files are not checked by TypeScript [^1]. @@ -136,12 +136,12 @@ type Foo = { > Why? In TypeScript, `type` and `interface` can be used interchangeably to declare types. Use `type` for consistency. ```ts - // bad + // BAD interface Person { name: string; } - // good + // GOOD type Person = { name: string; }; @@ -223,11 +223,11 @@ type Foo = { - [1.8](#ts-nullish-coalescing) **Optional chaining and nullish coalescing**: Use optional chaining and nullish coalescing instead of the `get` lodash function. eslint: [`no-restricted-imports`](https://eslint.org/docs/latest/rules/no-restricted-imports) ```ts - // Bad + // BAD import lodashGet from "lodash/get"; const name = lodashGet(user, "name", "default name"); - // Good + // GOOD const name = user?.name ?? "default name"; ``` @@ -236,14 +236,14 @@ type Foo = { - [1.9](#type-inference) **Type Inference**: When possible, allow the compiler to infer type of variables. ```ts - // Bad + // BAD const foo: string = "foo"; const [counter, setCounter] = useState(0); - // Good + // GOOD const foo = "foo"; const [counter, setCounter] = useState(0); - const [username, setUsername] = useState(undefined); // Username is a union type of string and undefined, and its type cannot be interred from the default value of undefined + const [username, setUsername] = useState(undefined); // Username is a union type of string and undefined, and its type cannot be inferred from the default value of undefined ``` For function return types, default to always typing them unless a function is simple enough to reason about its return type. @@ -266,7 +266,7 @@ type Foo = { - [1.10](#jsdoc) **JSDoc**: Omit comments that are redundant with TypeScript. Do not declare types in `@param` or `@return` blocks. Do not write `@implements`, `@enum`, `@private`, `@override`. eslint: [`jsdoc/no-types`](https://github.com/gajus/eslint-plugin-jsdoc/blob/main/.README/rules/no-types.md) ```ts - // bad + // BAD /** * @param {number} age * @returns {boolean} Whether the person is a legal drinking age or nots @@ -275,7 +275,7 @@ type Foo = { return age >= 21; } - // good + // GOOD /** * @param age * @returns Whether the person is a legal drinking age or nots @@ -313,13 +313,13 @@ type Foo = { bar: string; }; - // good - type ReadOnlyFoo = Readonly; - - // bad + // BAD type ReadOnlyFoo = { readonly [Property in keyof Foo]: Foo[Property]; }; + + // GOOD + type ReadOnlyFoo = Readonly; ``` @@ -329,7 +329,7 @@ type Foo = { > Why? `object` refers to "any non-primitive type," not "any object". Typing "any non-primitive value" is not commonly needed. ```ts - // bad + // BAD const foo: object = [1, 2, 3]; // TypeScript does not error ``` @@ -361,12 +361,12 @@ type Foo = { return {foo}; } - // bad + // BAD import { ComponentProps } from "React"; import MyComponent from "./MyComponent"; type MyComponentProps = ComponentProps; - // good + // GOOD import MyComponent, { MyComponentProps } from "./MyComponent"; ``` @@ -386,7 +386,7 @@ type Foo = { }; // index.native.ts - import { GreetingModule } from "./types.ts"; + import { GreetingModule } from "./types"; function getHello() { return "hello from mobile code"; } @@ -400,7 +400,7 @@ type Foo = { export default Greeting; // index.ts - import { GreetingModule } from "./types.ts"; + import { GreetingModule } from "./types"; function getHello() { return "hello from other platform code"; } @@ -423,13 +423,13 @@ type Foo = { } // index.ios.ts - import { MyComponentProps } from ./types.ts; + import { MyComponentProps } from "./types"; export MyComponentProps; export default function MyComponent({ foo }: MyComponentProps) {...} // index.ts - import { MyComponentProps } from ./types.ts; + import { MyComponentProps } from "./types"; export MyComponentProps; export default function MyComponent({ foo }: MyComponentProps) {...} @@ -437,7 +437,7 @@ type Foo = { -- [1.16](#reusable-types) **Reusable Types**: Reusable type definitions, such as models (e.g. Report), must have their own file and be placed under `src/types/`. The type should be exported as a default export. +- [1.16](#reusable-types) **Reusable Types**: Reusable type definitions, such as models (e.g., Report), must have their own file and be placed under `src/types/`. The type should be exported as a default export. ```ts // src/types/Report.ts @@ -457,17 +457,17 @@ type Foo = { - [1.18](#no-inline-prop-types) **No inline prop types**: Do not define prop types inline for components that are exported. - > Why? Prop types might need to be exported from component files. //TODO: link to the export component types section. If the component is only used inside a file or module and not exported, then inline prop types can be used. + > Why? Prop types might [need to be exported from component files](#export-prop-types). If the component is only used inside a file or module and not exported, then inline prop types can be used. ```ts - // bad + // BAD export default function MyComponent({ foo, bar }: { foo: string, bar: number }){ // component implementation }; - // good - type MyComponentProp = { foo: string, bar: number }; - export default MyComponent({ foo, bar }: MyComponentProp){ + // GOOD + type MyComponentProps = { foo: string, bar: number }; + export default MyComponent({ foo, bar }: MyComponentProps){ // component implementation } ``` @@ -478,7 +478,7 @@ type Foo = { - I think types definitions in a third party library is incomplete or incorrect -When the library indeed contains incorrect type definitions and it cannot be updated, use module augmentation to correct them. All module augmentation code should be contained in `/src/global.d.ts`. +When the library indeed contains incorrect or missing type definitions and it cannot be updated, use module augmentation to correct them. All module augmentation code should be contained in `/src/global.d.ts`. ```ts declare module "external-library-name" { @@ -495,7 +495,7 @@ declare module "external-library-name" { - If you're migrating a module that doesn't have a default implementation (i.e. `index.ts`, e.g. `getPlatform`), convert `index.website.js` to `index.ts`. Without `index.ts`, TypeScript cannot get type information where the module is imported. -- Deprecate the usage of `underscore`. Use corresponding methods from `lodash`. eslint: [`no-restricted-imports`](https://eslint.org/docs/latest/rules/no-restricted-imports) +- Deprecate the usage of `underscore`. Use the corresponding methods from `lodash`. eslint: [`no-restricted-imports`](https://eslint.org/docs/latest/rules/no-restricted-imports) - Found type bugs. Now what? From 18b15227d8d4e1afd25d2c0cf753c7acba622787 Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Sun, 9 Jul 2023 23:45:25 -0700 Subject: [PATCH 13/20] add sections & change order of sections --- contributingGuides/TS_CHEATSHEET.md | 99 +++++++++++++++++------------ 1 file changed, 60 insertions(+), 39 deletions(-) diff --git a/contributingGuides/TS_CHEATSHEET.md b/contributingGuides/TS_CHEATSHEET.md index fa21abf140b6..e336697e773a 100644 --- a/contributingGuides/TS_CHEATSHEET.md +++ b/contributingGuides/TS_CHEATSHEET.md @@ -2,14 +2,17 @@ ## Table of Contents -- [1.1 `props.children`](#children-prop) -- [1.2 `forwardRef`](#forwardRef) -- [1.3 Animated styles](#animated-style) -- [1.4 Style Props](#style-props) -- [1.5 Render Prop](#render-prop) -- [1.6 Type Narrowing](#type-narrowing) -- [1.7 Errors in Try-Catch Clauses](#try-catch-clauses) -- [1.8 Const Assertion](#const-assertion) +- [CheatSheet](#cheatsheet) + - [1.1 `props.children`](#children-prop) + - [1.2 `forwardRef`](#forwardRef) + - [1.3 Style Props](#style-props) + - [1.4 Animated styles](#animated-style) + - [1.5 Render Prop](#render-prop) + - [1.6 Type Narrowing](#type-narrowing) + - [1.7 Errors in Try-Catch Clauses](#try-catch-clauses) + - [1.8 Const Assertion](#const-assertion) + - [1.9 Higher Order Components](#higher-order-components) + - [1.10 Function Overloading](#function-overloading) ## CheatSheet @@ -22,7 +25,7 @@ children?: React.ReactNode; }; - function WrapperComponent({ children }: Props) { + function WrapperComponent({ children }: WrapperComponentProps) { return {children}; } @@ -43,12 +46,12 @@ import { forwardRef, useRef, ReactNode } from "react"; import { TextInput, View } from "react-native"; - export type CustomButtonProps = { + export type CustomTextInputProps = { label: string; children?: ReactNode; }; - const CustomTextInput = forwardRef( + const CustomTextInput = forwardRef( (props, ref) => { return ( @@ -60,36 +63,14 @@ ); function ParentComponent() { - const ref = useRef; + const ref = useRef(); return ; } ``` - + -- [1.3](#animated-style) **Animated styles** - - ```ts - import {useRef} from 'react'; - import {Animated, StyleProp, ViewStyle} from 'react-native'; - - type MyComponentProps = { - style?: Animated.WithAnimatedValue>; - }; - - function MyComponent({ style }: Props) { - return ; - } - - function MyComponent() { - const anim = useRef(new Animated.Value(0)).current; - return ; - } - ``` - - - -- [1.4](#style-props) **Style Props** +- [1.3](#style-props) **Style Props** Use `StyleProp` to type style props. For pass-through style props, use types exported from `react-native` for the type parameter (e.g. `ViewStyle`). @@ -102,7 +83,7 @@ imageStyle?: StyleProp; }; - function MyComponentProps({ containerStyle, textStyle, imageStyle }: MyComponentProps) = { + function MyComponent({ containerStyle, textStyle, imageStyle }: MyComponentProps) = { Sample Image @@ -110,6 +91,28 @@ } ``` + + +- [1.4](#animated-style) **Animated styles** + + ```ts + import {useRef} from 'react'; + import {Animated, StyleProp, ViewStyle} from 'react-native'; + + type MyComponentProps = { + style?: Animated.WithAnimatedValue>; + }; + + function MyComponent({ style }: MyComponentProps) { + return ; + } + + function MyComponent() { + const anim = useRef(new Animated.Value(0)).current; + return ; + } + ``` + - [1.5](#render-prop) **Render Prop** @@ -138,7 +141,7 @@ -- [1.6](#type-narrowing) **Type Narrowing** Narrow types down using `typeof` or custom type guards. +- [1.6](#type-narrowing) **Type Narrowing** Narrow types down using `typeof`, discriminated unions, or custom type guards. Refer [this guide](https://medium.com/@hayata.suenaga/discriminated-unions-custom-type-guards-182ebe1f92fb) for more information on when to use discriminated unions and custom type guards. ```ts type Manager = { @@ -199,7 +202,7 @@ -- [1.8](#const-assersion) **Use const assertions for rigorous typing** +- [1.8](#const-assertion) **Use const assertions for rigorous typing** Use `as const` when you want to ensure that the types and values are as exact as possible and prevent unwanted mutations. @@ -213,3 +216,21 @@ const array1 = ["hello", 1]; // type: (string | number)[] const array2 = ["goodbye", 2]; // type: readonly ["goodbye", 2] ``` + + + +- [1.9](#higher-order-components) **Higher Order Components** + + Typing HOCs is hard. Refer to [this articles](https://medium.com/@hayata.suenaga/ts-higher-order-components-30c38dd19ae8) for detailed guideline on typing HOCs for different usages of HOCs. + + + +- [1.10](#function-overloading) **Function Overloading** + + Use [function overloads](https://www.typescriptlang.org/docs/handbook/2/functions.html#function-overloads) to provide more type information for functions. For the following types of functions, function overloading can be beneficial. + + - The return type depends on the input type + - When function accept different number of parameters + - There are type dependencies between parameters + + Refer to [this guide](https://medium.com/@hayata.suenaga/when-to-use-function-overloads-acc48f7e3142) to learn how to use functional overloads for each situation. From 01d32343ce9f1ffecf288001aaf6b01ce0ea3bc4 Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Tue, 11 Jul 2023 09:20:06 -0700 Subject: [PATCH 14/20] fix typos --- contributingGuides/PROPTYPES_CONVERSION_TABLE.md | 2 +- contributingGuides/TS_CHEATSHEET.md | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/contributingGuides/PROPTYPES_CONVERSION_TABLE.md b/contributingGuides/PROPTYPES_CONVERSION_TABLE.md index c45e71218df1..d4890d32bdb3 100644 --- a/contributingGuides/PROPTYPES_CONVERSION_TABLE.md +++ b/contributingGuides/PROPTYPES_CONVERSION_TABLE.md @@ -1,4 +1,4 @@ -# Expensify PropTypes Conversation Table +# Expensify PropTypes Conversion Table ## Table of Contents diff --git a/contributingGuides/TS_CHEATSHEET.md b/contributingGuides/TS_CHEATSHEET.md index e336697e773a..3baf8bec0b7e 100644 --- a/contributingGuides/TS_CHEATSHEET.md +++ b/contributingGuides/TS_CHEATSHEET.md @@ -187,7 +187,7 @@ - [1.7](#try-catch-clauses) **Error in Try-Catch Clauses** - Errors in try/catch clauses are inferred as `unknown`. If the error dat needs to be accessed, the type of the error needs to be checked and narrowed down. + Errors in try/catch clauses are inferred as `unknown`. If the error data needs to be accessed, the type of the error needs to be checked and narrowed down. ```ts try { @@ -211,7 +211,7 @@ const greeting2 = "goodbye" as const; // type: "goodbye" const person1 = { name: "Alice", age: 20 }; // type: { name: string, age: number } - const person2 = { name: "Bob", age: 30 } as const; // type: { readonly name: "Bob", readonly age; 30 } + const person2 = { name: "Bob", age: 30 } as const; // type: { readonly name: "Bob", readonly age: 30 } const array1 = ["hello", 1]; // type: (string | number)[] const array2 = ["goodbye", 2]; // type: readonly ["goodbye", 2] @@ -230,7 +230,7 @@ Use [function overloads](https://www.typescriptlang.org/docs/handbook/2/functions.html#function-overloads) to provide more type information for functions. For the following types of functions, function overloading can be beneficial. - The return type depends on the input type - - When function accept different number of parameters + - When function accepts different number of parameters - There are type dependencies between parameters Refer to [this guide](https://medium.com/@hayata.suenaga/when-to-use-function-overloads-acc48f7e3142) to learn how to use functional overloads for each situation. From e0826fd76bbd76fcb20bee490aedd219a341d093 Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Tue, 11 Jul 2023 12:36:26 -0700 Subject: [PATCH 15/20] change section order and improve code examples --- contributingGuides/TS_STYLE.md | 54 +++++++++++++++++----------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/contributingGuides/TS_STYLE.md b/contributingGuides/TS_STYLE.md index 5afe4470bcf6..26a25c4428e7 100644 --- a/contributingGuides/TS_STYLE.md +++ b/contributingGuides/TS_STYLE.md @@ -3,8 +3,6 @@ ## Table of Contents - [Other Expensify Resources on TypeScript](#other-expensify-resources-on-typescript) -- [Learning Resources](#learning-resources) -- [Exception to Rules](#exception-to-rules) - [General Rules](#general-rules) - [Guidelines](#guidelines) - [1.1 Naming Conventions](#naming-conventions) @@ -25,36 +23,16 @@ - [1.16 Reusable Types](#reusable-types) - [1.17 `.tsx`](#tsx) - [1.18 No inline prop types](#no-inline-prop-types) +- [Exception to Rules](#exception-to-rules) - [Communication Items](#communication-items) - [Migration Guidelines](#migration-guidelines) +- [Learning Resources](#learning-resources) ## Other Expensify Resources on TypeScript - [Expensify TypeScript React Native CheatSheet](./TS_CHEATSHEET.md) - [Expensify TypeScript PropTypes Conversion Table](./PROPTYPES_CONVERSION_TABLE.md) -## Learning Resources - -### Quickest way to learn TypeScript - -- Get up to speed quickly - - [TypeScript playground](https://www.typescriptlang.org/play?q=231#example) - - Go though all examples on the playground. Click on "Example" tab on the top -- Handy Reference - - [TypeScript CheatSheet](https://www.typescriptlang.org/cheatsheets) - - [Type](https://www.typescriptlang.org/static/TypeScript%20Types-ae199d69aeecf7d4a2704a528d0fd3f9.png) - - [Control Flow Analysis](https://www.typescriptlang.org/static/TypeScript%20Control%20Flow%20Analysis-8a549253ad8470850b77c4c5c351d457.png) -- TypeScript with React - - [React TypeScript CheatSheet](https://react-typescript-cheatsheet.netlify.app/) - - [List of built-in utility types](https://react-typescript-cheatsheet.netlify.app/docs/basic/troubleshooting/utilities) - - [HOC CheatSheet](https://react-typescript-cheatsheet.netlify.app/docs/hoc/) - -## Exception to Rules - -Most of the rules are enforced in ESLint or checked by TypeScript. If you think your particular situation warrants an exception, post the context in the `#expensify-open-source` Slack channel with your message prefixed with `TS EXCEPTION:`. Internal engineers will assess the case and suggest alternative or grants an exception. When an exception is granted, link the relevant Slack conversation in your pull request. Suppress ESLint or TypeScript warnings/errors with comments if necessary. - -This rule will apply until the migration is done. After the migration, exceptions are assessed and granted by PR reviewers. - ## General Rules Strive to type as strictly as possible. @@ -166,7 +144,7 @@ type Foo = { const COLORS = ["red", "green", "blue"] as const; type Color = TupleToUnion; // type: 'red' | 'green' | 'blue' - for (const colors of color) { + for (const color of COLORS) { printColor(color); } @@ -426,13 +404,13 @@ type Foo = { import { MyComponentProps } from "./types"; export MyComponentProps; - export default function MyComponent({ foo }: MyComponentProps) {...} + export default function MyComponent({ foo }: MyComponentProps) { /* ios specific implementation */ } // index.ts import { MyComponentProps } from "./types"; export MyComponentProps; - export default function MyComponent({ foo }: MyComponentProps) {...} + export default function MyComponent({ foo }: MyComponentProps) { /* Default implementation */ } ``` @@ -472,6 +450,12 @@ type Foo = { } ``` +## Exception to Rules + +Most of the rules are enforced in ESLint or checked by TypeScript. If you think your particular situation warrants an exception, post the context in the `#expensify-open-source` Slack channel with your message prefixed with `TS EXCEPTION:`. Internal engineers will assess the case and suggest alternative or grants an exception. When an exception is granted, link the relevant Slack conversation in your pull request. Suppress ESLint or TypeScript warnings/errors with comments if necessary. + +This rule will apply until the migration is done. After the migration, exceptions are assessed and granted by PR reviewers. + ## Communication Items > Comment in the `#expensify-open-source` Slack channel if any of the following situations are encountered. Each comment should be prefixed with `TS ATTENTION:`. Internal engineers will access each situation and prescribe solutions to each case. Internal engineers should refer to general solutions to each situation that follows each list item. @@ -513,3 +497,19 @@ declare module "external-library-name" { // @ts-expect-error const y: number = 123; // TS error: Unused '@ts-expect-error' directive. ``` + +## Learning Resources + +### Quickest way to learn TypeScript + +- Get up to speed quickly + - [TypeScript playground](https://www.typescriptlang.org/play?q=231#example) + - Go though all examples on the playground. Click on "Example" tab on the top +- Handy Reference + - [TypeScript CheatSheet](https://www.typescriptlang.org/cheatsheets) + - [Type](https://www.typescriptlang.org/static/TypeScript%20Types-ae199d69aeecf7d4a2704a528d0fd3f9.png) + - [Control Flow Analysis](https://www.typescriptlang.org/static/TypeScript%20Control%20Flow%20Analysis-8a549253ad8470850b77c4c5c351d457.png) +- TypeScript with React + - [React TypeScript CheatSheet](https://react-typescript-cheatsheet.netlify.app/) + - [List of built-in utility types](https://react-typescript-cheatsheet.netlify.app/docs/basic/troubleshooting/utilities) + - [HOC CheatSheet](https://react-typescript-cheatsheet.netlify.app/docs/hoc/) From a1021d252ffc9321617e98efabcae80d1b8953e6 Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Fri, 14 Jul 2023 11:10:18 -0700 Subject: [PATCH 16/20] edit guideline on rule exceptions & fix typos --- contributingGuides/TS_CHEATSHEET.md | 6 +++--- contributingGuides/TS_STYLE.md | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/contributingGuides/TS_CHEATSHEET.md b/contributingGuides/TS_CHEATSHEET.md index 3baf8bec0b7e..a8f4e88170b0 100644 --- a/contributingGuides/TS_CHEATSHEET.md +++ b/contributingGuides/TS_CHEATSHEET.md @@ -107,9 +107,9 @@ return ; } - function MyComponent() { + function App() { const anim = useRef(new Animated.Value(0)).current; - return ; + return ; } ``` @@ -221,7 +221,7 @@ - [1.9](#higher-order-components) **Higher Order Components** - Typing HOCs is hard. Refer to [this articles](https://medium.com/@hayata.suenaga/ts-higher-order-components-30c38dd19ae8) for detailed guideline on typing HOCs for different usages of HOCs. + Typing HOCs is hard. Refer to [this article](https://medium.com/@hayata.suenaga/ts-higher-order-components-30c38dd19ae8) for detailed guideline on typing HOCs for different usages of HOCs. diff --git a/contributingGuides/TS_STYLE.md b/contributingGuides/TS_STYLE.md index 26a25c4428e7..a1b925f37383 100644 --- a/contributingGuides/TS_STYLE.md +++ b/contributingGuides/TS_STYLE.md @@ -452,9 +452,11 @@ type Foo = { ## Exception to Rules -Most of the rules are enforced in ESLint or checked by TypeScript. If you think your particular situation warrants an exception, post the context in the `#expensify-open-source` Slack channel with your message prefixed with `TS EXCEPTION:`. Internal engineers will assess the case and suggest alternative or grants an exception. When an exception is granted, link the relevant Slack conversation in your pull request. Suppress ESLint or TypeScript warnings/errors with comments if necessary. +Most of the rules are enforced in ESLint or checked by TypeScript. If you think your particular situation warrants an exception, post the context in the `#expensify-open-source` Slack channel with your message prefixed with `TS EXCEPTION:`. The internal engineer assigned to the PR should be the one that approves each exception, however all discussion regarding granting exceptions should happen in the public channel instead of the GitHub PR page so that the TS migration team can access them easily. -This rule will apply until the migration is done. After the migration, exceptions are assessed and granted by PR reviewers. +When an exception is granted, link the relevant Slack conversation in your PR. Suppress ESLint or TypeScript warnings/errors with comments if necessary. + +This rule will apply until the migration is done. After the migration, discussion on granting exception can happen inside the PR page and doesn't need take place in the Slack channel. ## Communication Items From 904c6d672c1d8fff42b9f391266dcf3433b9b490 Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Tue, 18 Jul 2023 13:33:30 -0700 Subject: [PATCH 17/20] address TS reviews --- contributingGuides/PROPTYPES_CONVERSION_TABLE.md | 15 +++++++++++++-- contributingGuides/TS_STYLE.md | 10 +++++++++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/contributingGuides/PROPTYPES_CONVERSION_TABLE.md b/contributingGuides/PROPTYPES_CONVERSION_TABLE.md index d4890d32bdb3..79794e7d90ad 100644 --- a/contributingGuides/PROPTYPES_CONVERSION_TABLE.md +++ b/contributingGuides/PROPTYPES_CONVERSION_TABLE.md @@ -36,12 +36,12 @@ type Props = { | PropTypes | TypeScript | Instructions | | -------------------------------------------------------------------- | --------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `PropTypes.any` | `T`, `Record` or `any` | Figure out what would be the correct data type and use it.

If you know that it's a object but isn't possible to determine the internal structure, use `Record`. | -| `PropTypes.array` or `PropTypes.arrayOf(T)` | `T[]` or `Array` | Convert to `T[]` or `Array`, where `T` is the data type of the array.

If `T` isn't a primitive type, create a separate `type` for the object structure of your prop and use it. | +| `PropTypes.array` or `PropTypes.arrayOf(T)` | `T[]` or `Array` | Convert to `T[]`, where `T` is the data type of the array.

If `T` isn't a primitive type, create a separate `type` for the object structure of your prop and use it. | | `PropTypes.bool` | `boolean` | Convert to `boolean`. | | `PropTypes.func` | `(arg1: Type1, arg2, Type2...) => ReturnType` | Convert to the function signature. | | `PropTypes.number` | `number` | Convert to `number`. | | `PropTypes.object`, `PropTypes.shape(T)` or `PropTypes.exact(T)` | `T` | If `T` isn't a primitive type, create a separate `type` for the `T` object structure of your prop and use it.

If you want an object but isn't possible to determine the internal structure, use `Record`. | -| `PropTypes.objectOf(T)` | `Record` | Convert to a `Record` where `T` is the data type of your dictionary.

If `T` isn't a primitive type, create a separate `type` for the object structure and use it. | +| `PropTypes.objectOf(T)` | `Record` | Convert to a `Record` where `T` is the data type of values stored in the object.

If `T` isn't a primitive type, create a separate `type` for the object structure and use it. | | `PropTypes.string` | `string` | Convert to `string`. | | `PropTypes.node` | `React.ReactNode` | Convert to `React.ReactNode`. `ReactNode` includes `ReactElement` as well as other types such as `strings`, `numbers`, `arrays` of the same, `null`, and `undefined` In other words, anything that can be rendered in React is a `ReactNode`. | | `PropTypes.element` | `React.ReactElement` | Convert to `React.ReactElement`. | @@ -85,6 +85,16 @@ const propTypes = { icon: PropTypes.elementType, date: PropTypes.instanceOf(Date), size: PropTypes.oneOf(["small", "medium", "large"]), + + optionalString: PropTypes.string, + /** + * Note that all props listed above are technically optional because they lack the `isRequired` attribute. + * However, in most cases, props are actually required but the `isRequired` attribute is left out by mistake. + * + * For each prop that appears to be optional, determine whether the component implementation assumes that + * the prop has a value (making it non-optional) or not. Only those props that are truly optional should be + * labeled with a `?` in their type definition. + */ }; // After @@ -127,5 +137,6 @@ type Props = { icon: React.ElementType; date: Date; size: Size; + optionalString?: string; }; ``` diff --git a/contributingGuides/TS_STYLE.md b/contributingGuides/TS_STYLE.md index a1b925f37383..7a6282bc654c 100644 --- a/contributingGuides/TS_STYLE.md +++ b/contributingGuides/TS_STYLE.md @@ -243,6 +243,8 @@ type Foo = { - [1.10](#jsdoc) **JSDoc**: Omit comments that are redundant with TypeScript. Do not declare types in `@param` or `@return` blocks. Do not write `@implements`, `@enum`, `@private`, `@override`. eslint: [`jsdoc/no-types`](https://github.com/gajus/eslint-plugin-jsdoc/blob/main/.README/rules/no-types.md) + > Not all parameters or return values need to be listed in the JSDoc comment. If there is no comment accompanying the parameter or return value, omit it. + ```ts // BAD /** @@ -255,17 +257,23 @@ type Foo = { // GOOD /** - * @param age * @returns Whether the person is a legal drinking age or nots */ + function canDrink(age: number): boolean { + return age >= 21; + } ``` + In the above example, because the parameter age doesn't have any accompanying comment, it is completely omitted from the JSDoc. + - [1.11](#proptypes-and-defaultprops) **`propTypes` and `defaultProps`**: Do not use them. Use object destructing to assign default values if necessary. > Refer to [the propTypes Migration Table](./PROPTYPES_CONVERSION_TABLE.md) on how to type props based on existing `propTypes`. + > Assign a default value to each optional prop unless the default values is `undefined` or `null`. + ```tsx type MyComponentProps = { requiredProp: string; From 2fd543a804418f3c26f8df1b47c80249327df12e Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Wed, 19 Jul 2023 21:36:52 -0700 Subject: [PATCH 18/20] fix typos --- contributingGuides/TS_STYLE.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contributingGuides/TS_STYLE.md b/contributingGuides/TS_STYLE.md index 7a6282bc654c..965ce24a5357 100644 --- a/contributingGuides/TS_STYLE.md +++ b/contributingGuides/TS_STYLE.md @@ -264,7 +264,7 @@ type Foo = { } ``` - In the above example, because the parameter age doesn't have any accompanying comment, it is completely omitted from the JSDoc. + In the above example, because the parameter `age` doesn't have any accompanying comment, it is completely omitted from the JSDoc. @@ -272,7 +272,7 @@ type Foo = { > Refer to [the propTypes Migration Table](./PROPTYPES_CONVERSION_TABLE.md) on how to type props based on existing `propTypes`. - > Assign a default value to each optional prop unless the default values is `undefined` or `null`. + > Assign a default value to each optional prop unless the default values is `undefined`. ```tsx type MyComponentProps = { @@ -493,9 +493,9 @@ declare module "external-library-name" { - Found type bugs. Now what? - If TypeScript migration uncovers a bug that has been “invisible,” there are two options an author of a migration PR can take + If TypeScript migration uncovers a bug that has been “invisible,” there are two options an author of a migration PR can take: - - Fix issues if they are minor. Document each fix in the PR comment + - Fix issues if they are minor. Document each fix in the PR comment. - Suppress a TypeScript error stemming from the bug with `@ts-expect-error`. Create a separate GH issue. Prefix the issue title with `[TS ERROR #]`. Cross-link the migration PR and the created GH issue. On the same line as `@ts-expect-error`, put down the GH issue number prefixed with `TODO:`. > The `@ts-expect-error` annotation tells the TS compiler to ignore any errors in the line that follows it. However, if there's no error in the line, TypeScript will also raise an error. From 6450b6533fb4b8eec1bf7a95c0b57dc15bcbfcfd Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Wed, 19 Jul 2023 22:40:14 -0700 Subject: [PATCH 19/20] remove usage of any & fix typo --- contributingGuides/PROPTYPES_CONVERSION_TABLE.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contributingGuides/PROPTYPES_CONVERSION_TABLE.md b/contributingGuides/PROPTYPES_CONVERSION_TABLE.md index 79794e7d90ad..6b151c75ba4b 100644 --- a/contributingGuides/PROPTYPES_CONVERSION_TABLE.md +++ b/contributingGuides/PROPTYPES_CONVERSION_TABLE.md @@ -35,10 +35,10 @@ type Props = { | PropTypes | TypeScript | Instructions | | -------------------------------------------------------------------- | --------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `PropTypes.any` | `T`, `Record` or `any` | Figure out what would be the correct data type and use it.

If you know that it's a object but isn't possible to determine the internal structure, use `Record`. | +| `PropTypes.any` | `T`, `Record` or `unknown` | Figure out what would be the correct data type and use it.

If you know that it's a object but isn't possible to determine the internal structure, use `Record`. | | `PropTypes.array` or `PropTypes.arrayOf(T)` | `T[]` or `Array` | Convert to `T[]`, where `T` is the data type of the array.

If `T` isn't a primitive type, create a separate `type` for the object structure of your prop and use it. | | `PropTypes.bool` | `boolean` | Convert to `boolean`. | -| `PropTypes.func` | `(arg1: Type1, arg2, Type2...) => ReturnType` | Convert to the function signature. | +| `PropTypes.func` | `(arg1: Type1, arg2: Type2...) => ReturnType` | Convert to the function signature. | | `PropTypes.number` | `number` | Convert to `number`. | | `PropTypes.object`, `PropTypes.shape(T)` or `PropTypes.exact(T)` | `T` | If `T` isn't a primitive type, create a separate `type` for the `T` object structure of your prop and use it.

If you want an object but isn't possible to determine the internal structure, use `Record`. | | `PropTypes.objectOf(T)` | `Record` | Convert to a `Record` where `T` is the data type of values stored in the object.

If `T` isn't a primitive type, create a separate `type` for the object structure and use it. | @@ -120,7 +120,7 @@ type Props = { // It's not possible to infer the data as it can be anything because of reasons X, Y and Z. // eslint-disable-next-line @typescript-eslint/no-explicit-any - anotherUnknownData: any; + anotherUnknownData: unknown; indexes: number[]; items: Item[]; From 8b3bad572dc5081b7d72df23215728c9e15704ac Mon Sep 17 00:00:00 2001 From: Hayata Suenaga Date: Fri, 21 Jul 2023 10:25:19 -0700 Subject: [PATCH 20/20] address PR reviews --- contributingGuides/PROPTYPES_CONVERSION_TABLE.md | 2 +- contributingGuides/TS_CHEATSHEET.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/contributingGuides/PROPTYPES_CONVERSION_TABLE.md b/contributingGuides/PROPTYPES_CONVERSION_TABLE.md index 6b151c75ba4b..4ef1cd5ca655 100644 --- a/contributingGuides/PROPTYPES_CONVERSION_TABLE.md +++ b/contributingGuides/PROPTYPES_CONVERSION_TABLE.md @@ -40,7 +40,7 @@ type Props = { | `PropTypes.bool` | `boolean` | Convert to `boolean`. | | `PropTypes.func` | `(arg1: Type1, arg2: Type2...) => ReturnType` | Convert to the function signature. | | `PropTypes.number` | `number` | Convert to `number`. | -| `PropTypes.object`, `PropTypes.shape(T)` or `PropTypes.exact(T)` | `T` | If `T` isn't a primitive type, create a separate `type` for the `T` object structure of your prop and use it.

If you want an object but isn't possible to determine the internal structure, use `Record`. | +| `PropTypes.object`, `PropTypes.shape(T)` or `PropTypes.exact(T)` | `T` | If `T` isn't a primitive type, create a separate `type` for the `T` object structure of your prop and use it.

If you want an object but it isn't possible to determine the internal structure, use `Record`. | | `PropTypes.objectOf(T)` | `Record` | Convert to a `Record` where `T` is the data type of values stored in the object.

If `T` isn't a primitive type, create a separate `type` for the object structure and use it. | | `PropTypes.string` | `string` | Convert to `string`. | | `PropTypes.node` | `React.ReactNode` | Convert to `React.ReactNode`. `ReactNode` includes `ReactElement` as well as other types such as `strings`, `numbers`, `arrays` of the same, `null`, and `undefined` In other words, anything that can be rendered in React is a `ReactNode`. | diff --git a/contributingGuides/TS_CHEATSHEET.md b/contributingGuides/TS_CHEATSHEET.md index a8f4e88170b0..df6d70b5ae90 100644 --- a/contributingGuides/TS_CHEATSHEET.md +++ b/contributingGuides/TS_CHEATSHEET.md @@ -141,7 +141,7 @@ -- [1.6](#type-narrowing) **Type Narrowing** Narrow types down using `typeof`, discriminated unions, or custom type guards. Refer [this guide](https://medium.com/@hayata.suenaga/discriminated-unions-custom-type-guards-182ebe1f92fb) for more information on when to use discriminated unions and custom type guards. +- [1.6](#type-narrowing) **Type Narrowing** Narrow types down using `typeof`, discriminated unions, or custom type guards. Refer to [this guide](https://medium.com/@hayata.suenaga/discriminated-unions-custom-type-guards-182ebe1f92fb) for more information on when to use discriminated unions and custom type guards. ```ts type Manager = { @@ -214,7 +214,7 @@ const person2 = { name: "Bob", age: 30 } as const; // type: { readonly name: "Bob", readonly age: 30 } const array1 = ["hello", 1]; // type: (string | number)[] - const array2 = ["goodbye", 2]; // type: readonly ["goodbye", 2] + const array2 = ["goodbye", 2] as const; // type: readonly ["goodbye", 2] ```