From 73b92db7d13b1d96f862cd062b1d31c484917788 Mon Sep 17 00:00:00 2001 From: "D. Ror" Date: Tue, 23 May 2023 17:06:35 -0400 Subject: [PATCH] Update TS style guide (#2175) --- docs/style_guide/ts_style_guide.md | 271 +++++++++++++----- src/components/GoalTimeline/GoalList.tsx | 4 +- .../DragDropComponents/DragSense.tsx | 8 +- .../ReviewEntriesComponent/CellColumns.tsx | 4 +- src/utilities/utilities.ts | 4 +- 5 files changed, 221 insertions(+), 70 deletions(-) diff --git a/docs/style_guide/ts_style_guide.md b/docs/style_guide/ts_style_guide.md index b67f2ab46c..74ef6fa5fb 100644 --- a/docs/style_guide/ts_style_guide.md +++ b/docs/style_guide/ts_style_guide.md @@ -2,25 +2,35 @@ Key Sections: -- [Variable](#variable-and-function) -- [Class](#class) -- [Component](#component) -- [Interface](#interface) -- [Type](#type) -- [Namespace](#namespace) -- [Enum](#enum) +- [Case](#case) (`camelCase` vs. `PascalCase` vs. `ALL_CAPS`) + - [Variable](#variable-and-function) + - [Component](#component) + - [Class](#class) + - [Interface](#interface) + - [Type](#type) + - [Namespace](#namespace) + - [Enum](#enum) + - [Filename](#filename) +- [Interface names](#interface-names) +- [Test filenames](#test-filenames) +- [Type files](#type-files) - [`null` vs. `undefined`](#null-vs-undefined) +- [`const` vs. `let` vs. `var`](#const-vs-let-vs-var) - [Formatting](#formatting) -- [Single vs. Double Quotes](#quotes) +- [Quotes](#quotes) (single vs. double) - [Use semicolons](#semicolons) - [Annotate Arrays as `Type[]`](#array) -- [File Names](#filename) - [`type` vs `interface`](#type-vs-interface) - [One-line `if` statements](#one-line-if-statements) -- [imports](#imports) -- [KeyboardEvents](#keyboardevents) +- [`import`s](#imports) +- [`KeyboardEvent`s](#keyboardevents) +- [Components](#components) +- [Function return type](#function-return-type) +- [Hooks](#hooks) -## Variable and Function +## Case + +### Variable and Function - Use `camelCase` for variable and function names @@ -40,7 +50,29 @@ var fooVar; function barFunc() {} ``` -## Class +### Component + +- Use `PascalCase` for component name, even if function component. + +> Reason: Follows React naming convention. + +**Bad** + +```tsx +const component: React.FC = () => { + return ; +}; +``` + +**Good** + +```tsx +const Component: React.FC = () => { + return ; +}; +``` + +### Class - Use `PascalCase` for class names. @@ -80,29 +112,7 @@ class Foo { } ``` -## Component - -- Use `PascalCase` for component name, even if function component. - -> Reason: Follows React naming convention. - -**Bad** - -```tsx -const component: React.FC = () => { - return ; -}; -``` - -**Good** - -```tsx -const Component: React.FC = () => { - return ; -}; -``` - -## Interface +### Interface - Use `PascalCase` for name. @@ -112,23 +122,7 @@ const Component: React.FC = () => { > Reason: Similar to class -- **Don't** prefix with `I` - -> Reason: Unconventional. `lib.d.ts` defines important interfaces without an `I` (e.g. Window, Document etc). - -**Bad** - -```ts -interface IFoo {} -``` - -**Good** - -```ts -interface Foo {} -``` - -## Type +### Type - Use `PascalCase` for name. @@ -142,7 +136,7 @@ interface Foo {} > Reason: Follows Redux naming convention -## Namespace +### Namespace - Use `PascalCase` for names @@ -161,7 +155,7 @@ namespace foo {} namespace Foo {} ``` -## Enum +### Enum - Use `PascalCase` for enum names @@ -200,6 +194,47 @@ enum Color { } ``` +### Filename + +Name files and folders with `camelCase` (e.g., `utilities.ts`, `index.tsx`, `tractor.png`, `components/`), unless it is +the name of the contained Component (e.g., `DataEntryTable.tsx`, `ReviewEntries/`). + +> Reason: `camelCase` is conventional across many JS teams. + +## Interface names + +- **Don't** prefix with `I` + +> Reason: Unconventional. `lib.d.ts` defines important interfaces without an `I` (e.g. Window, Document etc). + +**Bad** + +```ts +interface IFoo {} +``` + +**Good** + +```ts +interface Foo {} +``` + +## Test filenames + +- The test file associated with `path/to/Component.tsx` should be `path/to/tests/Component.test.tsx`. + +- Auxiliary test files with data or mock objects should have filenames ending in `Mock.ts`, e.g., + `path/to/tests/DataMock.ts`. + +> Reason: `*.test.*` and `*Mock.ts` files are ignored by our CodeCov settings. + +## Type files + +- Separate type files should contain `type`s, `class`es, `interface`s, `enum`s but not any classes or functions that + need unit testings. Such files should have filenames ending in `Types.ts`, e.g., `MergeDupReduxTypes.ts`. + +> Reason: `*Types.ts` files are ignored by our CodeCov settings. + ## Null vs. Undefined - Prefer not to use either for explicit unavailability @@ -266,6 +301,34 @@ if (error) > Remark: Use `===` / `!==` to check for `null` / `undefined` on **primitives** that might be other falsy values (like > `''`,`0`,`false`). +## `const` vs. `let` vs. `var` + +- `const` allows the variable to be mutated, but doesn't allow it to be redeclared; prefer `const` any time your + variable only needs to be declared once. + +- `let` and `var` both allow redeclaration, but `var` is a global variable and `let` is limited to the scope of its + declaration; prefer `let` to `var`. + +**Bad** + +```ts +var shouldClapHands = false; +for (var i = 0; i < 3; i++) { + var you = getYouAtTime(i); + shouldClapHands ||= you.isHappy() && you.knowsIt(); +} +``` + +**Good** + +```ts +let shouldClapHands = false; +for (let i = 0; i < 3; i++) { + const you = getYouAtTime(i); + shouldClapHands ||= you.isHappy() && you.knowsIt(); +} +``` + ## Formatting Use [Prettier](https://prettier.io/) to format TypeScript code as described in the README. @@ -274,7 +337,7 @@ Use [Prettier](https://prettier.io/) to format TypeScript code as described in t ## Quotes -- Prefer double quotes ("). +- Prefer double quotes (`"your text"`) to single quotes (`'your text'`). > Reason: Follows Prettier naming convention. @@ -296,12 +359,6 @@ Use [Prettier](https://prettier.io/) to format TypeScript code as described in t > Reasons: Its easier to read. Its used by the TypeScript team. Makes easier to know something is an array as the mind > is trained to detect `[]`. -## Filename - -Name files with `camelCase`. E.g. `accordion.tsx`, `myControl.tsx`, `utils.ts`, `map.ts` etc. - -> Reason: Conventional across many JS teams. - ## type vs. interface - Use `type` when you _might_ need a union or intersection: @@ -329,7 +386,7 @@ class X implements FooBar { ## One line `if` statements -Add braces to one-line `if` statements; +- Add braces to one-line `if` statements; **Good** @@ -351,7 +408,7 @@ if (isEmpty) ## imports -Use absolute `import` statements everywhere for consistency. +- Use absolute `import` statements everywhere for consistency. **Good** @@ -372,7 +429,7 @@ import { Project } from "../../../../types/project"; ## KeyboardEvents -Use `ts-key-enum` when comparing to `React.KeyboardEvent`s. +- Use `ts-key-enum` when comparing to `React.KeyboardEvent`s. **Good** @@ -391,3 +448,87 @@ if (event.key === "Enter") { ``` > Reason: Avoid typos and increase the number of mistakes that can be caught at compile time. + +## Components + +- Prefer functional components to class components. + +**Good** + +```ts +function Component(props: ComponentProps): ReactElement { + return ; +} +``` + +**Bad** + +```ts +class Component extends React.Component { + render() { + return ; + } +} +``` + +## Function return type + +- Specify the return type of a named function, whether declared as a `function` or `const`. +- The return type of a functional component should be `ReactElement`. + +**Good** + +```tsx +import {ReactElement} from React; + +function Component(props: { name: string }): ReactElement { + const sayHi = (name: string): string => { + return `Hi, ${name}!`; + } + + return sayHi(props.name); +} +``` + +**Bad** + +```tsx +function Component(props: { name: string }) { + const sayHi = (name: string) => { + return `Hi, ${name}!`; + }; + + return sayHi(props.name); +} +``` + +> Reason: Ensure all return paths are the expected type. + +## Hooks + +- Prefer `useAppDispatch` to `useDispatch` and `useAppSelector` to `useSelector`. + +**Good** + +```ts +import { StoreState } from "types"; +import { useAppDispatch, useAppSelector } from "types/hooks"; + +function Component(): ReactElement { + const dispatch = useAppDispatch(); + const subState = useAppSelector((state: StoreState) => state.subState); +} +``` + +**Bad** + +```ts +import { useDispatch, useSelector } from "react-redux"; + +function Component(): ReactElement { + const dispatch = useDispatch(); + const subState = useSelector((state) => state.subState); +} +``` + +> Reason: See `types/hooks.ts`. diff --git a/src/components/GoalTimeline/GoalList.tsx b/src/components/GoalTimeline/GoalList.tsx index a3aac5f308..82c4248539 100644 --- a/src/components/GoalTimeline/GoalList.tsx +++ b/src/components/GoalTimeline/GoalList.tsx @@ -75,7 +75,9 @@ export default function GoalList(props: GoalListProps): ReactElement { : makeGoalTile(tileSize, props.orientation)}
{ - if (props.scrollToEnd && element) element.scrollIntoView(true); + if (props.scrollToEnd && element) { + element.scrollIntoView(true); + } }} /> diff --git a/src/goals/MergeDupGoal/MergeDupStep/DragDropComponents/DragSense.tsx b/src/goals/MergeDupGoal/MergeDupStep/DragDropComponents/DragSense.tsx index 6be6198591..6d4551a8ed 100644 --- a/src/goals/MergeDupGoal/MergeDupStep/DragDropComponents/DragSense.tsx +++ b/src/goals/MergeDupGoal/MergeDupStep/DragDropComponents/DragSense.tsx @@ -20,9 +20,13 @@ interface DragSenseProps { } function arraysEqual(arr1: T[], arr2: T[]): boolean { - if (arr1.length !== arr2.length) return false; + if (arr1.length !== arr2.length) { + return false; + } for (let i = 0; i < arr1.length; i++) { - if (arr1[i] !== arr2[i]) return false; + if (arr1[i] !== arr2[i]) { + return false; + } } return true; } diff --git a/src/goals/ReviewEntries/ReviewEntriesComponent/CellColumns.tsx b/src/goals/ReviewEntries/ReviewEntriesComponent/CellColumns.tsx index bc6701e340..f83876ea78 100644 --- a/src/goals/ReviewEntries/ReviewEntriesComponent/CellColumns.tsx +++ b/src/goals/ReviewEntries/ReviewEntriesComponent/CellColumns.tsx @@ -332,7 +332,9 @@ const columns: Column[] = [ } // If the two glosses SEEM identical, sort by length - if (compare === 0) compare = codeA.length - codeB.length; + if (compare === 0) { + compare = codeA.length - codeB.length; + } } count++; } diff --git a/src/utilities/utilities.ts b/src/utilities/utilities.ts index 71c79dd83f..ebf10e9bd7 100644 --- a/src/utilities/utilities.ts +++ b/src/utilities/utilities.ts @@ -16,7 +16,9 @@ export function randomIntString(): string { /** Quicksort implementation O(n log n). */ export function quicksort(arr: T[], score: (item: T) => number): T[] { - if (arr.length <= 1) return arr; + if (arr.length <= 1) { + return arr; + } const pivotIndex = 0; const pivot = arr[0];