gestalt icon indicating copy to clipboard operation
gestalt copied to clipboard

Internal: Change type import styles

Open jackhsu978 opened this issue 6 months ago • 2 comments

Summary

This PR enables the following ESLint rules:

  1. consistent-type-specifier-style with prefer-inline option
  2. consistent-type-imports with inline-type-imports option

TL;DR is that type keyword is now required for type imports

import { type Foo } from 'Foo';
import type Bar from 'Bar';
type T = Foo;
const x: Bar = 1;

Why?

The benefits include:

  • Avoiding Unintentional Side Effects
  • Isolated Module Transpilation, which may reduce bundle size.

Deep Dive

How to import a named export type?

BAD

// 🔴 ReactNode and Reactelement are type imports
import { ReactNode, ReactElement } from 'react';

BAD because we prefer inline type specifiers import { type A, type B } over top-level type specifier import type { A, B }.

// 🔴 Prefer inline type specifiers over top-level type specifier
import type { ReactNode, ReactElement } from 'react';

GOOD

// ✅ Inline type specifiers
import { type ReactNode, type ReactElement } from 'react';

How to import the default export type?

If we are just importing the default export type, we can either use inline type specifier

// inline type specifier
import { type default as Foo } from './Foo';

or the top-level type specifier, which is more concise:

// top-level type specifier
import type Foo from './Foo';

How to import the default export type AND a name import type?

When we import both the default export type AND a named import, we can only use the inline type specifiers like this in order to write it in one import statement:

// Use the inline type specifiers if we are also doing some named imports
import { type default as Foo, type Bar } from './Foo';

Don't: duplicated import statements

BAD

// 🔴 Duplicated import statements for the same module
import type Foo from './Foo';
import { type Bar } from './Foo';

BAD

// 🔴 Only use the top-level specifier if we only import the default export type
// DO NOT use it if we are importing any named import types.
import type Foo, { Bar } from './Foo';

BAD

// 🔴 LegacyContext is not a named export type
import { type Foo, type Bar } from './Foo';

GOOD

// ✅ This correctly imports the default export type as Foo,
// as well as the named export type named Bar
import { type default as Foo, type Bar } from './Foo';

How to import the type of an instance of a class

Suppose we have MyClass.ts

export default class MyClass {}

The default export MyClass is also the type of its instance:

import type MyClass from './MyClass';

type Props = {
  obj: MyClass;
};

How to import a value solely for its type

Suppose we have Foo.tsx

export const MY_CONST = { a: 1, b: 2 } as const;
export default function Foo({ children }: { children: string }) {
  return <div>{children}</div>;
}

And we want to import the types for MY_CONST and Foo:

import { type ComponentProps } from 'react';
import { type default as Foo, type MY_CONST } from './Foo';

type Props = {
  constKeys: keyof typeof MY_CONST; // evaluated type: 'a' | 'b'
  children: ComponentProps<typeof Foo>['chlidren']; // evaluated type: string
};

[!Tip] Why do we use type import even though Foo and MY_CONST are not actually types?

This is because we are only calling typeof on these values and TypeScript knows that these imports are only necessary for type checking.

If value is imported, derive type from it instead of adding type imports

Note that MyClass, Foo, and MY_CONST are imported for type checking AND runtime. In such case, type imports would be redundant since we can just derive the types from the values.

import MyClass from './MyClass';
import Foo, { MY_CONST } from './Foo';

type Props = {
  constKeys: keyof typeof MY_CONST; // evaluated type: 'a' | 'b'
  children: ComponentProps<typeof Foo>['chlidren']; // evaluated type: string
};

export default function Bar(props: Props) {
  return (
    <Foo
      onSomeEvent={() => {
        const obj: MyClass = new MyClass();
        obj.doSomething(MY_CONST[constKeys]);
      }}
    >
      {children}
    </Foo>
  );
}

jackhsu978 avatar Aug 12 '24 17:08 jackhsu978