Skip to content

Commit

Permalink
Increase "react", "api" import specificity (#2973)
Browse files Browse the repository at this point in the history
  • Loading branch information
imnasnainaec authored Apr 22, 2024
1 parent 613bd07 commit 8bee879
Show file tree
Hide file tree
Showing 31 changed files with 193 additions and 118 deletions.
55 changes: 42 additions & 13 deletions docs/style_guide/ts_style_guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@ Key Sections:
- [Formatting](#formatting)
- [Quotes](#quotes) (single vs. double)
- [Use semicolons](#semicolons)
- [Annotate Arrays as `Type[]`](#array)
- [`type` vs `interface`](#type-vs-interface)
- [Arrays (annotate as `Type[]`)](#arrays)
- [`type` vs. `interface`](#type-vs-interface)
- [One-line `if` statements](#one-line-if-statements)
- [`import`s](#imports)
- [Absolute vs. relative paths](#absolute-vs-relative-paths)
- [Specificity preferred](#specificity-preferred)
- [`KeyboardEvent`s](#keyboardevents)
- [Components](#components)
- [Function return type](#function-return-type)
Expand Down Expand Up @@ -59,17 +61,17 @@ function barFunc() {}
**Bad**

```tsx
const component: React.FC = () => {
function component(): ReactElement {
return <subComponent />;
};
}
```

**Good**

```tsx
const Component: React.FC = () => {
function Component(): ReactElement {
return <SubComponent />;
};
}
```

### Class
Expand Down Expand Up @@ -235,7 +237,7 @@ interface Foo {}

> Reason: `*Types.ts` files are ignored by our CodeCov settings.
## Null vs. Undefined
## `null` vs. `undefined`

- Prefer not to use either for explicit unavailability

Expand Down Expand Up @@ -352,14 +354,14 @@ Use [Prettier](https://prettier.io/) to format TypeScript code as described in t
> [google/angular](https://github.com/angular/angular/), [facebook/react](https://github.com/facebook/react),
> [Microsoft/TypeScript](https://github.com/Microsoft/TypeScript/).
## Array
## Arrays

- Annotate arrays as `foos:Foo[]` instead of `foos:Array<Foo>`.

> 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 `[]`.
## type vs. interface
## `type` vs. `interface`

- Use `type` when you _might_ need a union or intersection:

Expand Down Expand Up @@ -406,28 +408,55 @@ if (isEmpty)
> Reason: Avoiding braces can cause developers to miss bugs, such as Apple's infamous
> [goto-fail bug](https://nakedsecurity.sophos.com/2014/02/24/anatomy-of-a-goto-fail-apples-ssl-bug-explained-plus-an-unofficial-patch/)
## imports
## `import`s

### Absolute vs. relative paths

- Use absolute `import` statements everywhere for consistency.

**Good**

```ts
import { type Project } from "api/models";
import { getAllProjects } from "backend";
import { Project } from "types/project";
```

**Bad**

```ts
import { type Project } from "../../../../api/models";
import { getAllProjects } from "../../../../backend";
import { Project } from "../../../../types/project";
```

> Reason: Provides consistency for imports across all files and shortens imports of commonly used top level modules.
> Developers don't have to count `../` to know where a module is, they can simply start from the root of `src/`.
## KeyboardEvents
### Specificity preferred

- Generally import the specific things needed (e.g., not `React` when `{ type ReactElement }` will do), and from a more
specific target (e.g., `from "api/models"` rather than `from "api"`):

**Good**

```ts
import { type ReactElement } from "react";

import { type Project } from "api/models";

function Component(props: { project: Project }): ReactElement {}
```

**Bad**

```ts
import React from "react";

import { type Project } from "api";

function Component(props: { project: Project }): React.ReactElement {}
```

## `KeyboardEvent`s

- Use `ts-key-enum` when comparing to `React.KeyboardEvent`s.

Expand Down
10 changes: 5 additions & 5 deletions src/components/AppBar/SpeakerMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,20 @@ import {
Typography,
} from "@mui/material";
import {
ForwardedRef,
MouseEvent,
ReactElement,
type ForwardedRef,
type MouseEvent,
type ReactElement,
useEffect,
useState,
} from "react";
import { useTranslation } from "react-i18next";
import { useSelector } from "react-redux";

import { Speaker } from "api";
import { type Speaker } from "api/models";
import { getAllSpeakers } from "backend";
import { buttonMinHeight } from "components/AppBar/AppBarTypes";
import { setCurrentSpeaker } from "components/Project/ProjectActions";
import { StoreState } from "types";
import { type StoreState } from "types";
import { useAppDispatch } from "types/hooks";
import { themeColors } from "types/theme";

Expand Down
17 changes: 12 additions & 5 deletions src/components/AppBar/UserMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,24 @@ import {
MenuItem,
Typography,
} from "@mui/material";
import React, { Fragment, ReactElement, useState } from "react";
import {
type CSSProperties,
type ForwardedRef,
Fragment,
type MouseEvent,
type ReactElement,
useState,
} from "react";
import { useTranslation } from "react-i18next";
import { useNavigate } from "react-router-dom";

import { isSiteAdmin } from "backend";
import * as LocalStorage from "backend/localStorage";
import {
type TabProps,
buttonMinHeight,
shortenName,
tabColor,
TabProps,
} from "components/AppBar/AppBarTypes";
import { clearCurrentProject } from "components/Project/ProjectActions";
import { useAppDispatch } from "types/hooks";
Expand All @@ -46,7 +53,7 @@ export default function UserMenu(props: TabProps): ReactElement {
const [isAdmin, setIsAdmin] = useState(false);
const username = LocalStorage.getCurrentUser()?.username;

function handleClick(event: React.MouseEvent<HTMLButtonElement>): void {
function handleClick(event: MouseEvent<HTMLButtonElement>): void {
setAnchorElement(event.currentTarget);
}

Expand Down Expand Up @@ -106,7 +113,7 @@ export default function UserMenu(props: TabProps): ReactElement {
interface UserMenuListProps {
isAdmin: boolean;
onSelect: () => void;
forwardedRef?: React.ForwardedRef<any>;
forwardedRef?: ForwardedRef<any>;
}

/**
Expand All @@ -118,7 +125,7 @@ export function UserMenuList(props: UserMenuListProps): ReactElement {
const { t } = useTranslation();
const navigate = useNavigate();

const iconStyle: React.CSSProperties =
const iconStyle: CSSProperties =
document.body.dir == "rtl" ? { marginLeft: 6 } : { marginRight: 6 };

return (
Expand Down
4 changes: 2 additions & 2 deletions src/components/AppBar/tests/NavigationButtons.test.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Provider } from "react-redux";
import renderer, { ReactTestInstance } from "react-test-renderer";
import renderer, { type ReactTestInstance } from "react-test-renderer";
import configureMockStore from "redux-mock-store";

import { Permission } from "api";
import { Permission } from "api/models";
import NavigationButtons, {
dataCleanupButtonId,
dataEntryButtonId,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
import { Autocomplete } from "@mui/material";
import React, { ReactElement, useContext, useEffect } from "react";
import {
type KeyboardEvent,
type ReactElement,
type RefObject,
useContext,
useEffect,
} from "react";
import { Key } from "ts-key-enum";

import { WritingSystem } from "api/models";
import { type WritingSystem } from "api/models";
import { LiWithFont, TextFieldWithFont } from "utilities/fontComponents";
import SpellCheckerContext from "utilities/spellCheckerContext";

interface GlossWithSuggestionsProps {
isNew?: boolean;
isDisabled?: boolean;
gloss: string;
glossInput?: React.RefObject<HTMLInputElement>;
glossInput?: RefObject<HTMLInputElement>;
updateGlossField: (newValue: string) => void;
handleEnter: () => void;
onBlur?: () => void;
Expand Down Expand Up @@ -81,7 +87,7 @@ export default function GlossWithSuggestions(
{option}
</LiWithFont>
)}
onKeyPress={(e: React.KeyboardEvent) => {
onKeyPress={(e: KeyboardEvent) => {
if (e.key === Key.Enter) {
props.handleEnter();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,24 @@
import { Autocomplete, AutocompleteCloseReason } from "@mui/material";
import React, { ReactElement, useEffect } from "react";
import {
type KeyboardEvent,
type ReactElement,
type RefObject,
type SyntheticEvent,
useEffect,
} from "react";
import { Key } from "ts-key-enum";

import { WritingSystem } from "api/models";
import { type WritingSystem } from "api/models";
import { LiWithFont, TextFieldWithFont } from "utilities/fontComponents";

interface VernWithSuggestionsProps {
isNew?: boolean;
isDisabled?: boolean;
vernacular: string;
vernInput?: React.RefObject<HTMLInputElement>;
vernInput?: RefObject<HTMLInputElement>;
updateVernField: (newValue: string, openDialog?: boolean) => void;
onBlur: () => void;
onClose?: (e: React.SyntheticEvent, reason: AutocompleteCloseReason) => void;
onClose?: (e: SyntheticEvent, reason: AutocompleteCloseReason) => void;
suggestedVerns?: string[];
handleEnter: () => void;
vernacularLang: WritingSystem;
Expand Down Expand Up @@ -49,7 +55,7 @@ export default function VernWithSuggestions(
// onInputChange is triggered by typing
props.updateVernField(value);
}}
onKeyPress={(e: React.KeyboardEvent) => {
onKeyPress={(e: KeyboardEvent) => {
if (e.key === Key.Enter) {
props.handleEnter();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from "react";
import { createRef } from "react";
import renderer from "react-test-renderer";

import GlossWithSuggestions from "components/DataEntry/DataEntryTable/EntryCellComponents/GlossWithSuggestions";
Expand All @@ -20,7 +20,7 @@ describe("GlossWithSuggestions", () => {
renderer.create(
<GlossWithSuggestions
gloss={"gloss"}
glossInput={React.createRef<HTMLInputElement>()}
glossInput={createRef<HTMLInputElement>()}
updateGlossField={jest.fn()}
handleEnter={jest.fn()}
analysisLang={newWritingSystem()}
Expand All @@ -36,7 +36,7 @@ describe("GlossWithSuggestions", () => {
<GlossWithSuggestions
isNew
gloss={""}
glossInput={React.createRef<HTMLInputElement>()}
glossInput={createRef<HTMLInputElement>()}
updateGlossField={jest.fn()}
handleEnter={jest.fn()}
analysisLang={newWritingSystem()}
Expand All @@ -52,7 +52,7 @@ describe("GlossWithSuggestions", () => {
<GlossWithSuggestions
isDisabled
gloss={""}
glossInput={React.createRef<HTMLInputElement>()}
glossInput={createRef<HTMLInputElement>()}
updateGlossField={jest.fn()}
handleEnter={jest.fn()}
analysisLang={newWritingSystem()}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from "react";
import { createRef } from "react";
import renderer from "react-test-renderer";

import VernWithSuggestions from "components/DataEntry/DataEntryTable/EntryCellComponents/VernWithSuggestions";
Expand All @@ -20,7 +20,7 @@ describe("VernWithSuggestions", () => {
renderer.create(
<VernWithSuggestions
vernacular={"vern"}
vernInput={React.createRef<HTMLInputElement>()}
vernInput={createRef<HTMLInputElement>()}
updateVernField={jest.fn()}
handleEnter={jest.fn()}
onBlur={jest.fn()}
Expand All @@ -37,7 +37,7 @@ describe("VernWithSuggestions", () => {
<VernWithSuggestions
isNew
vernacular={""}
vernInput={React.createRef<HTMLInputElement>()}
vernInput={createRef<HTMLInputElement>()}
updateVernField={jest.fn()}
handleEnter={jest.fn()}
onBlur={jest.fn()}
Expand All @@ -54,7 +54,7 @@ describe("VernWithSuggestions", () => {
<VernWithSuggestions
isDisabled
vernacular={""}
vernInput={React.createRef<HTMLInputElement>()}
vernInput={createRef<HTMLInputElement>()}
updateVernField={jest.fn()}
handleEnter={jest.fn()}
onBlur={jest.fn()}
Expand Down
4 changes: 2 additions & 2 deletions src/components/Dialogs/DeleteEditTextDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
TextField,
Tooltip,
} from "@mui/material";
import React, { ReactElement, useState } from "react";
import { type KeyboardEvent, type ReactElement, useState } from "react";
import { useTranslation } from "react-i18next";
import { Key } from "ts-key-enum";

Expand Down Expand Up @@ -69,7 +69,7 @@ export default function DeleteEditTextDialog(
}
}

function confirmIfEnter(event: React.KeyboardEvent<any>): void {
function confirmIfEnter(event: KeyboardEvent<any>): void {
if (event.key === Key.Enter) {
onSave();
}
Expand Down
9 changes: 7 additions & 2 deletions src/components/Dialogs/EditTextDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ import {
InputAdornment,
TextField,
} from "@mui/material";
import React, { ReactElement, useEffect, useState } from "react";
import {
type KeyboardEvent,
type ReactElement,
useEffect,
useState,
} from "react";
import { useTranslation } from "react-i18next";
import { Key } from "ts-key-enum";

Expand Down Expand Up @@ -60,7 +65,7 @@ export default function EditTextDialog(
}
}

function confirmIfEnter(event: React.KeyboardEvent<any>): void {
function confirmIfEnter(event: KeyboardEvent<any>): void {
if (event.key === Key.Enter) {
onConfirm();
}
Expand Down
Loading

0 comments on commit 8bee879

Please sign in to comment.