From 1a8934e551fd53b7579a3f3025f03248fdce84e2 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Wed, 29 Jan 2025 15:38:19 +0100 Subject: [PATCH 1/4] feat(frontend):add guide for strict typescript settings --- .../strict-typescript-settings-guide.mdx | 437 ++++++++++++++++++ 1 file changed, 437 insertions(+) create mode 100644 develop-docs/frontend/strict-typescript-settings-guide.mdx diff --git a/develop-docs/frontend/strict-typescript-settings-guide.mdx b/develop-docs/frontend/strict-typescript-settings-guide.mdx new file mode 100644 index 0000000000000..14b4c8b43bf4c --- /dev/null +++ b/develop-docs/frontend/strict-typescript-settings-guide.mdx @@ -0,0 +1,437 @@ +--- +title: Strict TypeScript settings guide +sidebar_order: 75 +--- + +## Introduction + +We have recently turned on more strict compiler settings in the TypeScript compiler, including [noUncheckedIndexedAccess](https://www.typescriptlang.org/tsconfig/#noUncheckedIndexedAccess) and [noImplicitAny](https://www.typescriptlang.org/tsconfig/#noImplicitAny). While those settings should make it easier to write safe code in the future, it's not always clear how to proceed with existing code. + +This guide aims at explaining the most common situations & pitfalls and how to fix & avoid them. + +## Missing type annotations + +`noImplicitAny` has surfaced a lot of missing type annotations, where the compiler has automatically filled in an `any` type for us. Simple example from a react component: + +```tsx +// ❌ +const handleClick = (e) => { + e.stopPropagation() + props.onClick() +} +``` + +Here, we didn't see any error without this compiler setting, but now we get: + + + Parameter 'e' implicitly has an 'any' type. + + +In the migration, we addressed many of those problems by just adding an explicit any: + +```tsx {2} +// ⚠️ +const handleClick = (e: any) => { + e.stopPropagation() + props.onClick() +} +``` + +This is obviously not great, but since we had 3k+ errors, this was the fastest way to get this setting to be turned on. So there's a good chance you'll see many explicit any type annotations now. + +### Recommended approaches + +- Sometimes, it's enough to change `any` to `unknown` (given that enough type narrowing happens inside the function). `unknown` is totally safe to use, and if you have no errors, this is a good way forward. The above example is not one of them though. +- Look at the usages of the function and potentially inline it into one of those usages while removing the manual `:any`. You can then check via type inference what the type of e would be in that usage. You can then give it an explicit type, in this case: `React.MouseEvent` + +```tsx {diff} {2} +// ✅ +- const handleClick = (e: any) => { ++ const handleClick = (e: React.MouseEvent) => { + e.stopPropagation() + props.onClick() +} +``` + +## Missing handling for array indexed access + +`noUncheckedIndexedAccess` has revealed many cases where we "know" an element exists in an Array, but TypeScript does not know it. + +### for loops + +Inside of for loops, the access of elements is usually safe, because the index is checked. However, TypeScript doesn't know that, so we need the `!` operator: + +```ts {3} +// ❌ +for (let i = 0; i < collection.length; i++) { + const child = collection[i]!; + // ... +} +``` + +#### `for..of` + +The easiest fix is to convert this to a `for..of` loop: + +```ts +// ✅ +for (const child of collection) { + // ... +} +``` + +There is also a stylistic [typescript-eslint](https://typescript-eslint.io/rules/prefer-for-of/) rule that can catch those cases, as this arguably easier to read, too. + +#### functional access + +Another approach would be to switch to a more functional style, like `collection.map` or `collection.forEach`: + +```ts +// ✅ +collection.forEach(child => { + // ... +}) +``` + +If this is a good approach depends on what's actually happening in the body of the function, and if performance is a concern and the collection itself is large, the loop might be the better approach. + +### Accessing a specific element of an array + +Oftentimes, we need to access a specific element, mostly the first element, of an array after we've checked the length: + +```ts +// ❌ +if (collection.length > 0) { + const first = collection[0]!; + // ... +} +``` + +There are various ways to address this: + +#### Explicit item check + +We could check for existence of the item itself instead of checking for the length: + +```ts {3} +// ✅ +const first = collection[0] +if (first) { + // ... +} +``` + +`first` will be properly narrowed here, but note that the `if` check is not 100% identical to a length check. If the array contains `[null]` or `[undefined]` at the first index, we would pass the length check, but not the "existence check". + +#### Optional chaining + +We could not check the length at all and just embrace that `first` is potentially `undefined` and just continue with optional chaining: + +```ts +// ✅ +const first = collection[0] +first?.toUpperCase() +``` + +#### use `as const` for static elements + +Sometimes, we know that `collection[0]` exists because we have manually defined it: + +```ts +// ❌ +const collection = ['hello', 'world'] +const first = collection[0]! +``` + +In this case, accessing `collection[0]` is likely safe, but TypeScript doesn't know that. Arrays can be mutated later, so an `Array` is not guaranteed to have those two elements. + +The fix is to add `as const` to the definition. Const assertions are very powerful - they make sure that the most narrow type possible is inferred, and they also make the collection `readonly` (immutable): + +```ts {diff} +- const collection = ['hello', 'world'] ++ const collection = ['hello', 'world'] as const +const first = collection[0] +``` + +#### use a type-narrowing length check + +Especially for checking the first element, which is arguably the most common case, having a descriptive helper function that checks if lists are "non empty" is possible to implement at runtime and on type level with a user-defined type-guard: + +```ts + +type NonEmptyArray = readonly [T, ...ReadonlyArray] + +export const isNonEmpty = ( + array: ReadonlyArray | undefined, +): array is NonEmptyArray => !!array && array.length > 0; +``` + +Since this user-defined type-guard will narrow our array to be a NonEmptyArray, we can now continue to use a length check as before: + +```ts +// ✅ +if (isNonEmpty(collection)) { + const first = collection[0]; + // ... +} +``` + +## Iterating over indexed objects + +Iterating over every item in an object has similar drawbacks of a for loop. Traditionally, code may look like this: + +``` +// ❌ +function doSomething(input: Record) { + Object.keys(input).forEach(key => { + const item = input[key]! + // ... + } +} +``` + +We'd expect `item` to never be `undefined` here, but with `noUncheckedIndexedAccess`, Typescript won't know that, as `key` is not narrowed to `keyof typeof input`. + +The best solution here would be to switch to `Object.entries` or `Object.values` to get access to the `item` directly: + +``` +// ✅ +function doSomething(input: Record) { + Object.entries(input).forEach(([key, item]) => { + // ... + } +} +``` + +## Expression ... can't be used to index type ... + +A very common error we had to disable with `ts-expect-error` during the migration is: + + +TS7053: Element implicitly has an any type because expression of type ... can't be used to index type ... + + +This most commonly happens because we have a structure without an index signature - an object with a "fixed" set of fields: + +```ts +interface Person { + firstName: string, + lastName: string +} +``` + +Similar to the error seen when [Iterating over indexed objects](#iterating-over-indexed-objects), iterating over those objects with `Object.keys` won't work: + +```ts +// ❌ +function print(person: Person) { + Object.keys(person).forEach(key => { + console.log(person[key]) + }) +} +``` + +And we also can't do lookups with arbitrary string keys: + +``` +// ❌ +function printKey(person: Person, key: string) { + console.log(person[key]) +} +``` + +The error we are getting in these situations is: + + +TS7053: Element implicitly has an any type because expression of type string can't be used to index type Person + +No index signature with a parameter of type string was found on type Person + + +While this is similar to the error before, it can't easily be solved with a non-null assertion - it's not even solvable with `key as any`. Typescript simply won't allow accessing a seemingly random key on an object that doesn't have an index signature. + +How we address this problem depends on our intent: + +### Strict lookups + +The easiest way is to disallow lookups by `string`. If we change our `printKey` function to accept a more narrow index type: + +```ts +// ✅ +function printKey(person: Person, key: keyof Person) { + console.log(person[key]); +} +``` + +Of course, this might lead to errors on the call-sides if we have strings there that might potentially not be a valid key. + +### Fallback lookups + +Sometimes, we want to allow arbitrary strings to be passed in, e.g. because we get them from an endpoint that is typed as `string`. In those cases, we might want to just leverage that JavaScript will return `undefined` at runtime: + +```ts +// ❌ +function printKey(person: Person, key: string) { + console.log(person[key]?.toUpperCase() ?? 'N/A'); +} +``` + +While this works fine at runtime and is also quite easy to read and understand, TypeScript doesn't like it, and there is no easy way to narrow `key`. A type assertion might help: + +```ts +// ⚠️ +function printKey(person: Person, key: string) { + console.log(person[key as keyof Person]?.toUpperCase() ?? 'N/A'); +} +``` + +But it has the drawback that the optional chaining might be seen as unnecessary by tools like `eslint`. That's because `person[key as keyof Person]` returns `string`, not `string | undefined`. The correct type assertion would be quite verbose: + +```tsx +// ✅ +function printKey(person: Person, key: string) { + console.log( + (person[key as keyof Person] as string | undefined)?.toUpperCase() ?? 'N/A' + ); +} +``` + +so I don't think we want that littered in our code-base. + + +### Narrowing with the `in` operator + +The `in` operator is good for checks at runtime, and it also narrows the type correctly if we check for a hard-coded field. However, it can't narrow for arbitrary strings like `key:string` , so this also won't work: + +```ts +// ❌ +function printKey(person: Person, key: string) { + console.log(key in person ? person[key].toUpperCase() : 'N/A'); +} +``` + +We need to assert the key here again, but at least this time, the assertion would be safe: + +```ts +// ✅ +function printKey(person: Person, key: string) { + console.log(key in person ? person[key as keyof Person].toUpperCase() : 'N/A'); +} +``` + +This pattern seems like the best approach, if we apply it sparingly. If we need it more often, a `hasKey` helper function to narrow keys might be a good idea: + +```ts + const hasKey = >( + obj: T, + key: PropertyKey + ): key is keyof typeof obj => { + return key in obj; + }; +``` + +With this, we could do: + +```ts +// ✅ +function printKey(person: Person, key: string) { + console.log(hasKey(person, key) ? person[key].toUpperCase() : 'N/A'); +} +``` + +Which isn't terrible. + +## Partial object lookup + +Another common issue revolves around defining mapping objects with a finite set of keys that aren't exhaustive. As a real-life example, let's look at [this `PRODUCTS` mapping](https://github.com/getsentry/getsentry/blob/ee6c3b24603ca244767e78bcc9471c0238ddf328/static/getsentry/gsApp/components/productTrial/productTrialAlert.tsx#L26-L34): + +```ts +const PRODUCTS = { + [DataCategory.ERRORS]: 'Error Monitoring', + [DataCategory.TRANSACTIONS]: 'Performance Monitoring', + [DataCategory.REPLAYS]: 'Session Replay', + [DataCategory.PROFILES]: 'Profiling', + [DataCategory.MONITOR_SEATS]: 'Crons', + [DataCategory.ATTACHMENTS]: 'Attachments', + [DataCategory.SPANS]: 'Spans', +}; +``` + +We then might reasonably accept that we could access `Products` with a variable that is of type `DataCategory` + +```ts +// ❌ +function getProduct(category: DataCategory) { + return PRODUCTS[category]; +} +``` + +But `PRODUCTS` doesn't contain all DataCategories - it's not exhaustive. We're getting the following error: + +```ts +// ❌ +function getProduct(category: DataCategory) { + return PRODUCTS[category]; +} +``` + + +TS7053: Element implicitly has an any type because expression of type DataCategory can't be used to index type + +Property '[DataCategory. PROFILE_DURATION]' does not exist on type ... + + +### Solution 1: Make the mapping exhaustive + +If our intention was to have an exhaustive mapping - every `DataCategory` should be in the `PRODUCTS`, we should ensure this with `satisfies`: + +```tsx {10} +// ✅ +const PRODUCTS = { + [DataCategory.ERRORS]: 'Error Monitoring', + [DataCategory.TRANSACTIONS]: 'Performance Monitoring', + [DataCategory.REPLAYS]: 'Session Replay', + [DataCategory.PROFILES]: 'Profiling', + [DataCategory.MONITOR_SEATS]: 'Crons', + [DataCategory.ATTACHMENTS]: 'Attachments', + [DataCategory.SPANS]: 'Spans', +} satisfies Record; +``` + +This will give us a good error in the right place if our enum grows: + + +TS1360: Type ... does not satisfy the expected type `Record` + +Type ... is missing the following properties from type `Record`: profileDuration, spansIndexed, profileChunks + + +### Solution 2: Define the object as `Partial` + +If our intention was to not have all `DataCategories` mapped, we have to define our mapping object as `Partial`: + +```tsx +// ✅ +const PRODUCTS: Partial> = { + // ... +} +``` + +Accessing with a `DataCategory` will then be possible, however, accessing *any field* will return `string | undefined`. This isn't great if we know that we've put a certain key into our mapping and we're accessing it statically, e.g. in tests: + +```tsx +// ❌ +const errorProduct = PRODUCTS[DataCategory.ERRORS].toUpperCase() +``` + +This won't work because we might be accessing `.toUpperCase()` on `undefined` , e.g. if we remove `DataCategory.ERRORS` from our Partial mapping. + +For tests, `!` might be good enough, but for runtime code, just embracing the fact that no category is guaranteed to be in the mapping is likely best, so let's use optional chaining then: + +```tsx +// ✅ +const errorProduct = PRODUCTS[DataCategory.ERRORS]?.toUpperCase() + +// ⚠️ +const errorProduct = PRODUCTS[DataCategory.ERRORS]!.toUpperCase() +``` From d75666f54c2ee5221823f5a43bb10107e7a20924 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Mon, 10 Feb 2025 14:18:10 +0100 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: Alex Krawiec --- .../strict-typescript-settings-guide.mdx | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/develop-docs/frontend/strict-typescript-settings-guide.mdx b/develop-docs/frontend/strict-typescript-settings-guide.mdx index 14b4c8b43bf4c..604a44d621965 100644 --- a/develop-docs/frontend/strict-typescript-settings-guide.mdx +++ b/develop-docs/frontend/strict-typescript-settings-guide.mdx @@ -1,15 +1,15 @@ --- -title: Strict TypeScript settings guide +title: Strict TypeScript Settings Guide sidebar_order: 75 --- ## Introduction -We have recently turned on more strict compiler settings in the TypeScript compiler, including [noUncheckedIndexedAccess](https://www.typescriptlang.org/tsconfig/#noUncheckedIndexedAccess) and [noImplicitAny](https://www.typescriptlang.org/tsconfig/#noImplicitAny). While those settings should make it easier to write safe code in the future, it's not always clear how to proceed with existing code. +We have recently turned on stricter settings in the TypeScript compiler, including [noUncheckedIndexedAccess](https://www.typescriptlang.org/tsconfig/#noUncheckedIndexedAccess) and [noImplicitAny](https://www.typescriptlang.org/tsconfig/#noImplicitAny). While those settings should make it easier to write safe code in the future, it's not always clear how to proceed with existing code. -This guide aims at explaining the most common situations & pitfalls and how to fix & avoid them. +This guide aims at explaining the most common situations and pitfalls, as well as how to fix and avoid them. -## Missing type annotations +## Missing Type Annotations `noImplicitAny` has surfaced a lot of missing type annotations, where the compiler has automatically filled in an `any` type for us. Simple example from a react component: @@ -21,13 +21,13 @@ const handleClick = (e) => { } ``` -Here, we didn't see any error without this compiler setting, but now we get: +Here, we didn't see any errors with the old compiler settings, but now we get: Parameter 'e' implicitly has an 'any' type. -In the migration, we addressed many of those problems by just adding an explicit any: +In the migration, we addressed many of those problems by just adding an explicit `any`: ```tsx {2} // ⚠️ @@ -37,9 +37,9 @@ const handleClick = (e: any) => { } ``` -This is obviously not great, but since we had 3k+ errors, this was the fastest way to get this setting to be turned on. So there's a good chance you'll see many explicit any type annotations now. +This obviously isn't great, but since we had 3k+ errors, this was the fastest way to get this setting up and running. So there's a good chance you'll see many explicit `any` type annotations now. -### Recommended approaches +### Recommended Approaches - Sometimes, it's enough to change `any` to `unknown` (given that enough type narrowing happens inside the function). `unknown` is totally safe to use, and if you have no errors, this is a good way forward. The above example is not one of them though. - Look at the usages of the function and potentially inline it into one of those usages while removing the manual `:any`. You can then check via type inference what the type of e would be in that usage. You can then give it an explicit type, in this case: `React.MouseEvent` @@ -53,11 +53,11 @@ This is obviously not great, but since we had 3k+ errors, this was the fastest w } ``` -## Missing handling for array indexed access +## Missing Handling for Array Indexed Access `noUncheckedIndexedAccess` has revealed many cases where we "know" an element exists in an Array, but TypeScript does not know it. -### for loops +### For Loops Inside of for loops, the access of elements is usually safe, because the index is checked. However, TypeScript doesn't know that, so we need the `!` operator: @@ -82,7 +82,7 @@ for (const child of collection) { There is also a stylistic [typescript-eslint](https://typescript-eslint.io/rules/prefer-for-of/) rule that can catch those cases, as this arguably easier to read, too. -#### functional access +#### Functional Access Another approach would be to switch to a more functional style, like `collection.map` or `collection.forEach`: @@ -93,11 +93,11 @@ collection.forEach(child => { }) ``` -If this is a good approach depends on what's actually happening in the body of the function, and if performance is a concern and the collection itself is large, the loop might be the better approach. +Whether or not this is a good approach depends on what's actually happening in the body of the function, as well as the size of the collection and how much you're concerned bout performance. -### Accessing a specific element of an array +### Accessing a Specific Element of an Array -Oftentimes, we need to access a specific element, mostly the first element, of an array after we've checked the length: +Oftentimes, we need to access a specific element, usually the first element, of an array after we've checked the length: ```ts // ❌ @@ -109,7 +109,7 @@ if (collection.length > 0) { There are various ways to address this: -#### Explicit item check +#### Explicit Item Check We could check for existence of the item itself instead of checking for the length: @@ -123,7 +123,7 @@ if (first) { `first` will be properly narrowed here, but note that the `if` check is not 100% identical to a length check. If the array contains `[null]` or `[undefined]` at the first index, we would pass the length check, but not the "existence check". -#### Optional chaining +#### Optional Chaining We could not check the length at all and just embrace that `first` is potentially `undefined` and just continue with optional chaining: @@ -133,7 +133,7 @@ const first = collection[0] first?.toUpperCase() ``` -#### use `as const` for static elements +#### Use `as const` for Static Elements Sometimes, we know that `collection[0]` exists because we have manually defined it: @@ -153,7 +153,7 @@ The fix is to add `as const` to the definition. Const assertions are very powerf const first = collection[0] ``` -#### use a type-narrowing length check +#### Use a Type-Narrowing Length Check Especially for checking the first element, which is arguably the most common case, having a descriptive helper function that checks if lists are "non empty" is possible to implement at runtime and on type level with a user-defined type-guard: @@ -176,9 +176,9 @@ if (isNonEmpty(collection)) { } ``` -## Iterating over indexed objects +## Iterating Over Indexed Objects -Iterating over every item in an object has similar drawbacks of a for loop. Traditionally, code may look like this: +Iterating over every item in an object has similar drawbacks to a for loop. Traditionally, code may look like this: ``` // ❌ @@ -252,7 +252,7 @@ While this is similar to the error before, it can't easily be solved with a non- How we address this problem depends on our intent: -### Strict lookups +### Strict Lookups The easiest way is to disallow lookups by `string`. If we change our `printKey` function to accept a more narrow index type: @@ -265,7 +265,7 @@ function printKey(person: Person, key: keyof Person) { Of course, this might lead to errors on the call-sides if we have strings there that might potentially not be a valid key. -### Fallback lookups +### Fallback Lookups Sometimes, we want to allow arbitrary strings to be passed in, e.g. because we get them from an endpoint that is typed as `string`. In those cases, we might want to just leverage that JavaScript will return `undefined` at runtime: @@ -299,7 +299,7 @@ function printKey(person: Person, key: string) { so I don't think we want that littered in our code-base. -### Narrowing with the `in` operator +### Narrowing With the `in` Operator The `in` operator is good for checks at runtime, and it also narrows the type correctly if we check for a hard-coded field. However, it can't narrow for arbitrary strings like `key:string` , so this also won't work: @@ -341,7 +341,7 @@ function printKey(person: Person, key: string) { Which isn't terrible. -## Partial object lookup +## Partial Object Lookup Another common issue revolves around defining mapping objects with a finite set of keys that aren't exhaustive. As a real-life example, let's look at [this `PRODUCTS` mapping](https://github.com/getsentry/getsentry/blob/ee6c3b24603ca244767e78bcc9471c0238ddf328/static/getsentry/gsApp/components/productTrial/productTrialAlert.tsx#L26-L34): @@ -381,7 +381,7 @@ TS7053: Element implicitly has an any type because expression of type DataCatego Property '[DataCategory. PROFILE_DURATION]' does not exist on type ... -### Solution 1: Make the mapping exhaustive +### Solution 1: Make the Mapping Exhaustive If our intention was to have an exhaustive mapping - every `DataCategory` should be in the `PRODUCTS`, we should ensure this with `satisfies`: @@ -406,7 +406,7 @@ TS1360: Type ... does not satisfy the expected type `Record`: profileDuration, spansIndexed, profileChunks -### Solution 2: Define the object as `Partial` +### Solution 2: Define the Object as `Partial` If our intention was to not have all `DataCategories` mapped, we have to define our mapping object as `Partial`: @@ -426,7 +426,7 @@ const errorProduct = PRODUCTS[DataCategory.ERRORS].toUpperCase() This won't work because we might be accessing `.toUpperCase()` on `undefined` , e.g. if we remove `DataCategory.ERRORS` from our Partial mapping. -For tests, `!` might be good enough, but for runtime code, just embracing the fact that no category is guaranteed to be in the mapping is likely best, so let's use optional chaining then: +For tests, `!` might be good enough, but for runtime code, just embracing the fact that no category is guaranteed to be in the mapping is likely best, so we recommend optional chaining: ```tsx // ✅ From ca74852796254f9cc221697240f00982eef10f4a Mon Sep 17 00:00:00 2001 From: TkDodo Date: Mon, 10 Feb 2025 14:21:19 +0100 Subject: [PATCH 3/4] fix: add code block around variable --- develop-docs/frontend/strict-typescript-settings-guide.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/develop-docs/frontend/strict-typescript-settings-guide.mdx b/develop-docs/frontend/strict-typescript-settings-guide.mdx index 604a44d621965..1ec87c1b468da 100644 --- a/develop-docs/frontend/strict-typescript-settings-guide.mdx +++ b/develop-docs/frontend/strict-typescript-settings-guide.mdx @@ -42,7 +42,7 @@ This obviously isn't great, but since we had 3k+ errors, this was the fastest wa ### Recommended Approaches - Sometimes, it's enough to change `any` to `unknown` (given that enough type narrowing happens inside the function). `unknown` is totally safe to use, and if you have no errors, this is a good way forward. The above example is not one of them though. -- Look at the usages of the function and potentially inline it into one of those usages while removing the manual `:any`. You can then check via type inference what the type of e would be in that usage. You can then give it an explicit type, in this case: `React.MouseEvent` +- Look at the usages of the function and potentially inline it into one of those usages while removing the manual `:any`. You can then check via type inference what the type of `e` would be in that usage. You can then give it an explicit type, in this case: `React.MouseEvent` ```tsx {diff} {2} // ✅ From bcd6b6d32d64807e81e957a257a191f7951a2bb4 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Mon, 10 Feb 2025 14:27:25 +0100 Subject: [PATCH 4/4] fix: better description of unknown --- .../frontend/strict-typescript-settings-guide.mdx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/develop-docs/frontend/strict-typescript-settings-guide.mdx b/develop-docs/frontend/strict-typescript-settings-guide.mdx index 1ec87c1b468da..3ddf260fa285c 100644 --- a/develop-docs/frontend/strict-typescript-settings-guide.mdx +++ b/develop-docs/frontend/strict-typescript-settings-guide.mdx @@ -31,8 +31,8 @@ In the migration, we addressed many of those problems by just adding an explicit ```tsx {2} // ⚠️ -const handleClick = (e: any) => { - e.stopPropagation() +const handleClick = (event: any) => { + event.stopPropagation() props.onClick() } ``` @@ -41,13 +41,13 @@ This obviously isn't great, but since we had 3k+ errors, this was the fastest wa ### Recommended Approaches -- Sometimes, it's enough to change `any` to `unknown` (given that enough type narrowing happens inside the function). `unknown` is totally safe to use, and if you have no errors, this is a good way forward. The above example is not one of them though. -- Look at the usages of the function and potentially inline it into one of those usages while removing the manual `:any`. You can then check via type inference what the type of `e` would be in that usage. You can then give it an explicit type, in this case: `React.MouseEvent` +- Sometimes, it's enough to change `any` to `unknown` (given that enough type narrowing happens inside the function). `unknown` is [the type-safe counterpart](https://www.typescriptlang.org/docs/handbook/2/functions.html#unknown) of `any`, because it isn't assignable to anything but itself and `any` without a type assertion or a control flow based narrowing. If you have no errors, this is a good way forward. The above example is not one of them though, because we want to call the `stopPropagation` function on the `event` variable. +- Look at the usages of the function and potentially inline it into one of those usages while removing the manual `:any`. You can then check via type inference what the type of `event` would be in that usage. You can then give it an explicit type, in this case: `React.MouseEvent` ```tsx {diff} {2} // ✅ -- const handleClick = (e: any) => { -+ const handleClick = (e: React.MouseEvent) => { +- const handleClick = (event: any) => { ++ const handleClick = (event: React.MouseEvent) => { e.stopPropagation() props.onClick() }