Suggestion: option to include undefined in index signatures

[OliverJAsh]

Update: fixed by --noUncheckedIndexedAccess in TypeScript 4.1

---

Update: for my latest proposal see comment https://github.com/Microsoft/TypeScript/issues/13778#issuecomment-406316164

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 https://github.com/Microsoft/TypeScript/issues/9235, https://github.com/Microsoft/TypeScript/issues/13161, https://github.com/Microsoft/TypeScript/issues/12287, and https://github.com/Microsoft/TypeScript/pull/7140#issuecomment-192606629.

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 https://github.com/Microsoft/TypeScript/issues/11238#issuecomment-250562397

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." :wink:. 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.

[radix]

There are (at least) two other problems with putting |undefined in your object type definitions:

• it means you can assign undefined into these objects, which is definitely not intended
• other operations, like Object.values (or _.values) will require you to handle undefined in the results

[ypresto]

tslint reports false-positive warning of constant condition, because typescript returns wrong type information (= lacking | undefined). https://github.com/palantir/tslint/issues/2944

[sompylasar]

One of the regularly skipped errors with the absence of | undefined in the array indexing is this pattern when used in place of find:

const array = [ 1, 2, 3 ];
const firstFour = array.filter((x) => (x === 4))[0];
// if there is no `4` in the `array`,
// `firstFour` will be `undefined`, but TypeScript thinks `number` because of the indexer signature.

const array = [ 1, 2, 3 ];
const firstFour = array.find((x) => (x === 4));
// `firstFour` will be correctly typed as `number | undefined` because of the `find` signature.

[felixfbecker]

I would definitely use this flag. Yes, old for loops will be annoying to work with, but we have the ! operator to tell the compiler when we know it's defined:

for (let i = 0; i < arr.length; i++) {
  foo(arr[i]!)
}

Also, this problem is not a problem with the newer, way better for of loops and there is even a TSLint rule prefer-for-of that tells you to not use old-style for loops anymore.

Currently I feel like the type system is inconsistent for the developer. array.pop() requires an if check or a ! assertion, but accessing via [array.length - 1] does not. ES6 map.get() requires an if check or a ! assertion, but an object hash does not. @sompylasar's example is also good.

Another example is destructuring:

const specifier = 'Microsoft/TypeScript'
const [repo, revision] = specifier.split('@') // types of repo and revision are string
console.log('Repo: ' + repo)
console.log('Short rev: ' + revision.slice(0, 7)) // Error: Cannot call function 'slice' on undefined

I would have preferred if the compiler forced me to do this:

const specifier = 'Microsoft/TypeScript'
const [repo, revision] = specifier.split('@') // types of repo and revision are string | undefined
console.log('Repo: ', repo || 'no repo')
console.log('Short rev:', revision ? revision.slice(0, 7) : 'no revision')

These are actual bugs I've seen that could have been prevented by the compiler.

Imo this shouldn't belong into the typings files, but should rather be a type system mechanic - when accessing anything with an index signature, it can be undefined. If your logic ensured that it isn't, just use !. Otherwise add an if and you're good.

I think a lot of people would prefer the compiler to be strict with some needed assertions than to be loose with uncaught bugs.

[pelotom]

I'd really like to see this flag added. In my company's code base, array random access is the rare exception and for loops are code smells that we'd usually want to rewrite with higher-order functions.

[zpdDG4gta8XKpMCd]

@pelotom what is your concern then (since it seems you mostly got yourself out of trouble)?

[pelotom]

@aleksey-bykov mostly object index signatures, which occur extensively in third-party libraries. I would like accessing a property on { [k: string]: A } to warn me that the result is possibly undefined. I only mentioned array indexing because it was brought up as a case for why the flag would be too annoying to work with.

[zpdDG4gta8XKpMCd]

you know you can rewrite them exactly the way you want? (given a bit of extra work)

[pelotom]

Yes, I could rewrite everyone's typings for them, or I could switch on a compiler flag 😜

[zpdDG4gta8XKpMCd]

keep playing captain O...: you can rewrite your lib.d.ts today and be a happy owner of more sound codebase or you can wait for the flag for the next N years

[pelotom]

@aleksey-bykov how can it be done by rewriting lib.d.ts?

[zpdDG4gta8XKpMCd]

declare type Keyed<T> = { [key: string]: T | undefined; }

[zpdDG4gta8XKpMCd]

then in the Array defintion in lib.es2015.core.d.ts, replace

[n: number]: T;

with

[n: number]: T | undefined;

[pelotom]

