microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
99.86k stars 12.36k forks source link

noUncheckedIndexAccess - disable per type or property (use case: dynamic Proxy which returns value for every prop) #47594

Open falkenhawk opened 2 years ago

falkenhawk commented 2 years ago

Suggestion

noUncheckedIndexAccess - disable per type or property (use case: dynamic Proxy which returns value for every prop)

🔍 Search Terms

noUncheckedIndexAccess, proxy, disable

✅ Viability Checklist

My suggestion meets these guidelines:

⭐ Suggestion

Now that #13778 is implemented and it works nicely for most of the cases, increasing type safety, I would like to ask if you would consider providing a way to disable it on a type/prop level.

📃 Motivating Example

We like the new rule and we enabled it in our codebase. But in some specific cases (e.g. a dynamic Proxy which always returns a value for any property or any dynamic getter) we would like to have a possibility to disable/override it.

I provided a simple example below, but we use it to a greater extent incl. nested proxies, dynamic tree-like structures etc. all the dynamic magic stuff javascript is meant for 🙈

In order to improve DX we would also like to avoid sprinkling non-null assertion operators everywhere we use them, like const a = foo.something()!.nested()!.etc!

I tried overriding index signature types with NonNullable, Required, Exclude, index signature modifier -? etc. but no luck.

💻 Use Cases

In this simple example, foo is a proxy which returns name of dynamically accessed property as string. It will always be a string, so undefined should be excluded.

// tsconfig.json
{ "compilerOptions": { "noUncheckedIndexAccess": true } }

interface DynamicProxy {
  [key: string]: string;
}

// Proxy which always returns string value for any property
const handlers: ProxyHandler<DynamicProxy> = {
  get(target, propKey, receiver) {
    if (typeof propKey === 'symbol') return propKey.toString();
    return propKey;
  },
}

const foo = new Proxy<DynamicProxy>({}, handlers);

const something: string = foo.something;
// Type 'string | undefined' is not assignable to type 'string'.
//  Type 'undefined' is not assignable to type 'string'.(2322)

[ playground ]

freshtonic commented 2 years ago

This is the exact same use case I currently have.

I really want this feature. Essentially it boils down to being able to declare a type that guarantees that it has ALL properties of type string, and and runtime the dereference will be handled by a proxy that can then do some work at runtime to achieve our desired outcome.

nartallax commented 1 year ago

Other (now closed) issue mentioned another, far more common use-case (which I'm currently having): CSS module imports. For those who never used it: many modern packagers allow you to import css/less/sass file and reference CSS classes from this file; this helps with minification/isolation. Typescript does not explicitly supports that; but there's a boilerplate that fixes it. It looks roughly like this (it's .d.ts):

declare module "*.module.scss" {
    const css: {readonly [key: string]: string}
    export = css
}

That allows to import scss files like this:

import * as css from "./my_component.module.scss"

And refer to classes in that file like this:

let myMangledClassName = css.myClassName

The thing is, I don't want for css.myClassName to return undefined, because my packager (Parcel, not sure if anything else supports it) is able to check on build if myClassName is really present in css file or not.

So making it string | undefined is excessive; I'd rather make it just string and rely on Parcel to check this part.

laverdet commented 1 year ago

@nartallax CSS modules was my last blocker for this feature as well. We ended up using css-modules-typescript-loader which is a Webpack loader that emits .d.ts files for each CSS file in your project. It takes some getting used to but the safety of noUncheckedIndexAccess was worth it for us.

deenns commented 1 year ago

I also use Proxy and without this feature the noUncheckedIndexAccess flag spoils developer experience. Besides Proxy it's also needed when one exports env variables as constants (! cannot be used here), e.g.

const { ONE, TWO, THREE } = process.env as Required<Record<string, string>>;

Unfortunately, in the example above the Required utility type doesn't work. And it would be nice if it could override the noUncheckedIndexAccess flag.

Currently, without the noUncheckedIndexAccess flag, if you write

const record: Partial<Record<string, string>> = {};
const value = record['prop'];

value will be string | undefined. That is, with Partial you can enable noUncheckedIndexAccess for a certain type.

However, if you add the noUncheckedIndexAccess flag and use Required, then

const record: Required<Record<string, string>> = {};
const value = record['prop'];

value still will be string | undefined. But it would be logical to make it just string, since in the docs it is said that Required is the opposite of Partial.

Maybe the previous example isn't completely correct, since {} is definitely not a proper record, but a type assertion should work

const record = {} as Required<Record<string, string>>;
matthew-dean commented 1 year ago

However, if you add the noUncheckedIndexAccess flag and use Required, then value still will be string | undefined

I'm wondering if this can be fixed as well. One should be able to explicitly state for a type, that every indexed key is valid and will return a value other than undefined.

jukkahyv commented 1 month ago

This might also make it easier to gradually enable noUncheckedIndexedAccess. I really like the setting, but when I tried turning it on, I got 100+ compilation errors. Not something that can be fixed in a single PR.