Suggestion: option to include undefined in index signatures

[OliverJAsh]

Update: fixed by --noUncheckedIndexedAccess in TypeScript 4.1

---

Update: for my latest proposal see comment #13778 (comment)

With strictNullChecks enabled, TypeScript does not include undefined in index signatures (e.g. on an object or array). This is a well known caveat and discussed in several issues, namely #9235, #13161, #12287, and #7140 (comment).

Example:

const xs: number[] = [1,2,3];
xs[100] // number, even with strictNullChecks

However, it appears from reading the above issues that many TypeScript users wish this wasn't the case. Granted, if index signatures did include undefined, code will likely require much more guarding, but—for some—this is an acceptable trade off for increased type safety.

Example of index signatures including undefined:

const xs: number[] = [1,2,3];
xs[100] // number | undefined

I would like to know whether this behaviour could be considered as an extra compiler option on top of strictNullChecks. This way, we are able to satisfy all groups of users: those who want strict null checks with or without undefined in their index signatures.

[mhegazy]

With the exception of strictNullChecks, we do not have flags that change the type system behavior. flags usually enable/disable error reporting.
you can always have a custom version of the library that defines all indexers with | undefined. should work as expected.

[OliverJAsh]

@mhegazy That's an interesting idea. Any guidance on how to override the type signatures for array/object?

[gcnew]

There isinterface Array<T> in lib.d.ts. I searched by the regexp \[\w+: (string|number)\] to find other indexing signatures as well.

[OliverJAsh]

Interesting, so I tried this:

{
    // https://github.com/Microsoft/TypeScript/blob/1f92bacdc81e7ae6706ad8776121e1db986a8b27/lib/lib.d.ts#L1300
    declare global {
        interface Array<T> {
            [n: number]: T | undefined;
        }
    }

    const xs = [1,2,3]
    const x = xs[100]
    x // still number :-(
}

Any ideas?

[mhegazy]

copy lib.d.ts locally, say lib.strict.d.ts, change the index signature to [n: number]: T | undefined;, include the file in your compilation. you should see the intended effect.

[OliverJAsh]

Cool, thanks for that.

The issue with the suggested fix here is it requires forking and maintaining a separate lib file.

I wonder if this feature is demanded enough to warrant some sort of option out of the box.

On a side note, it's interesting that the type signature for the get method on ES6 collections (Map/Set) returns T | undefined when Array/Object index signatures do not.

[mhegazy]

this is a conscious decision. it would be very annoying for this code to be an error:

var a = [];
for (var i =0; i< a.length; i++) {
    a[i]+=1; // a[i] is possibly undefined
}

and it would be unreasonable to ask every user to use !. or to write

var a = [];
for (var i =0; i< a.length; i++) {
    if (a[i]) {
        a[i]+=1; // a[i] is possibly undefined
    }
}

For map this is not the case generally.

Similarly for your types, you can specify | undefined on all your index signatures, and you will get the expected behavior. but for Array it is not reasonable. you are welcome to fork the library and make whatever changes you need to do, but we have no plans to change the declaration in the standard library at this point.

I do not think adding a flag to change the shape of a declaration is something we would do.

[Strate]

@mhegazy but for arrays with holes a[i] is actually possibly undefined:

let a: number[] = []
a[0] = 0
a[5] =0
for (let i = 0; i < a.length; i++) {
  console.log(a[i])
}

Output is:

0
undefined
undefined
undefined
undefined
0

[RyanCavanaugh]

We remain quite skeptical that anyone would get any benefit from this flag in practice. Maps and maplike things can already opt in to | undefined at their definition sites, and enforcing EULA-like behavior on array access doesn't seem like a win. We'd likely need to substantially improve CFA and type guards to make this palatable.

If someone wants to modify their lib.d.ts and fix all the downstream breaks in their own code and show what the overall diff looks like to show that this has some value proposition, we're open to that data. Alternatively if lots of people are really excited to use postfix ! more but don't yet have ample opportunities to do so, this flag would be an option.

[DanTup]

We remain quite skeptical that anyone would get any benefit from this flag in practice. Maps and maplike things can already opt in to | undefined at their definition sites

Isn't one of the goals of TypeScript to allow errors to be caught at "compile" time rather than rely on the user to remember/know to do something specific? This seems to go against that goal; requiring the user to do something in order to avoid crashes. The same could be said for many other features; they're not needed if the developer always does x. The goal of TypeScript is (presumably) to make the job easier and eliminate these things.

I came across this bug because I was enabling strictNullChecks on existing code and I already had a comparison so I got the error. If I'd been writing brand new code I probably wouldn't have realised the issue here (the type system was telling me I always always getting a value) and ended up with a runtime failure. Relying on TS developers to remember (or worse, even know) that they're supposed to be declaring all their maps with | undefined feels like TypeScript is failing to do what people actually want it for.

[kitsonk]

