From 34935f79175517d5d98b089ae211d746a7bdf7a0 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 16 Jul 2026 13:01:40 -0400 Subject: [PATCH 1/2] Add `Derive types from authoritative sources` guideline to Type Inference --- docs/typescript.md | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/docs/typescript.md b/docs/typescript.md index 8435cae..ebbf899 100644 --- a/docs/typescript.md +++ b/docs/typescript.md @@ -122,6 +122,45 @@ const updatedTransactionMeta = { }; ``` +#### Derive types from authoritative sources instead of re-declaring them + +When a type already exists at an authoritative source β€” a controller's state type, a function's return, a package's exported type, a schema β€” reference and **derive** from it rather than hand-writing a fresh type that restates its shape. This is the type-level counterpart to preferring inference: indexed access (`State['field']`), `typeof`, `ReturnType` / `Parameters`, and utility types (`Pick` / `Omit` / `Partial`) let a derived type track its source automatically. + +- A hand-written type that restates an existing one **duplicates** the shape: every reader must reconcile the two, and every change has to touch both. +- The restatement is a _guess_ at the source's shape, and the guess is almost always **wider** than the real type β€” it admits values the authoritative type would reject (see [Avoid unintentionally widening an inferred type with a type annotation](#avoid-unintentionally-widening-an-inferred-type-with-a-type-annotation)). Marking every field optional is the most common way this happens. +- Because the copy is hard-coded rather than derived, it is **brittle against code drift** (see [Prefer type inference over annotations and assertions](#prefer-type-inference-over-annotations-and-assertions)): when the source evolves the copy silently goes stale, and the compiler cannot flag the divergence, so the failure surfaces at runtime rather than at build. + +Define a fresh type only when no authoritative source exists β€” a genuinely new shape at a boundary the code owns. Before defining, look for the source: the type is often already exported from the `@metamask/*` package the value comes from. + +🚫 Re-declare a slice of an existing controller state type + +```typescript +// Restates part of NetworkController state: all-optional (wider than the real, +// required field), keyed by `string` instead of `Hex`, and unlinked from the +// source β€” so it drifts silently when the controller's shape changes. +type NetworkControllerState = { + networkConfigurationsByChainId?: Record< + string, + { + defaultRpcEndpointIndex?: number; + rpcEndpoints?: { networkClientId?: string }[]; + } + >; +}; +``` + +βœ… Derive the slice from the authoritative exported type + +```typescript +import type { NetworkState } from '@metamask/network-controller'; + +// Tracks NetworkController's real shape (`Record`) +// and narrows to just the field in use. +type NetworkConfigurations = NetworkState['networkConfigurationsByChainId']; +``` + +The derived form stays correct when `NetworkController` changes `networkConfigurationsByChainId`; the hand-written copy does not, and the compiler cannot tell you it has gone stale. + ### Type Annotations An explicit type annotation may be used to override an inferred type if: From ee26d36a0dd22d13c5d1b0f8783328d5cf0697b8 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 16 Jul 2026 13:08:55 -0400 Subject: [PATCH 2/2] Use the too-wide-dependency counterexample and add the example anchor --- docs/typescript.md | 43 +++++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/docs/typescript.md b/docs/typescript.md index ebbf899..362400e 100644 --- a/docs/typescript.md +++ b/docs/typescript.md @@ -132,34 +132,41 @@ When a type already exists at an authoritative source β€” a controller's state t Define a fresh type only when no authoritative source exists β€” a genuinely new shape at a boundary the code owns. Before defining, look for the source: the type is often already exported from the `@metamask/*` package the value comes from. -🚫 Re-declare a slice of an existing controller state type +**Example ([πŸ”— permalink](#example-5f6f1503-1ce6-432f-8d38-61d0f44a03ce)):** + +🚫 A dependency typed too wide, which forces every consumer to re-cast the shape by hand: ```typescript -// Restates part of NetworkController state: all-optional (wider than the real, -// required field), keyed by `string` instead of `Hex`, and unlinked from the -// source β€” so it drifts silently when the controller's shape changes. -type NetworkControllerState = { - networkConfigurationsByChainId?: Record< - string, - { - defaultRpcEndpointIndex?: number; - rpcEndpoints?: { networkClientId?: string }[]; - } - >; +type Dependencies = { + // `Record` is a placeholder, not a type: it discards the real + // state shape, so nothing downstream can be read without a cast. + getMetaMaskState: () => Record; +}; + +// Reading a field requires re-declaring its shape at the use site … +const { tokensChainsCache } = getMetaMaskState() as { + tokensChainsCache?: Record }>; }; + +// … and passing the state to a typed selector requires an `as never` to force +// the mismatch through: +getTokensControllerAllTokens({ metamask: getMetaMaskState() } as never); ``` -βœ… Derive the slice from the authoritative exported type +βœ… Type the dependency from the authoritative controller state; the field is then derivable and the selector type-checks, with no casts: ```typescript -import type { NetworkState } from '@metamask/network-controller'; +import type { TokenListState } from '@metamask/assets-controllers'; + +type Dependencies = { + // Composed from the controller-state types the module actually reads. + getMetaMaskState: () => TokenListState /* & …other controller state… */; +}; -// Tracks NetworkController's real shape (`Record`) -// and narrows to just the field in use. -type NetworkConfigurations = NetworkState['networkConfigurationsByChainId']; +const { tokensChainsCache } = getMetaMaskState(); // typed; no cast ``` -The derived form stays correct when `NetworkController` changes `networkConfigurationsByChainId`; the hand-written copy does not, and the compiler cannot tell you it has gone stale. +The `Record` did not save work β€” it moved the work downstream into a cast at every use site, including an `as never` that silently defeats the selector's parameter type. Deriving the dependency from the authoritative state removes both. ### Type Annotations