Skip to content

Commit b4511bc

Browse files
MajorLiftMajorLift
committed
Add Types section
1 parent 0eab0d6 commit b4511bc

File tree

1 file changed

+303
-3
lines changed

1 file changed

+303
-3
lines changed

‎docs/typescript.md

Lines changed: 303 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,313 @@
22

33
These guidelines specifically apply to TypeScript.
44

5-
## Provide explicit return types for functions/methods
5+
## Types
6+
7+
### Type Inference
8+
9+
TypeScript is very good at inferring types. A well-maintained codebase can provide strict type safety with explicit type annotations being the exception rather than the rule.
10+
11+
When writing TypeScript, type inferences should generally be preferred over type declarations and assertions. There are several reasons for this:
12+
13+
#### Advantages
14+
15+
- Explicit type annotations (`:`) and type assertions (`as`, `!`) prevent inference-based narrowing of the user-supplied types.
16+
- The compiler errs on the side of trusting user input, which prevents it from providing additional, and sometimes crucial, type information that it could otherwise infer.
17+
<!-- TODO: Expand into entry. Add example. -->
18+
- In TypeScript v4.9+ the `satisfies` operator can be used to assign type constraints that are narrowable through type inference.
19+
- Type inferences are responsive to changes in code without requiring user input, while annotations and assertions rely on hard-coding, making them brittle against code drift.
20+
- The `as const` operator can be used to further narrow an inferred abstract type into a specific literal type.
21+
22+
🚫 Type declarations
23+
24+
```ts
25+
const name: string = 'METAMASK'; // Type 'string'
26+
27+
const BUILT_IN_NETWORKS = new Map<string, `0x${string}`>([
28+
['mainnet', '0x1'],
29+
['goerli', '0x5'],
30+
]); // Type 'Map<string, `0x${string}`>'
31+
```
32+
33+
✅ Type inferences
34+
35+
```ts
36+
const name = 'METAMASK'; // Type 'METAMASK'
37+
38+
const BUILT_IN_NETWORKS = {
39+
mainnet: '0x1',
40+
goerli: '0x5',
41+
} as const; // Type { readonly mainnet: '0x1'; readonly goerli: '0x5'; }
42+
```
43+
44+
### Type Narrowing
45+
46+
There is a clear exception to the above: if an explicit type annotation or assertion can narrow an inferred type further, thereby improving its accuracy, it should be applied.
47+
48+
##### Avoid widening a type with a type annotation
49+
50+
> **Warning**<br />
51+
> Double-check that a declared type is narrower than the inferred type.<br />
52+
> Enforcing an even wider type defeats the purpose of adding an explicit type annotation, as it _loses_ type information instead of adding it.
53+
54+
🚫
55+
56+
```ts
57+
const chainId: string = this.messagingSystem(
58+
'NetworkController:getProviderConfig',
59+
).chainId; // Type 'string'
60+
```
61+
62+
✅
63+
64+
```ts
65+
const chainId = this.messagingSystem(
66+
'NetworkController:getProviderConfig',
67+
).chainId; // Type '`0x${string}`'
68+
```
69+
70+
##### When instantiating an empty container type, provide a type annotation
71+
72+
This is one case where type inference is unable to reach a useful conclusion without user-provided information. Since the compiler cannot arbitrarily restrict the range of types that could be inserted into the container, it has to assume the widest type, which is often `any`. It's up to the user to narrow that into the intended type with an explicit type annotation.
73+
74+
🚫
75+
76+
```ts
77+
const tokens = []; // Type 'any[]'
78+
const tokensMap = new Map(); // Type 'Map<any, any>'
79+
```
80+
81+
✅
82+
83+
```ts
84+
const tokens: string[] = []; // Type 'string[]'
85+
const tokensMap = new Map<string, Token>(); // Type 'Map<string, Token>'
86+
```
87+
88+
##### Type guards and null checks can be used to improve type inference
89+
90+
<!-- TODO: Add explanation and examples -->
91+
92+
```ts
93+
function isSomeInterface(x: unknown): x is SomeInterface {
94+
return (
95+
'name' in x &&
96+
typeof x.name === 'string' &&
97+
'length' in x &&
98+
typeof x.length === 'number'
99+
);
100+
}
101+
102+
function f(x: SomeInterface | SomeOtherInterface) {
103+
if (isSomeInterface(x)) {
104+
// Type of x: 'SomeInterface | SomeOtherInterface'
105+
console.log(x.name); // Type of x: 'SomeInterface'. Type of x.name: 'string'.
106+
}
107+
}`
108+
```
109+
110+
### Type Assertions
111+
112+
`as` assertions are unsafe. They overwrite type-checked and inferred types with user-supplied types that suppress compiler errors.
113+
114+
#### Avoid `as`
115+
116+
Type assertions make the code brittle against changes. While TypeScript will throw type errors against some unsafe or structurally unsound type assertions, it will generally accept the user-supplied type without type-checking. This can cause silent failures where errors are suppressed, even though the type's relationship to the rest of the code, or the type itself, has been altered so that the type assertion is no longer valid.
117+
118+
#### Acceptable usages of `as`
119+
120+
##### To prevent or fix `any` usage
121+
122+
Unsafe as type assertions may be, they are still categorically preferable to using `any`.
123+
- With type assertions, we still get working intellisense, autocomplete, and other IDE features.
124+
- Type assertions also provide an indication of the expected type as intended by the author.
125+
- For type assertions to an incompatible shape, use `as unknown as` as a last resort rather than `any` or `as any`.
126+
127+
##### To define user-defined type guards
128+
129+
`as` syntax is often required to write type guards. See https://www.typescriptlang.org/docs/handbook/2/narrowing.html#using-type-predicates.
130+
131+
##### To type data objects whose shape and contents are determined at runtime
132+
133+
Preferably, this typing should be accompanied by schema validation performed with type guards and unit tests.
134+
- e.g. The output of `JSON.parse()` or `await response.json()` for a known JSON input.
135+
- e.g. The type of a JSON file.
136+
137+
<!-- TODO: Add example -->
138+
139+
##### In tests, for mocking or to exclude irrelevant but required properties from an input object
140+
141+
<!-- TODO: Add examples -->
142+
It's recommended to provide accurate typing if there's any chance that omitting properties affects the accuracy of the test.
143+
144+
Otherwise, only mocking the properties needed in the test improves readability by making the intention and scope of the mocking clear, not to mention being convenient to write.
145+
146+
### Compiler Directives
147+
148+
<!-- TODO: Add section for `@ts-expect-error` -->
149+
150+
#### Avoid `any`
151+
152+
`any` is the most dangerous form of explicit type declaration, and should be completely avoided if possible.
153+
154+
- `any` doesn't represent the widest type, or indeed any type at all. `any` is a compiler directive for _disabling_ static type checking for the value or type to which it's assigned.
155+
- `any` suppresses all error messages about its assignee. This includes errors that are changed or newly introduced by alterations to the code. This makes `any` the cause of dangerous **silent failures**, where the code fails at runtime but the compiler does not provide any prior warning.
156+
- `any` subsumes all other types it comes into contact with. Any type that is in a union, intersection, is a property of, or has any other relationship with an `any` type or value is erased and becomes an `any` type itself.
157+
158+
<!-- TODO: Add examples -->
159+
160+
```ts
161+
const handler:
162+
| ((payload_0: ComposableControllerState, payload_1: Patch[]) => void)
163+
| ((payload_0: any, payload_1: Patch[]) => void); // Type of 'payload_0': 'any'
164+
```
165+
166+
- `any` pollutes all surrounding and downstream code.
167+
<!-- TODO: Add examples -->
168+
169+
#### Fixes for `any`
170+
171+
##### Try `unknown` instead
172+
173+
`unknown` is the universal supertype i.e. the widest possible type.
174+
175+
- When typing the assignee, `any` and `unknown` are interchangeable (every type is assignable to both).
176+
- However, when typing the assigned, `unknown` can't be used to replace `any`, as `unknown` is only assignable to `unknown`. In this case, try `never`, which is assignable to all types.
177+
<!-- TODO: Add example -->
178+
179+
##### Don't allow generic types to use `any` as a default argument
180+
181+
Some generic types use `any` as a default generic argument. This can silently introduce an `any` type into the code, causing unexpected behavior and suppressing useful errors.
182+
183+
🚫
184+
185+
```ts
186+
const NETWORKS = new Map({
187+
mainnet: '0x1',
188+
goerli: '0x5',
189+
}); // Type 'Map<any, any>'
190+
191+
const mockGetNetworkConfigurationByNetworkClientId = jest.fn(); // Type 'jest.Mock<any, any>'
192+
mockGetNetworkConfigurationByNetworkClientId.mockImplementation(
193+
(origin, type) => {},
194+
); // No error!
195+
// Even though 'mockImplementation' should only accept callbacks with a signature of '(networkClientId: string) => NetworkConfiguration | undefined'
196+
```
197+
198+
✅
199+
200+
```ts
201+
const NETWORKS = new Map<string, `0x${string}`>({
202+
mainnet: '0x1',
203+
goerli: '0x5',
204+
}); // Type 'Map<string, `0x${string}`>'
205+
206+
const mockGetNetworkConfigurationByNetworkClientId = jest.fn<
207+
ReturnType<NetworkController['getNetworkConfigurationByNetworkClientId']>,
208+
Parameters<NetworkController['getNetworkConfigurationByNetworkClientId']>
209+
>(); // Type 'jest.Mock<NetworkConfiguration | undefined, [networkClientId: string]>'
210+
mockGetNetworkConfigurationByNetworkClientId.mockImplementation(
211+
(origin, type) => {},
212+
);
213+
// Argument of type '(origin: any, type: any) => void' is not assignable to parameter of type '(networkClientId: string) => NetworkConfiguration | undefined'.
214+
// Target signature provides too few arguments. Expected 2 or more, but got 1.ts(2345)
215+
```
216+
217+
#### Acceptable usages of `any`
218+
219+
##### Assigning new properties to a generic type at runtime
220+
221+
In most type errors involving property access or runtime property assignment, `any` usage can be avoided by substituting with `as unknown as`.
222+
223+
🚫
224+
225+
```ts
226+
for (const key of getKnownPropertyNames(this.internalConfig)) {
227+
(this as any)[key] = this.internalConfig[key];
228+
}
229+
230+
delete addressBook[chainId as any];
231+
// Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{ [chainId: `0x${string}`]: { [address: string]: AddressBookEntry; }; }'.
232+
// No index signature with a parameter of type 'string' was found on type '{ [chainId: `0x${string}`]: { [address: string]: AddressBookEntry; }; }'.ts(7053)
233+
```
234+
235+
✅
236+
237+
```ts
238+
for (const key of getKnownPropertyNames(this.config)) {
239+
(this as unknown as typeof this.config)[key] = this.config[key];
240+
}
241+
242+
delete addressBook[chainId as unknown as `0x${string}`];
243+
```
244+
245+
However, when assigning to a generic type, using `as any` is the only solution.
246+
247+
🚫
248+
249+
```ts
250+
(state as RateLimitState<RateLimitedApis>).requests[api][origin] = previous + 1;
251+
// is generic and can only be indexed for reading.ts(2862)
252+
```
253+
254+
✅
255+
256+
```ts
257+
(state as any).requests[api][origin] = previous + 1;
258+
```
259+
260+
Even in this case, however, `any` usage might be avoidable by using `Object.assign` or spread operator syntax instead of assignment.
261+
262+
```ts
263+
Object.assign(state, {
264+
requests: {
265+
...state.requests,
266+
[api]: { [origin] },
267+
},
268+
});
269+
270+
{
271+
...state,
272+
requests: {
273+
...state.requests,
274+
[api]: { [origin] },
275+
},
276+
};
277+
```
278+
279+
##### Within generic constraints
280+
281+
✅
282+
283+
```ts
284+
class BaseController<
285+
...,
286+
messenger extends RestrictedControllerMessenger<N, any, any, string, string>
287+
> ...
288+
```
289+
290+
- In general, using `any` in this context is not harmful in the same way that it is in other contexts, as the `any` types only are not directly assigned to any specific variable, and only function as constraints.
291+
- That said, more specific constraints provide better type safety and intellisense, and should be preferred wherever possible.
292+
293+
##### Catching errors
294+
295+
- `catch` only accepts `any` and `unknown` as the error type.
296+
- Recommended: Use `unknown` with type guards like `isJsonRpcError`.
297+
- Avoid typing an error object with `any` if it is passed on to be used elsewhere instead of just being thrown, as the `any` type will infect the downstream code.
298+
299+
##### In tests, for mocking or to intentionally break features
300+
301+
<!-- TODO: Add examples -->
302+
303+
## Functions
304+
305+
##### For functions and methods, provide explicit return types
6306

7307
Although TypeScript is capable of inferring return types, adding them explicitly makes it much easier for the reader to see the API from the code alone and prevents unexpected changes to the API from emerging.
8308

9309
🚫
10310

11-
```javascript
311+
```ts
12312
async function removeAccount(address: Hex) {
13313
const keyring = await this.getKeyringForAccount(address);
14314
@@ -25,7 +325,7 @@ async function removeAccount(address: Hex) {
25325

26326
✅
27327

28-
```javascript
328+
```ts
29329
async function removeAccount(address: Hex): Promise<KeyringControllerState> {
30330
const keyring = await this.getKeyringForAccount(address);
31331

0 commit comments

Comments
 (0)