We remain quite skeptical that anyone would get any benefit from this flag in practice. Maps and maplike things can already opt in to | undefined at their definition sites
Isn't one of the goals of TypeScript to allow errors to be caught at "compile" time rather than rely on the user to remember/know to do something specific?

Actually the goal is:

1. Statically identify constructs that are likely to be errors.

What is being discussed here the likelyhood of an error (low in the opinion of the TypeScript team) and the common productive usability of the language. Some of the early change to CFA have been to be less alarmist or improve the CFA analysis to more intelligently determine these things.

I think the question from the TypeScript team is that instead of arguing the strictly correctness of it, to provide examples of where this sort of strictness, in common usage would actually identify an error that should be guarded against.

[RyanCavanaugh]

I went into the reasoning a bit more at this comment #11238 (comment)

Think of the two types of keys in the world: Those which you know do have a corresponding property in some object (safe), those which you don't know to have a corresponding property in some object (dangerous).

You get the first kind of key, a "safe" key, by writing correct code like

for (let i = 0; i < arr.length; i++) {
  // arr[i] is T, not T | undefined

or

for (const k of Object.keys(obj)) {
  // obj[k] is T, not T | undefined

You get the second kind from key, the "dangerous" kind, from things like user inputs, or random JSON files from disk, or some list of keys which may be present but might not be.

So if you have a key of the dangerous kind and index by it, it'd be nice to have | undefined in here. But the proposal isn't "Treat dangerous keys as dangerous", it's "Treat all keys, even safe ones, as dangerous". And once you start treating safe keys as dangerous, life really sucks. You write code like

for (let i = 0; i < arr.length; i++) {
  console.log(arr[i].name);

and TypeScript is complaining at you that arr[i] might be undefined even though hey look I just @#%#ing tested for it. Now you get in the habit of writing code like this, and it feels stupid:

for (let i = 0; i < arr.length; i++) {
  // TypeScript makes me use ! with my arrays, sad.
  console.log(arr[i]!.name);

Or maybe you write code like this:

function doSomething(myObj: T, yourObj: T) {
  for (const k of Object.keys(myObj)) {
    console.log(yourObj[k].name);
  }
}

and TypeScript says "Hey, that index expression might be | undefined, so you dutifully fix it because you've seen this error 800 times already:

function doSomething(myObj: T, yourObj: T) {
  for (const k of Object.keys(myObj)) {
    console.log(yourObj[k]!.name); // Shut up TypeScript I know what I'm doing
  }
}

But you didn't fix the bug. You meant to write Object.keys(yourObj), or maybe myObj[k]. That's the worst kind of compiler error, because it's not actually helping you in any scenario - it's only applying the same ritual to every kind of expression, without regard for whether or not it was actually more dangerous than any other expression of the same form.

I think of the old "Are you sure you want to delete this file?" dialog. If that dialog appeared every time you tried to delete a file, you would very quickly learn to hit del y when you used to hit del, and your chances of not deleting something important reset to the pre-dialog baseline. If instead the dialog only appeared when you were deleting files when they weren't going to the recycling bin, now you have meaningful safety. But we have no idea (nor could we) whether your object keys are safe or not, so showing the "Are you sure you want to index that object?" dialog every time you do it isn't likely to find bugs at a better rate than not showing it all.

[RyanCavanaugh]

Statically identify constructs that are likely to be errors.

Perhaps this needs to be amended to say "Statically identify constructs that are more likely than others to be errors." 😉. I'm reminded of when we get bugs that are essentially "I used * when I meant to use /, can you using make * a warning?"

[DanTup]

I understand, but index out of range is a real and common issue; forcing people to enumerate arrays in a way that they can't do this would not be a bad thing.

The fix with ! I actually dislike too - what if someone comes along and makes a change such that the assumption is now invalid? You're back to square one (a potential runtime failure for something that the compiler should catch). There should be safe ways of enumerating arrays that do not rely on either lying about the types or using ! (eg. can't you do something like array.forEach(i => console.log(i.name)?).

You already narrow types based on code so in theory couldn't you could spot patterns that are safe narrow the type to remove | undefined in those cases, giving best of both worlds? I'd argue that if you can't easily convey to the compiler that you're not accessing a valid element then maybe your guarantee is either invalid or could easily be accidentally be broken in future.

That said, I only use TS on one project and that will ultimately be migrated to Dart so it's unlikely to make any real difference to me. I'm just sad that the general quality of software is bad and there's an opportunity to help eliminate errors here that is seemingly being ignored for the sake of convenience. I'm sure the type system could be made solid and the common annoyances addressed in a way that doesn't introduce these holes.

Anyway, that's just my 2 cents.. I don't want to drag this out - I'm sure you understand where we're coming from and you're far better placed to make decisions on this than me :-)

[kitsonk]

I think there are a few things to consider. There are a lot of patterns for iterating over arrays in common use that account for the number of elements. While it is a possible pattern to just randomly access indexes on arrays, in the wild that is a very uncommon pattern and is not likely to be a statical error. While there are modern ways to iterate, the most common would be something like:

for (let i = 0; i < a.length; i++) {
  const value = a[i];
}

If you assume spare arrays are uncommon (they are) it is of little help to have value be | undefined. If there is a common pattern, in the wild, where this is risky (and likely an error) then I think the TypeScript would listen to consider this, but the patterns that are in general use, having to again again against all values of an index access be undefined is clearly something that affects productivity and as pointed out, can be opted into if you are in a situation where it is potentially useful.

I think there has been conversation before about improving CFA so that there is a way to express the co-dependancy of values (e.g. Array.prototype.length relates to the index value) so that things like index out of bounds could be statically analysed. Obviously that is a significant piece of work, wrought with all sorts of edge cases and considerations I wouldn't like to fathom (though it is likely Anders wakes up in a cold sweat over some things like this).

So it becomes a trade off... Without CFA improvements, complicate 90% of code with red herrings to catch potentially 10% bad code. Otherwise it is investing in major CFA improvements, which might be wrought with their own consequences of stability and issues against again, finding what would be unsafe code.

There is only so much TypeScript can do to save us from ourselves.

[DanTup]

All this focus is on arrays and I agree it's less likely to be an issue there, but most of the original issues raised (like mine) were about maps where I don't think the common case is always-existing keys at all?

[mhegazy]

All this focus is on arrays and I agree it's less likely to be an issue there, but most of the original issues raised (like mine) were about maps where I don't think the common case is always-existing keys at all?

If this is your type, add | undefined to the index signature. It is already an error to index into an type with no index signature under --noImplicitAny.
ES6 Map is already defined with get as get(key: K): V | undefined;.

[zpdDG4gta8XKpMCd]

i rewrote all definitions of Arrays and Maps to make index signatures returning | undefined, never regreted since that, found a few bugs, it doesn't cause any discomfort because i work with arrays indirectly via a handmade lib that keeps checks for undefined or ! inside of it

would be great if TypeScript could control flow the checks like C# does (to eliminate index range checks to save some processor time), for example:

declare var values: number[];
for (let index = 0, length = values.length; index< length; index ++) {
   const value = value[index]; // always defined, because index is within array range and only controlled by it
}

(to those who uses sparse arrays - kill yourself with hot burning fire)

as for Object.keys, it takes a special type say allkeysof T to let the control flow analysis do safe narrowings

[radix]

I think this would be a good option to have, because right now we are essentially lying about the type of the indexing operation, and it can be easy to forget to add | undefined to my object types. I think adding ! in the cases where we know we want to ignore undefineds would be a nice way to deal with indexing operations when this option is enabled.

[Perfectoff]

I think the best solution would be to throw an exception when trying to access a non-existent element. This is in line with normal programming practice in statically typed languages. And if you want to get a nullable value without throwing an exception, then a method like "tryGet" is used.
As I see, this behavior can be implemented by replacing the code during the compilation to the javascript file, like this:
object [index] --> (object[index] ?? throw ("index out of range"))
object.tryGet [index] --> object[index]
And, accordingly, add the tryGet method to the Array, Map and other interfaces with indexer.

[rubenlg]

@Perfectoff As a principle, TypeScript never changes the runtime behavior of JavaScript code:
https://www.typescriptlang.org/docs/handbook/typescript-from-scratch.html#runtime-behavior

What you are proposing are run-time assertions, and that could be implemented as a separate library.

[thw0rted]

It seems to me that @Perfectoff just wants to use a language that isn't Javascript. JS arrays have never had a concept of "out of bounds", Arrays have always been sparse, and Objects have always allowed arbitrary indexing. That's the whole reason undefined exists! If you want an array that can't be sparse, write one, but you're much better off learning to use iterators and forEach properly.

[AlexGalays]

It seems to me that @Perfectoff just wants to use a language that isn't Javascript. JS arrays have never had a concept of "out of bounds", Arrays have always been sparse, and Objects have always allowed arbitrary indexing. That's the whole reason undefined exists! If you want an array that can't be sparse, write one, but you're much better off learning to use iterators and forEach properly.

I see your point but almost nobody use the optional sparse capabilities of Arrays in their production applications. If types can help alleviate the historicaly poor technical choices of JS, why not do it.

[thw0rted]

If types can help alleviate the historicaly poor technical choices of JS, why not do it.

That's exactly what this issue was about! One could argue that sparse arrays are "one of the bad parts", sure. The fix for that, IMHO, is forcing you to check your indexed access, or avoid it altogether.

[nh2]

@OliverJAsh Coul dyou edit the issue description to mention that --noUncheckedIndexedAccess in TypeScript 4.1 fixes this?

The thread is so long and still continuously edited that the solution is hard to find.