Complete TypeScript guidelines

[mcmire]

We've started a document which is intended to contain TypeScript guidelines, but it is severely lacking in content.

We should get to an MVP by adding the following guidelines:

• Use enums instead of union types
• Avoid type assertions, or explain their usage when necessary

[mcmire]

Off the top of my head, here's a list of guidelines we could include:

• Constrain generic types if necessary
  • It may not be enough just to have a type or a function take another type — you might have to constrain it if it's not allowed to be anything (e.g. extends Json)
• 💡 Use Partial to type the accumulating variable in reduce
• 💡 Use Omit to reduce requirements
  • If you want to reuse a type for, say, an argument to a function, but you only use part of that type, then you can use Omit to reduce that type to a subset. This makes it easier to test the function because you don't have to create a full version of the object that represents the original type.
• Don't use numeric enums
  • Numeric enums are misleading because it creates a reverse mapping from value to property name, making Object.values not do what you think it does
• Use enums instead of union types
  • Inevitably you will want to refer to the values of a union type somewhere (perhaps as the argument to a function). You can of course just use a literal which represents a member of that union — but if you have an enum, then all of the values are special, and any time you use a value then anyone can see where that value comes from.
• Don't use any
  • any doesn't actually represent "any type"; if you declare a value as any, it tells TypeScript to disable typechecking for that value anywhere it appears.
  • any is dangerous because it infects all surrounding and downstream code. For instance, if you have a function that is declared to return any which actually returns an object, all properties of that object will be any.
  • Need to truly represent something as "any value"? Use unknown instead.
• Avoid type assertions, or explain why they are needed
  • Notable exceptions: reduce, iterating over the keys of an enum
• Avoid type annotations
  • Notable exceptions: reduce (?)
• Need to create a data structure with specific values, but ensure that it's declared with a more abstract type? Create a function that takes an argument of the type you want, then use it to build the object. (Note: This will be solved in a future version of TypeScript with the satisfies keyword.)

There are also a couple of tickets we've created already:

• Add entry forany, as usage in TypeScript Guidelines doc #47
• Add TS guideline: "Document safe or necessary uses of type assertions" #57

[desi]

To get to an MVP we are going to do the enum bullet, don't use any bullet and the type assertions bullet. We will come back to additional items later. Pointing based on this comment.

[MajorLift]

Additional ideas (WIP):

Prefer as const

🚫

const templateString = `Failed with error message: ${error.message}` // Type 'string'

export const PROVIDER_ERRORS = {
  limitExceeded: () =>
    ({
      jsonrpc: '2.0',
      id: IdGenerator(),
      error: {
        code: -32005,
        message: 'Limit exceeded',
      },
    }),
    ...
}
/** evaluates to:
{
  jsonrpc: number;
  id: number;
  error: {
    code: number;
    message: string;
  };
}
*/
export const SUPPORTED_NETWORK_CHAINIDS = {
  MAINNET: '0x1',
  BSC: '0x38',
  OPTIMISM: '0xa',
  POLYGON: '0x89',
  AVALANCHE: '0xa86a',
  ARBITRUM: '0xa4b1',
  LINEA_MAINNET: '0xe708',
};
/** evaluates to:
{
  readonly MAINNET: string,
  readonly BSC: string,
  readonly OPTIMISM: string,
  readonly POLYGON: string,
  readonly AVALANCHE: string,
  readonly ARBITRUM: string,
  readonly LINEA_MAINNET: string,
}
*/

✅

const templateString = `Failed with error message: ${error.message}` as const // Type 'Failed with error message: ${typeof error.message}'

export const PROVIDER_ERRORS = {
  limitExceeded: () =>
    ({
      jsonrpc: '2.0',
      id: IdGenerator(),
      error: {
        code: -32005,
        message: 'Limit exceeded',
      },
    } as const),
  ...
}
/** evaluates to:
{
  readonly jsonrpc: '2.0';
  readonly id: ReturnType<IdGenerator>;
  readonly error: {
    code: -32005;
    message: 'Limit exceeded';
  };
}
*/
export const SUPPORTED_NETWORK_CHAINIDS = {
  MAINNET: '0x1',
  BSC: '0x38',
  OPTIMISM: '0xa',
  POLYGON: '0x89',
  AVALANCHE: '0xa86a',
  ARBITRUM: '0xa4b1',
  LINEA_MAINNET: '0xe708',
} as const;
/** evaluates to:
{
  readonly MAINNET: '0x1',
  readonly BSC: '0x38',
  readonly OPTIMISM: '0xa',
  readonly POLYGON: '0x89',
  readonly AVALANCHE: '0xa86a',
  readonly ARBITRUM: '0xa4b1',
  readonly LINEA_MAINNET: '0xe708',
}
*/