@aleksey-bykov maybe you missed the part where I said I don't care about arrays. I care about where third party libraries have declared something to be of type { [k: string]: T }, and I want accessing such an object to return something possibly undefined. There's no way to accomplish that by simply editing lib.d.ts; it requires changing the signatures of the library in question.

[zpdDG4gta8XKpMCd]

do you have control over 3rd party definition files? if so you can fix them

[pelotom]

And now we're back to

Yes, I could rewrite everyone's typings for them, or I could switch on a compiler flag 😜

Time is a flat circle.

[zpdDG4gta8XKpMCd]

don't be silly, you don't use "everyone's typings" do you? it's literally a day of work max for a typical project, been there done it

[pelotom]

Yes, I have to edit others' typings all the time and I'd like to do it less.

[zpdDG4gta8XKpMCd]

and you will in N years, maybe, for now you can suffer or man up

[pelotom]

Thanks for your incredibly constructive input 👍

[zpdDG4gta8XKpMCd]

constructive input for this is as follows, this issue needs to be closed, because: a. either the decision on whether [x] can be undefined or not is left to the developers by

• letting them keep it all in their heads as they always did before
• or by altering lib.d.ts and 3rd-party.d.ts as was suggested

b. or it takes special syntax / types / flow analysis / N years to mitigate something that can be easily done by hands in #a

[pelotom]

The issue is a proposal for (b), except no new syntax is being proposed, it's just a compiler flag.

What it comes down to is that the type { [x: string]: {} } is almost always a lie; barring the use of Proxy, there's no object which can have an infinite number of properties, much less every possible string. The proposal is to have a compiler flag which recognizes this. It may be that it's too hard to implement this for what is gained; I'll leave that call to the implementors.

[zpdDG4gta8XKpMCd]

the point is that neither

• T | undefined
• nor T

is right for the general case

in order to make it right for the general case you need to encode the information about the prerense of values into the types of their containers which calls for a dependent type system ... which by itself isn't a bad thing to have :) but might be as complex as all current typescript type system done to this day, for the sake of ... saving you some edits?

[pelotom]

T | undefined is correct for the general case, for reasons I just gave. Gonna ignore your nonsensical ramblings about dependent types, have a nice day.

[zpdDG4gta8XKpMCd]

you can ignore me as much as you want but T | undefined is an overshoot for

declare var items: number[];
for (var index = 0; index < items.length; index ++) {
   void items[index];
}

[sompylasar]

I'd rather have T | undefined there by default and tell the compiler that index is a numeric index range of items thus doesn't get out if bounds when applied to items; in the simple cases such as a set of frequently used for/while loop shapes, the compiler could infer that automatically; in complex cases, sorry, there can be undefineds. And yes, value-based types would be a good fit here; literal string types are so useful, why not have literal boolean and number and range/set-of-ranges types? As far as TypeScript goes, it tries to cover everything that can be expressed with JavaScript (in contrast to, for example, Elm which limits that).

[mhegazy]

it's literally a day of work max for a typical project, been there done it

@aleksey-bykov, curious what was your experience after that change? how often do you have to use !? and how often do you find the compiler flagging actual bugs?

[zpdDG4gta8XKpMCd]

@mhegazy honestly i didn't notice much difference moving from T to T | undefined, neither did i catch any bugs, i guess my problem is that i work with arrays via utility functions which keep ! in them, so literally there was no effect for the outside code:

https://github.com/aleksey-bykov/basic/blob/master/array.ts

[OliverJAsh]

In which lib file can I find the index type definition for objects? I have located and updated Array from [n: number]: T to [n: number]: T | undefined. Now I would like to do the same thing for objects.

[zpdDG4gta8XKpMCd]

there is no standard interface (like Array for arrays) for objects with the index signature, you need to look for exact definitions per each case in your code and fix them

[OliverJAsh]

you need to look for exact definitions per each case in your code and fix them

How about a direct key lookup? E.g.

const xs = { foo: 'bar' }
xs['foo']

Is there any way to enforce T | undefined instead of T here? Currently I use these helpers in my codebase everywhere, as type safe alternatives to index lookups on arrays and objects:

// TS doesn't return the correct type for array and object index signatures. It returns `T` instead
// of `T | undefined`. These helpers give us the correct type.
// https://github.com/Microsoft/TypeScript/issues/13778
export const getIndex = function<X> (index: number, xs: X[]): X | undefined {
  return xs[index];
};
export const getKeyInMap = function<X> (key: string, xs: { [key: string]: X }): X | undefined {
  return xs[key];
};

---

@mhegazy As I write this, I am fixing a bug in production on https://unsplash.com that could have been caught with stricter index signature types.