Skip to content

Commit

Permalink
Merge pull request #32475 from margelo/@chrispader/standardize-type-o…
Browse files Browse the repository at this point in the history
…nly-imports

[No QA] Standardize type-only imports
  • Loading branch information
roryabraham authored Jan 3, 2024
2 parents 339987b + 8c5f3a7 commit e6616e3
Show file tree
Hide file tree
Showing 784 changed files with 1,917 additions and 1,486 deletions.
15 changes: 15 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,21 @@ module.exports = {
'@typescript-eslint/switch-exhaustiveness-check': 'error',
'@typescript-eslint/consistent-type-definitions': ['error', 'type'],
'@typescript-eslint/no-floating-promises': 'off',
'@typescript-eslint/consistent-type-imports': [
'error',
{
prefer: 'type-imports',
fixStyle: 'separate-type-imports',
},
],
'@typescript-eslint/no-import-type-side-effects': 'error',
'@typescript-eslint/consistent-type-exports': [
'error',
{
fixMixedExportsWithInlineTypeSpecifier: false,
},
],
'import/consistent-type-specifier-style': ['error', 'prefer-top-level'],
'es/no-nullish-coalescing-operators': 'off',
'es/no-optional-chaining': 'off',
'valid-jsdoc': 'off',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import Category from './Category';
import type Category from './Category';
import newComponent from './newComponentCategory';

const categories: Category[] = [newComponent];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import traverse from '@babel/traverse';
import CONST from '../../../../libs/CONST';
import GithubUtils from '../../../../libs/GithubUtils';
import promiseSome from '../../../../libs/promiseSome';
import Category from './Category';
import type Category from './Category';

type SuperClassType = {superClass: {name?: string; object: {name: string}; property: {name: string}} | null; name: string};

Expand Down
2 changes: 1 addition & 1 deletion assets/emojis/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import TravelAndPlaces from '@assets/images/emojiCategoryIcons/plane.svg';
import AnimalsAndNature from '@assets/images/emojiCategoryIcons/plant.svg';
import Activities from '@assets/images/emojiCategoryIcons/soccer-ball.svg';
import FrequentlyUsed from '@assets/images/history.svg';
import {HeaderEmoji, PickerEmojis} from './types';
import type {HeaderEmoji, PickerEmojis} from './types';

const skinTones = [
{
Expand Down
2 changes: 1 addition & 1 deletion assets/emojis/en.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {EmojisList} from './types';
import type {EmojisList} from './types';

/* eslint-disable @typescript-eslint/naming-convention */
const enEmojis: EmojisList = {
Expand Down
2 changes: 1 addition & 1 deletion assets/emojis/es.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {EmojisList} from './types';
import type {EmojisList} from './types';

/* eslint-disable @typescript-eslint/naming-convention */
const esEmojis: EmojisList = {
Expand Down
2 changes: 1 addition & 1 deletion assets/emojis/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import emojis from './common';
import enEmojis from './en';
import esEmojis from './es';
import {Emoji} from './types';
import type {Emoji} from './types';

type EmojiTable = Record<string, Emoji>;

Expand Down
2 changes: 1 addition & 1 deletion assets/emojis/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import IconAsset from '@src/types/utils/IconAsset';
import type IconAsset from '@src/types/utils/IconAsset';

type Emoji = {
code: string;
Expand Down
39 changes: 36 additions & 3 deletions contributingGuides/TS_STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
- [1.19 Satisfies operator](#satisfies-operator)
- [1.20 Hooks instead of HOCs](#hooks-instead-of-hocs)
- [1.21 `compose` usage](#compose-usage)
- [1.22 Type imports](#type-imports)
- [Exception to Rules](#exception-to-rules)
- [Communication Items](#communication-items)
- [Migration Guidelines](#migration-guidelines)
Expand Down Expand Up @@ -383,7 +384,7 @@ type Foo = {

<a name="file-organization"></a><a name="1.15"></a>

- [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. Do not use [`satisfies` operator](#satisfies-operator) for platform-specific implementations, always define shared types that complies with all variants.
- [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. Do not use [`satisfies` operator](#satisfies-operator) for platform-specific implementations, always define shared types that complies with all variants.

> 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.

Expand Down Expand Up @@ -514,7 +515,7 @@ type Foo = {
<a name="hooks-instead-of-hocs"></a><a name="1.20"></a>

- [1.20](#hooks-instead-of-hocs) **Hooks instead of HOCs**: Replace HOCs usage with Hooks whenever possible.

> Why? Hooks are easier to use (can be used inside the function component), and don't need nesting or `compose` when exporting the component. It also allows us to remove `compose` completely in some components since it has been bringing up some issues with TypeScript. Read the [`compose` usage](#compose-usage) section for further information about the TypeScript issues with `compose`.

> Note: Because Onyx doesn't provide a hook yet, in a component that accesses Onyx data with `withOnyx` HOC, please make sure that you don't use other HOCs (if applicable) to avoid HOC nesting.
Expand Down Expand Up @@ -571,7 +572,7 @@ type Foo = {
<a name="compose-usage"></a><a name="1.21"></a>
- [1.21](#compose-usage) **`compose` usage**: Avoid the usage of `compose` function to compose HOCs in TypeScript files. Use nesting instead.
> Why? `compose` function doesn't work well with TypeScript when dealing with several HOCs being used in a component, many times resulting in wrong types and errors. Instead, nesting can be used to allow a seamless use of multiple HOCs and result in a correct return type of the compoment. Also, you can use [hooks instead of HOCs](#hooks-instead-of-hocs) whenever possible to minimize or even remove the need of HOCs in the component.
```ts
Expand Down Expand Up @@ -607,6 +608,38 @@ type Foo = {
export default withCurrentUserPersonalDetails(ComponentWithReportOrNotFound);
```
<a name="type-imports"></a><a name="1.22"></a>
- [1.22](#type-imports) **Type imports/exports**: Always use the `type` keyword when importing/exporting types
> Why? In order to improve code clarity and consistency and reduce bundle size after typescript transpilation, we enforce the all type imports/exports to contain the `type` keyword. This way, TypeScript can automatically remove those imports from the transpiled JavaScript bundle
Imports:
```ts
// BAD
import {SomeType} from './a'
import someVariable from './a'

import {someVariable, SomeOtherType} from './b'

// GOOD
import type {SomeType} from './a'
import someVariable from './a'
```
Exports:
```ts
// BAD
export {SomeType}
export someVariable
// or
export {someVariable, SomeOtherType}

// GOOD
export type {SomeType}
export someVariable
```
## 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:`. 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.
Expand Down
Loading

0 comments on commit e6616e3

Please sign in to comment.