To avoid making the entire object readonly, apply as const to individual properties.

export const SUPPORTED_NETWORK_CHAINIDS = {
  MAINNET: '0x1' as const,
  BSC: '0x38' as const,
  OPTIMISM: '0xa' as const,
  POLYGON: '0x89' as const,
  AVALANCHE: '0xa86a' as const,
  ARBITRUM: '0xa4b1' as const,
  LINEA_MAINNET: '0xe708' as const,
};
/** evaluates to:
{
  MAINNET: '0x1',
  BSC: '0x38',
  OPTIMISM: '0xa',
  POLYGON: '0x89',
  AVALANCHE: '0xa86a',
  ARBITRUM: '0xa4b1',
  LINEA_MAINNET: '0xe708',
}
*/

Exceptions

🚫

const walletName = 'metamask' as const;
this.messagingSystem.registerActionHandler(
      `${name}:call` as const,
      ...
);

✅

const walletName = 'metamask';
this.messagingSystem.registerActionHandler(
      `${name}:call`,
      ...
);

Prefer the nullish coalescing operator ??

• ?? guarantees that the left-hand operand is nullish but not falsy, which is both safer and reduces cognitive overhead for reviewers.
• Using || leads reviewers to assume that ?? was intentionally avoided, forcing them to consider cases where the left-hand operand is falsy. This should never happen if the left-hand operand is guaranteed to be nullish and not falsy.

General guidelines on when to use undefined vs. ?: vs. null

Prefer negation (!) over strict equality checks for undefined, but not for null.

• Unlike with vanilla JavaScript, in TypeScript, we can rely on type inference and narrowing to correctly handle null checks without using strict equals.
• This does not apply if the null check needs to exclude falsy values.
• Because our codebase usually uses undefined for empty values, if a null is used, it should be assumed to be an intentional choice to represent an expected exception case or a specific class of empty value.

Prefer as never for empty types

• never is more accurate for typing an empty container rather than a representation of its fully populated shape.
• Asserting that an empty container is a fully-populated type can cause unexpected runtime behavior.
  • At compile time, properties may seem accessible with no errors.
  • At runtime, these same properties may evaluate as undefined or even throw errors like "Cannot read property of undefined".
• never is assignable to all types, meaning this practice will rarely cause compiler errors.

🚫

export class ExampleClass<S extends BaseState> {
  defaultState: S = {} as S;
  ...
}

obj.reduce(
    (acc, [key, value]) => {
      ...
    },
    {} as Record<keyof typeof obj, Json>,
)

✅

export class ExampleClass<S extends BaseState> {
  defaultState: S = {} as never;
  ...
}

obj.reduce<Record<keyof typeof obj, Json>>(
    (acc, [key, value]) => {
      ...
    },
    {} as never,
)

Object.entries

• Object.keys and Object.entries both type the object keys (index signature) as string instead of keyof typeof obj. An annotation or assertion is necessary to accurately narrow the index signature.
  • @metamask/utils has a getKnownPropertyNames method which resolves this issue by return an array of type keyof typeof obj.
• Strong typing for the entries operation using generic arguments is unavailable, as the generic parameter for Object.entries only allows typing the object's values.

🚫

Object.entries<Json>(state).forEach(([key, value]) => {
  ...
}

(Object.keys(state) as (keyof typeof state)[]).forEach((key) => {
  ...
}

✅

import { getKnownPropertyNames } from '@metamask/utils'

getKnownPropertyNames(state).forEach((key) => { // Type of 'key': 'keyof typeof state'
  ...
}

Object.entries(state).forEach(([key, value]: [keyof typeof state, Json]) => {
  ...
}

Array.prototype.reduce

• Use the generic argument to type the fully populated accumulator.
• Type the initial value of the accumulator object as never.

🚫 These examples work but rely on unsafe type assertions.

Object.entries(state).reduce(
    (acc, [key, value]: [keyof typeof state, Json]) => {
      acc[key] = value;
      return acc;
    },
    {} as Record<keyof typeof state, Json>, // typing accumulator requires type assertion
)

Object.entries(state).reduce<Partial<Record<keyof typeof state, Json>>>(
    (acc, [key, value]: [keyof typeof state, Json]) => {
      acc[key] = value;
      return acc;
    },
    {},
) as Record<keyof typeof state, Json> // typing return value requires type assertion

✅ Generic parameter U safely types both the accumulator object and the return value.

Object.entries(state).reduce<Record<keyof typeof state, Json>>(
    (acc, [key, value]: [keyof typeof state, Json]) => {
      acc[key] = value;
      return acc;
    },
    {} as never, // requires no unsafe assumptions about initial state of accumulator
) // consistent typing for accumulator and return value

Avoid implicit, hard-coded generic parameters that are inaccessible from the outermost user-facing type.

• These are also called "return type-only generics" and are discouraged by the Google TypeScript style guide.
• Implicit generic parameters make it difficult to debug typing issues, and also makes the type brittle against future updates.
• Exception: If the intention is to encapsulate certain generic parameters as "private" fields, wrapping the type in another type that only exposes "public" generic parameters is a good approach.

When augmenting an object type with &, remember to apply Omit to any overlapping property keys.

• & does not automatically overwrite overlapping entries like the spread (...) operator does: https://tsplay.dev/NDJ48W

type Original = {
  a: string;
  b: number;
  c: boolean;
}
type Augment = {
  c: 'hello'
}
type NoOmit = Original & Augment // never
type UseOmit = Omit<Original, 'c'> & Augment // { a: string; b: number; c: 'hello'; }

• One edge case to be especially careful of is if an optional property is being overwritten. Unlike the above case, this will fail silently: https://tsplay.dev/m0OBxN

type Original = {
  a: string;
  b: number;
  c?: boolean;
}
type Augment = {
  c?: 'hello'
}
type NoOmit = Original & Augment // { a: string; b: number; c?: undefined; }

Avoid using && to assert non-nullability

-       const parsedData = stringData && returnsDataOrUndefined(stringData);
-       if (parsedData && parsedData.name) {
+       const parsedData = data
+         ? returnsDataOrUndefined(stringData)
+         : undefined;

Using && to assert non-nullability like this is not a good pattern, since it pollutes the type signature of the assignee.

In this case parsedData gets stuck with a "" type which is neither useful nor actually representative of its expected behavior.

[MajorLift]

@mcmire @desi Thoughts on converting this to an epic? I think we have enough items and content here that this would be better managed as a bunch of small PRs.

[MajorLift]

WIP: This may or may not be appropriate to include in the guidelines.

The focus of the guidelines should be on conveying preferred practices to already knowledgeable readers, not providing instructions about TypeScript.

However, having a glossary to establish terminology used throughout the guidelines might be useful. This terminology could then be used in code review as well.

Type System Concepts

Assignee vs. Assigned

• Assignee: The type being assigned to.
• Assigned: The type being assigned.
• The Assigned type must be "assignable" to the Assignee type.

// Assignment operation - LHS: 'Assignee', RHS: 'Assigned'
const assignee: Assignee = assigned;

// Function call - parameter: 'Assignee', argument: 'Assigned'
function example(param: Assignee) {}
example(assigned);

// Generic parameters with constraints
type ExampleGeneric<Param extends Assignee> = Param;
type ExampleType = GenericExample<Assigned>;

// Relationship - The 'Assigned' type is a subtype of the 'Assignee'
type IsTrue = Assigned extends Assignee ? true : false;

Widest Subtype vs. Narrowest Supertype

• The widest subtype represents the minimum structure and content that a given type must satisfy.
  • "Subtype" is a meaningless category without the qualifier "widest" e.g. never is a subtype of all types.
  • The widest subtype is a full instance of the given type.
  • A narrower subtype would no longer be considered the same type.
  • "Greatest lower bound" and "inf" (infimum) are analogous concepts.
• The narrowest supertype represents the maximum structure and content that a given type can contain while still exclusively being the same type.
  • "Supertype" is a meaningless category without the qualifier "narrowest" e.g. unknown is a supertype of all types.
  • The narrowest supertype encompasses all instances of the given type.
  • A wider supertype would begin to contain exclusive elements of other types.
  • "Least upper bound" and "sup" (supermum) are analogous concepts.
• A suggested naming convention for the widest subtype is to suffix the type name with "Instance," and for the narrowest supertype to suffix with "Constraint," e.g. ExampleTypeInstance and ExampleTypeConstraint.

Signature vs. Constraint

• A type signature represents the exact structure and contents of a value or type, and is populated by literal expressions.
  • This is the assigned type in a type declaration or expression.
  • This is the widest subtype of a given type.
• A type constraint is a wider, abstract type that defines the properties and structure a type is required to satisfy in order to fit into a given category.
  • This is the assignee type in a type declaration or expression.
  • This is the narrowest supertype of a given type.

Subtyping shorthands

• Narrow extends Wide
• Assigned extends Assignee
• Subtype extends Supertype
• Subclass extends Superclass
• Member extends Union
• Intersection extends Member
• StrictlyDeepEqualTuple extends StrictlyDeepEqualTuple
• (...args: Supertype[]) => unknown extends (...args: Subtype[]) => unknown (contravariance)
• never extends Anything (never is assignable to anything)
• Anything extends unknown (anything is assignable to unknown)

Function types

Universal function supertype

🚫

type AnyParamsFunctionType = (...args: any[]) => any;

type NeverParamsFunctionType = (...args: never[]) => unknown;

✅

type FunctionSupertype = ((...args: never) => unknown) | ((...args: never[]) => unknown)

A. (...args: never[]) => unknown

The return type of this supertype function needs to be unknown (universal supertype), and the parameters need to be never (universal subtype).

This is because function parameters are contravariant, meaning (x: SuperType) => T is a subtype of (x: SubType) => T.

• There's an example of this in the TypeScript docs.
• And here's a more detailed reddit comment on this topic.

B. (...args: never) => unknown

In general, array types of arbitrary length are not assignable to tuple types that have fixed types designated to some or all of its index positions (the array is wider than the tuple since it has less constraints).

This means that there's a whole subcategory of functions that (...args: never[]) => unknown doesn't cover: functions that have a finite number of parameters that are strictly ordered and typed, while also accepting an arbitrary number of "rest" parameters. The spread argument list of such a function evaluates as a variadic tuple type with a minimum fixed length, instead of a freely expandable (or contractable) array type.

(...args: never[]) => unknown covers functions for which all of the params can be typed together as a group e.g. (...args: (string | number)[]) => T, or it has a fixed number of params e.g. (a: string, b: number) => T. But to cover cases like (a: string, b: number, ...rest: unknown[]) => T, we need another top type with an even narrower type for the params: (...args: never) => unknown.

Universal function subtype

Sometimes function types that look like the following become necessary:

type FunctionSubtype = (...args: unknown[]) => never;

• The universal function supertype is a universal assignee function that is assignable to by any function.
• The universal function subtype is a universal assigned function that is assignable to any function.

If the function type being defined needs to be assignable to other functions, then the universal function supertype is not the right choice.

[desi]

@desi convert this to an epic. Once converted to an epic remove the estimate and create additional issues that represent the comments and statements above.

[mcmire]

@MajorLift Your glossary seems like it could be quite helpful, even for people who think they are familiar with TypeScript. I too am not sure where it belongs exactly - perhaps another document, or maybe at the very bottom of the current document? We can always add it later, though.

[MajorLift]

It might make sense to have a dedicated doc for type concepts with explanations about some of the more complex types we use. I'll keep collecting material here until it reaches a critical mass.

[MajorLift]

• Duplicate: Create TypeScript Best Practices and style guide #4

[MajorLift]

Closed by #74, which, together with #80, covers the initial topics listed in #69 (comment).

The content suggested by later comments will be preserved here and included in future contributor-docs articles.