TypeScript: Suggestion: option to include undefined in index signatures
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.
About this issue
- Original URL
- State: closed
- Created 7 years ago
- Reactions: 713
- Comments: 248 (49 by maintainers)
Links to this issue
Commits related to this issue
- Force type of array index to include "undefined" TypeScript seems to assume that array indexing always returns the array type, but it can also be undefined. Issue: https://github.com/microsoft/TypeSc... — committed to owid/owid-grapher by danielgavrilov 4 years ago
- Force type of array index to include "undefined" TypeScript seems to assume that array indexing always returns the array type, but it can also be undefined. Issue: https://github.com/microsoft/TypeSc... — committed to owid/owid-grapher by danielgavrilov 4 years ago
- feat(entity): add undefined to Dictionary's index signature (#1719) BREAKING CHANGE: Dictionary could be producing undefined but previous typings were not explicit about it. — committed to ngrx/platform by alex-okrushko 5 years ago
- fixup! chore(node): apply lint — committed to DefinitelyTyped/DefinitelyTyped by SimonSchick 6 years ago
- feat: StringMap values are now "nullable" As we believe it's more safe this way. As per: https://github.com/microsoft/TypeScript/issues/13778 https://github.com/DefinitelyTyped/DefinitelyTyped/pull/... — committed to NaturalCycles/js-lib by kirillgroshkov 4 years ago
- feat: _stringMapValues, _stringMapEntries Needed due to https://github.com/microsoft/TypeScript/issues/13778 Only affects typings, no runtime effect. — committed to NaturalCycles/js-lib by kirillgroshkov 4 years ago
- Document that index signatures do not include undefined This has been a source of confusion and much debate. I do not propose to settle the debate but surely it is better to document the current behav... — committed to vobarian/TypeScript-Website by vobarian 4 years ago
- FE: Configure typescript to warn on undefined index access https://github.com/microsoft/TypeScript/issues/13778 — committed to YamanQD/Labeeb by HasanMothaffar 2 years ago
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.this is a conscious decision. it would be very annoying for this code to be an error:
and it would be unreasonable to ask every user to use
!
. or to writeFor 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 forArray
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.
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.@mhegazy but for arrays with holes
a[i]
is actually possibly undefined:Output is:
You can try this out in the TypeScript 4.1 beta, coming out soon ™ (or tonight’s nightly if you need it right now! 🙂)
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.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
or
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 likeand TypeScript is complaining at you that
arr[i]
might beundefined
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:Or maybe you write code like this:
and TypeScript says "Hey, that index expression might be
| undefined
, so you dutifully fix it because you’ve seen this error 800 times already:But you didn’t fix the bug. You meant to write
Object.keys(yourObj)
, or maybemyObj[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 hitdel
, 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.copy
lib.d.ts
locally, saylib.strict.d.ts
, change the index signature to[n: number]: T | undefined;
, include the file in your compilation. you should see the intended effect.When we explicitly index into an array/object, e.g. to get the first item of an array (
array[0]
), we want the result to includeundefined
.This is possible by adding
undefined
to the index signature.However, if I understand correctly, the issue with including
undefined
in the index signature is that, when mapping over the array or object, the values will includeundefined
even though they’re known not to beundefined
(otherwise we wouldn’t be mapping over them).Index types/signatures are used for both index lookups (e.g.
array[0]
) and mapping (e.g.for
loops andArray.prototype.map
), but we require different types for each of these cases.This is not an issue with
Map
becauseMap.get
is a function, and therefore its return type can be separate from the inner value type, unlike indexing into an array/object which is not a function, and therefore uses the index signature directly.So, the question becomes: how can we satisfy both cases?
Proposal
A compiler option which treats index lookups (e.g.
array[0]
) similar to howSet.get
andMap.get
are typed, by includingundefined
in the index value type (not the index signature itself). The actual index signature itself would not includeundefined
, so that mapping functions are not effected.Example:
This however won’t solve the case of looping over array/objects using a
for
loop, as that technique uses index lookups.For me and I suspect many others, this is acceptable because
for
loops are not used, instead preferring to use functional style, e.g.Array.prototype.map
. If we did have to use them, we would be happy using the compiler option suggested here along with non-null assertion operators.We could also provide a way to opt-in or opt-out, e.g. with some syntax to decorate the index signature (please forgive the ambiguous syntax I came up with for the example). This syntax would just be a way of signalling which behaviour we want for index lookups.
Opt-out (compiler option enables by default, opt-out where needed):
Opt-in (no compiler option, just opt-in where needed):
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
) returnsT | undefined
whenArray
/Object
index signatures do not.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:Also, this problem is not a problem with the newer, way better
for of
loops and there is even a TSLint ruleprefer-for-of
that tells you to not use old-stylefor
loops anymore.Currently I feel like the type system is inconsistent for the developer.
array.pop()
requires anif
check or a!
assertion, but accessing via[array.length - 1]
does not. ES6map.get()
requires anif
check or a!
assertion, but an object hash does not. @sompylasar’s example is also good.Another example is destructuring:
I would have preferred if the compiler forced me to do this:
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 anif
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.
I ran into an issue with using
| undefined
with map today while usingObject.entries()
.I have an Index type that’s described fairly well by
{[key: string]: string[]}
, with the obvious caveat that not every string key possible is represented in the Index. However, I wrote a bug that TS didn’t catch when trying to consume a value looked up from the Index; didn’t handle the undefined case.So I changed it to
{[key: string]: string[] | undefined}
as recommended, but now this leads to issues with my use ofObject.entries()
. TypeScript now assumes (reasonably, based on the type specification) that the Index may have keys that have a value of undefined specified, and so assumes the result of callingObject.entries()
on it may contain undefined values.However, I know this to be impossible; the only time I should get an
undefined
result is looking up a key that doesn’t exist, and that would not be listed when usingObject.entries()
. So to make TypeScript happy I either have to write code that has no real reason to exist, or override the warnings, which I’d prefer not to do.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 itwould be great if TypeScript could control flow the checks like C# does (to eliminate index range checks to save some processor time), for example:
(to those who uses sparse arrays - kill yourself with hot burning fire)
as for
Object.keys
, it takes a special type sayallkeysof T
to let the control flow analysis do safe narrowingsHow about the next 10 people who want to comment here just test out the draft PR #39560 instead?
Hey, i recognize that syntax; i write loops like this maybe once per year!
I found that in most instances where i actually do index into an array, i actually want to check for
undefined
afterwards.Fears of making this particular case harder to work with seem overblown. If you just want to iterate over the elements you can use
for .. of
. If you need the element index for some reason, useforEach
or iterate overentries
. In general it is extremely rare that you really need an index-based for loop.If there are better reasons of why one would want the status quo, i would like to see them, but regardless: This is an inconsistency in the system and having a flag to fix it would be highly appreciated by many, it seems.
In the interests of keeping this thread readable, I’ve hidden a number of tangential comments that weren’t meaningfully adding to the conversation. In general, you can assume that comments in the following vein have been made before in the preceding 212 comments and don’t need repeating:
There are very real practical and technical challenges that need solving here and it’s disappointing that we can’t do meaningful work in this thread because it’s filled with people dropping in to suggest that anything behind a flag has no real consequences or that any error that can be
ts-ignore
’d is not an upgrade burden.One idea that seems like it could be promising: what if you could use
?:
when defining an index signature to indicate that you expect values to be missing sometimes under normal use? It would act like| undefined
mentioned above, but without the awkward downsides. It would need to disallow explicitundefined
values, which I guess is a difference from usual?:
.It would look like this:
Compared to the previous approach of
| undefined
:There are (at least) two other problems with putting
|undefined
in your object type definitions:Object.values
(or_.values
) will require you to handleundefined
in the resultsI also want to chime in that this causes a lot of false positives in eslint/typescript now:
Eslint (correctly) infers from the type that this is an unnecessary check, because foo can never be null. So now not only do I have to consciously remember to do a null check on things I get back from an array, I also have to add an eslint disable line! And accessing things outside of a for loop like this is virtually the only way we ever do array access, since (like probably most TS devs these days), we use forEach/map/filter/etc functions when looping over arrays.
I really think we should just have a new compiler flag that’s set to true by default if
strict
is true, and if people don’t like it, they can opt out. We’ve added new breaking strict compiler checks in recent versions anyways.This is the source of quite likely the only runtime prod bugs I’ve seen in recent memory that was a failure of the type system.
Actually the goal is:
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.
Considering the ergonomics of the now available optional chaining operator, maybe it’s time to revisit the decision to not make this behaviour available via a flag?
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?
Encountering this was my first big wtf in Typescript. The language otherwise I have found to be wonderfully reliable and this let me down, and it is quite cumbersome to add “as T | undefined” to array accesses. Would love to see one of the proposals implemented.
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 ignoreundefined
s would be a nice way to deal with indexing operations when this option is enabled.Very much in favor of this as an option. It’s a noticeable hole in the type system, particularly when the
strictNullChecks
flag is enabled. Plain objects are used as maps all the time in JS, so TypeScript should support that use case.@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.
Simon, maybe I misread your comment but it sort of sounds like an argument in favor of the original suggestion. The “last mile” missing capability is to treat properties as being
| undefined
sometimes, depending on context, but not in other cases. That’s why I drew an analogy to #2521 upthread.In an ideal scenario, I could declare an array or object such that, given
I can somehow wind up with
arr[n]
types asT | undefined
arr[n] = undefined
is an errorarr
that gives me values typed asT
, notT | undefined
obj[k]
types asV | undefined
obj[k] = undefined
is an errorObject.entries()
for example should give me tuples of[K, V]
and nothing can be undefinedIt’s the asymmetric nature of the problem that defines this issue in my view.
Ran into this with array destructing of a function parameter:
Here I’d expect
a
to be of typestring | undefined
but it’s juststring
, unless I doI don’t think we have a single
for
loop in our code base, so I’d happily just add a flag to my tsconfig’s compiler options (could be namedstrictIndexSignatures
) to toggle this behavior for our project.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 likearray.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 😃
It seems silly that TS is doing its best to prevent access to undefined members, except this. This is by far the most common bug I’ve been fixing in our codebase.
Another use case for this issue—I ran into this problem when using typescript-eslint and enabling
no-unnecessary-condition
. In the past, when accessing an array by index to do some operation on the element at that index, we use optional chaining to ensure that that index is defined (in case the index is out of bounds), as inarray[i]?.doSomething()
. With the problem outlined in this issue, however,no-unnecessary-condition
flags that optional chaining as unnecessary (as the type is not nullable according to typescript) and the autofix removes the optional chaining, leading to runtime errors when the array access at the indexi
actually is undefined.Using
T | undefined
for the type signature is really not useful. We need a way to make it so that the index operator[n]
has aT|undefined
type, but e.g. usingmap
on an array should not give usT|undefined
values, because in that situation we should know they exist.To me, there’s a simple solution. Enable a flag which checks for this. Then, instead of:
You do:
Then, when doing random index accesses, you are required to check if the result is undefined, but not while iterating an enumerated collection.
I still think this warrants having an open issue.
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.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?”If you want to try it out in your repo (took me a bit to figure out):
yarn (add|upgrade) typescript@next
"noUncheckedIndexedAccess": true
Cheers, but I hope it’s clear this still needs to be added at the language level as it’s purely within the realm of typechecking and some projects do not have a linter or the right rules.
Sorry if this was offered before, but could
{[index: string]?: number} // -> number | undefined
be supported? It’s consistent with interface optional properties syntax - required by default, possibly undefined with “?”.Compiler option in 4.1 is cool, but having more granular control would be good too.
While I can follow the contra arguments from @RyanCavanaugh with:
I would drop in some short thoughts on it. At the core, TypeScript aims to make development, well… safer. To remember the first goal of TypeScript:
1. Statically identify constructs that are likely to be errors.
If we look at the compiler, we have already some magic behind the type inference. In the particular case the really excuse is: “we can’t infer the right type on this construct” and it’s totally fine. Over the time we have less and less cases, where compile is not able to infer the types. In other words, for me it’s just matter of time (and spending time on this) to get more sophisticated type inference.
From technical point of view, constructs like:
breaks the first goal of TypeScript. But this doesn’t happens if TypeScript knows the more precise types:
From practical point of view, it’s a trade-off
yet
. This issue and multiple closed issues shows that there is a high demand of the community for this change: ATM 238 upvoters vs. 2 downvoters. Of course it’s pity that above for loop doesn’t inference the “right” type, but well, I’m pretty sure that the most of upvoters can live with!
and the new?
as sign of attention and force it in safe cases. But on the other side get the right types on “dangerous” access.Because I agree that it’s a very sensitive change for big code bases, I vote for a configuration property, like it was proposed here. At least for getting feedback from community. If it’s not possible, then, well, TS 4.0 should get it.
Update from today’s SBS: We yelled at each other for 30 minutes and nothing happened. 🤷♂️
There is no notification from github, but to everyone who would try and play with undefined index from this issue, take a look to the draft PR above my comment.
@RyanCavanaugh thank you very much for giving us an opportunity to play with it. I run it against a pretty small (~105 ts(x) files) code base and played with it a little bit. I didn’t found any important issue. One line which was changed from:
to:
I will try it on a medium project next week.
@lonewarrior556 it’s 60 times more and it’s on the first page if you sort by upvotes 😃
This is very annoying if you want to enable ESLint’s
@typescript-eslint/no-unnecessary-condition
rule because it then complains about all the instances ofIt thinks it is an unnecessary condition (because Typescript says it is!) but it isn’t. I don’t really want to have to add
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
all over my codebase.If fixing this properly is too much for work for lazy Javascript programmers perhaps we could have an alternative array access syntax that would add
undefined
, e.g.(random syntax suggestion; alternatives welcome)
Just had an unexpected undefined error bring down my application for hundreds of users, an error which would have been caught at build time if this type check were in place. TypeScript has been an amazing way for us to deliver more reliable software, but its great utility is being undermined by this single omission.
@martpie
as const
is great if you can use it, that is.There are at least two reasons why i would rather not use
Array<T | undefined>
:forEach
,map
andfilter
, which do not passundefined
as element argument (unless that index is explicitly set that way). Depending on how much you use those functions that could be annoying to deal with.In case it helps anyone, here’s an ESLint plugin that flags unsafe array access and array/object destructuring. It doesn’t flag safe usages like tuples and (non-record) objects.
https://github.com/danielnixon/eslint-plugin-total-functions
It’s useless if we have to keep remembering to use a different, safer pattern anyway. The point is for TypeScript to have our back, whether we’re on a very good day, tired or new to a codebase.
[ moving discussion over here from #38470] Accepting all the arguments for why this wouldn’t be practical for arrays… I think this should definitely be addressed for Tuples:
why not make foo
string | number | undefined
?Giving this to tuples shouldn’t effect the majority of users, and the users that it effect need to have
strictNullChecks
enabled, and declare their arrays as constants… They are obviously looking for stricter type saftey.could also help out with these
Which originally caused my runtime error as a
number
became aNaN
, then returned anundefined
from a tuple… All of which was type safeAccording to Flow’s documentation the behavior with index signatures is the same as currently with TypeScript:
So in this concern both TS and Flow require the user to think of undefined keys instead of the compiler helping them 😦
Would be an advantage of TS if the compiler prevented users from making these kinds of bugs. Reading all the comments in this thread, it seems that the vast majority here would love to have this feature. Is the TS team still 100% against it?
Looking for a solution to this as errors relating to missing indexes have been a frequent source of bugs in my code.
Specifying a type like
{ [index: string]: string | undefined }
is not a solution, as it messes up typing for iterators likeObject.values(x).forEach(...)
which will never includeundefined
values.I’d like to see TypeScript generate errors when I don’t check for undefined after doing
someObject[someKey]
, but not when doingObject.values(someObject).forEach(...)
.As a workaround, I use a small dirty hack. I just add in
package.json
the simple postinstall script:Of course, it won’t work on windows, but that’s better than nothing.
@RyanCavanaugh There’s one very common example that I can show you where user are prone to accessing array incorrectly.
The code above shouldn’t be pass the compiler checking gracefully under
strictNullChecks
, because some uses ofgetBlock
will return undefined, for examplegetBlock('hello')
, in such cases I seriously wants the compiler the raise flag so that I can handle the undefined cases gracefully without exploding my application.And this also applies to a lot of common idioms such as accessing the last element of an array with
arr.slice(-1)[0]
, accessing the first elementarr[0]
etc.Ultimately, I want TypeScript to annoy me for such errors rather than having to deal with exploded applications.
Objects
I always want an index type of
[key: string (or number, symbol)]: V | undefined
, only sometimes I forget about theundefined
case. Whenever a dev has to explicitely tell the compiler “trust me, this is the type of that thing for real” you know it’s unsafe. It makes very little sense to typeMap.get
properly (strictly) but somehow plain objects get a free pass. Still, this is easily fixable in user land, so it’s not too bad. I don’t have a solution, at any rate.Arrays
Perhaps I’m missing something but it seems the argument that “you almost never access an Array in an unsafe way” can go both ways, especially with a new compiler flag.
I tend to think more and more people follow these two best practices:
With this in mind, the only remaining and rare cases where you need low level bracket access logic would really benefit from type safety.
This one seems like a no brainer and I don’t think copy-pasting the entire lib.d.ts locally is an acceptable workaround.
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 ofProxy
, 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.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:
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.
ha-ha. Not that I agree with @RyanCavanaugh 's whole post… but… c’mon… there is what a millions? TS users out there?!? 365 want this feature enough to comment and upvote in this thread…
I have previously suggested an approach that could make everybody happy. Hope it’s okay to repeat it in other words.
Basically the default behavior of the index signature would be changed to
| undefined
when reading from an array or object| undefined
when writing (because we only wantT
values in our object)Definition would be like that:
For people who do not want
undefined
included in the type, they can either disable it using a compiler option or disable it only for some types using the!
operator:To not break existing code, it might also be better to introduce a new compiler option that causes
undefined
to be included (as shown with theMaybeUndefined
type above). If that option is not specified, all index signature types behave as if the!
operator was used within the declaration.Another idea that comes up: One might want to use the same type at some places in the code with the two different behaviors (include
undefined
or not). A new Type mapping could be defined:Would that be a good extension of TypeScript?
I beg to differ.
?.
will work for accessing properties on the indexed value, but the compiler will still say the value is defined regardless of whether it is or isn’t (see #35139).To add to that, if you use eslint and want to use
no-unnecessary-condition
, such accesses will be flagged as unnecessary and removed because the compiler says it is never undefined and that would make?.
unnecessary.@alangpierce
This is an interesting idea that points in the right direction. However, I would invert the logic: The default case should be that index signatures include
| undefined
. If you want to tell the compiler that the undefined case can never occur in your code (so you never access invalid keys), you can add!:
to the signature, like this:and the default case without
!:
would look like we know it, but would be safer:That way you have the safety by default and at the same way you don’t have to explicitly include
undefined
which would allow to writeundefined
by accident.Sorry if that was already suggested, I could not read all the comments in this thread.
In general, I really think that the default behavior that we have now is not what users expect when using TypeScript. It’s better to be safe than sorry, especially when an error can be caught as easily as this one by the compiler.
Since I expect most to use typescript-eslint, is there any rule that could enforce that values have to be checked after indexing before they can be used? This might be helpful before support from TS is implemented.
This is beginning to feel like one of those “Javascript: The Bad Parts” talks:
I feel like, while there may not be movement from package owners, continued feedback from the community is still valuable, because it shows there is still a demand for better tooling around this issue.
I do agree with the delete file dialog analogy, however I think that this analogy can also be extended to forcing user to check something that is possibly undefined or null, so this explanation don’t really make sense, because if this explanation is true, the
strictNullChecks
option is going to induce the same behaviour, for example getting some element from the DOM usingdocument.getElementById
.But that is not the case, a lot of TypeScript user wants the compiler to raise flag about such code so that those edge cases can be handled appropriately instead of throwing the
Cannot access property X of undefined
error which is very very hard to trace.In the end, I hope these kind of features can be implemented as extra TypeScript compiler options, because that’s the reason users want to use TypeScript, they want to be warned about dangerous code.
Talking about accessing array or objects wrongly is unlikely to happen, do you have any data to backup this claim? Or is it just based on arbitrary gut feeling?
I haven’t read up on this issue or the pros and cons, various proposals, etc. (just found it in Google after being repeatedly surprised that array/object access isn’t handled consistently with strict null checks), but I have a related suggestion: an option to make array type inference as strict as possible unless specifically overridden.
For example:
By default,
balls
would be treated as[number, number, number]
. This could be overridden by writing:Further, tuple element access would be handled consistently with strict null checks. It’s surprising to me that in the following example
n
is currently inferred asnumber
even with strict null checks enabled.I would also expect array mutation methods such as
.push
to not exist in the tuple type definition, since such methods change the run-time type to be inconsistent with the compile-time type.It’s clear that there is a demand for this feature. I really hope TS team will find some solution. Not by just adding
| undefined
to indexer because it has it’s own issues (mentioned already) but by more “clever” way (reading returnsT|undefined
, writing requiresT
, good compiler checking forfor
loop, etc. good proposition were also mentioned already.)We are fine with runtime error when we mutate and work with arrays in non trivial, difficult to verify by compiler way. We just want error checking for most cases and are fine with using
!
sometimes.Having said that, if this stricter handling of arrays would be implemented, now with #24897 it would be possible to implement nice type narrowing when checking array length with constant. We could just narrow array to tuple with rest element.
It would be useful when you index by constant or destructure an array.
Other question is what would we do with big numbers like:
We can’t show the type of this huge tuple in IDE or compiler messages.
I guess we could have some syntax to represent “tuple of certain length with same type for all items”. So for example the tuple of 1000 strings is
string[10000]
and the type of narrowed array from above example could be[...string[99999], ...string[]]
.The other concern is if compiler infrastructure can support such big tuples now, and if not how hard would it be to do.
Why can’t we just keep it simple and not add any magic behaviour around old-style for loops at all. You can always use
!
to make things work. If your codebase is full of old-stylefor
loops, don’t use the flag. All modern codebases I’ve worked with use eitherforEach
orfor of
to iterate arrays and those codebases would benefit from the additional type safety.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.
In the process of enabling this rule on my project, I came across this interesting error:
In this case I did not expect
myRecord[key]
to return typeMyRecord[Key] | undefined
, because thekey
is constrained tokeyof MyRecord
.Update: filed issue https://github.com/microsoft/TypeScript/issues/40666
@the0rted I did read that comment. It does not implement a version of my suggestion. Maybe you should just read it again.
There is no point in discussing the idea of making
| undefined
return the default behavior – they will never release a version of Typescript that just explodes millions of lines of legacy code due to a compiler update. A change like that would be the single most breaking change in any project I’ve ever followed.I do agree with the rest of the post though.
I really feel like I just have to pop in every couple pages of comments and drop a link to #2521 again.
Nope! And at least (clicks link…) 116 others here agree with me. I would be so happy to at least have the option of typing reads and writes differently. This is just yet another great use case for that feature.
Yeah and it would be nice if the user of TypeScript could choose which downside they prefer 😉
Without this feature my application became very buggy as I’m dealing with multi-dimensional array, I have to always remind myself to access elements using
xs[i]?.[j]
instead ofxs[i][j]
, also I have to explicitly cast the accessed element likeconst element = xs[i]?.[j] as Element | undefined
to ensure type safety.I also hit an issue with this today. We’re accessing from an array via a computed index. That index could be out of range. So we have something like this in our code:
This results in
noNext
being defined asfalse
. Which is wrong. It can be true. I also don’t want to defineitems
asArray<Item | undefined>
because that gives a wrong expectation. If an index is there it should never beundefined
. But if you’re using the wrong index, it isundefined
. Sure, the above could probably be solved by using a.length
check instead or definingnoNext
explicitly asboolean
. But in the end this is something that bothers me since I started using TypeScript and I never understood why| undefined
is not included by default.Hey everyone, I feel like much of the discussion to be had here has been had. The package owners have been quite clear about their reasoning and have considered most of these arguments already. If they plan to address this, I’m sure they will make it known. Other than that, I don’t think this thread is really productive.
On Fri, Oct 25, 2019 at 11:59 AM brunnerh notifications@github.com wrote:
As a programmer who is new to TypeScript, I found the situation around indexing objects in strict mode to be unintuitive. I would have expected the result of a lookup to be
T | undefined
, similarly toMap.get
. Anyway, I just ran into this recently, and opened an issue in a library:screepers/typed-screeps#107
I’m probably going to close it now, because it seems there’s no good solution. I think I’m going to try “opting-in” to
T | undefined
by using a little utility function:I ported an app from Elm to Typescript recently and indexing operations being incorrectly typed is probably one of the biggest sources of bugs I’ve run into, with all the strictest settings of TS enabled (also stuff like
this
being unbound).@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.
That suggestion mischaracterizes the problem. The issue isn’t with TypeScript’s understanding of the type, itself. The issue comes from accessing the data, which is where new syntax has already been proposed earlier in this thread.
Sorry, when I said “this” suggestion I meant the capability in the OP, i.e. treating
array_of_T[i]
asT | undefined
. I do see that you’re asking about a syntax to mark a specific index operator as “maybe undefined”, but if you use the implementation in Ryan’s PR, you wouldn’t need that, because all index operators would be “maybe undefined”. Wouldn’t that meet your need?@MatthiasKunnen If this were some small edge case feature then I’d agree that that’s a viable reason not to add a flag for this. But direct array access shows up in code quite frequently, and is both a glaring hole in the type system and also a breaking change to fix, so I feel a flag would be warranted.
@RyanCavanaugh
thank you for your summary.
I’ve deleted my comment and if you wish, I can delete this comment too. But how to understand the first goal of the project:
1. Statically identify constructs that are likely to be errors.
(Source: https://github.com/Microsoft/TypeScript/wiki/TypeScript-Design-Goals )
I don’t understand your quoted comment, because unchecked index access can leads likely to a runtime error. If in JavaScript you expect things like that, in TypeScript you get dangerous and wrong confidence. Wrong types are worse than
any
.Of course, you are one of the core maintainers and thank you and the team and the contributors for ts. But it’s not just about you anymore. Compare upvotes: 365 with downvotes: 6! Only 6! It shows a huge demand for typesafety.
But let’s speak about solutions. Is there any solution the team would make happens or can think of?
Can you please elaborate a little bit more on whats wrong with
ts-ignore
in this case? I mean it can be automated with tools like codemod and doesn’t change the behavior of current code. Of course it’s not beautiful workaround, but well, it’s one possible way to introduce this change without flags.What do you think about automatic publishing of a patched version of ts, for example
4.0.0-breaking
? It introduce a some (much?) work on conflicts, but it allows to everyone to test changes and prepare the code base (not only for this feature request). This could be done for a some limited time period, like 3-6 months. We would be the first, who will use this version.I don’t feel particularly strongly about the need for this on arrays, since many other languages (including “safe” ones like Rust) leave the onus for bounds checks to the user, and so I and many developers are already accustomed to doing it. Also, the syntax tends to make it quite obvious in these cases, because they always use bracket notation like
foo[i]
.That said, I do feel strongly about adding this to string-indexed objects, because it’s very difficult to tell whether the signature of
foo.bar
is correct (due to the fieldbar
being explicitly defined), or possiblyundefined
(because it is part of an index signature). If this case (which I think is pretty high-value) gets solved, then the array case likely becomes trivial and probably worth doing too.The getter and setter syntax from javascript could be extended to type definitions. For example, the following syntax is fully understood by TypeScript:
However, TS currently “merges” these into a single type, since it doesn’t track “get” and “set” types of a field separately. This approach works for most common cases, but has its own shortcomings, such as requiring getters and setters to share the same type, and incorrectly assigning a getter type to a field that only defines a setter.
If TS were to track “get” and “set” separately for all fields (which likely has some real performance costs) it would solve these quirks, and also provide a mechanism for describing the feature described in this issue:
would essentially become shorthand for:
If generated declaration files always used the full getter+setter syntax, this would only be a problem for hand-written ones. Applying the current rules to remaining shorthand syntax in definition files only might be a solution here. It would certainly solve the compatibility issues, but would also add to the mental cost of reading
.ts
files vs.d.ts
files.Aside:
Now of course, this doesn’t address all the subtle caveats with getters/setters:
This is a further edge case, and it’s probably worth considering if it’s within the scope of TS goals at all… but either way it could probably be solved separately, since my proposal here is consistent with TypeScript’s current behavior.
In my opinion,
Array<string>[number]
should always bestring|undefined
. It is the reality, if I index into an array with any number, I will get either the array’s item type or undefined. You can’t really be any more specific there unless you are encoding array length as you could with tuples. Your example for writing would not type check, because you cannot assignstring|undefined
to an array index.This seems to be exactly what should be as they are two different things. No array will have ever index defined, so you’d always have the potential to get undefined. The type of
Array<string>[number]
would bestring|undefined
. In order to specify what you want theT
in anArray<T>
, a utility type could be used (naming not great):ArrayItemType<Array<string>> = string
. This doesn’t help withRecord
types, which may need something likeRecordValue<Record<string, number>, string> = string
.I agree that there’s no great solutions here, but I’m pretty sure I’d prefer the the soundness on the index reads.
I mean sure I could just cast it but that’s not really a solution. Similarly, anyone here could simply cast the for loop array indexing as well making this entire issue mute.
The problem is that Typescript doesn’t warn for this behavior. As the developer, we have to remember to manually cast these as undefined which is bad behavior for a tool to cause more work for devs. Also to be fair, the cast would actually have to look something like this:
That’s not nice at all. But like I said above the main issue isn’t even needing to type it like this, its that TS doesn’t warn for this at all. That leads to devs who forget this to push code, cool CI found no issues on tsc merge and deploy. TS gives a sense of confidence in the code and having inaccurate typings being provided gives a false sense of confidence. => Errors on runtime!
I like the idea but I suspect we’d have to get https://github.com/microsoft/TypeScript/issues/2521 first – which, don’t get me wrong, I still believe we absolutely should, but I’m not holding my breath.
All the other issues that run adjacent to this one seem to get redirected here, so I’ll assume this is the best place to ask: is a shorthand syntax for optional index signatures off the table? As @martpie points out, you have to write
interface Foo { [k: string]: Bar | undefined; }
which is less ergonomic thaninterface Foo { [k: string]?: Bar; }
.Can we get
?:
operator support? If not, why not? That is, the ergonomic benefit was sufficient to add the feature for single property definition – is it not “helpful enough” for index signatures?yes, everyone pretty much agrees it’s a very bad design decision, one contemporary to a remote time when TS was completely unsafe.
@yawaramin If you’re using sparse arrays, Typescript already doesn’t do the right thing. A
--strict-index
flag would fix that. This compiles:@RyanCavanaugh since you’re here, how about inferring
item :: T
forarr :: T[]
infor (const item of arr) ...
, and otherwise inferringarr[i] :: T | undefined
when using some--strict-index
? How about the case I care about,obj[key] :: V | undefined
butObject.values(obj) :: V[]
forobj :: { [key: string]: V }
?TypeScript‘s Control Flow Based Type Analysis could be improved to recognize this case to be safe and not require the
!
. If a human can deduce it is safe, a compiler can too. I‘m aware that this might be non-trivial to implement in the compiler though.This is how I’ve been working around this issue: https://github.com/danielnixon/total-functions/
@yawaramin Make sure you have
strictNullChecks
enabled in yourtsconfig.json
(which is also enabled by thestrict
flag)Most of this discussion has been focused on arrays, but, in my experience, object access is where this feature would be most helpful. For example, this (simplified) example from my codebase looks reasonable and compiles fine, but is definitely a bug:
not to imply that Aleksey would ever mutate an array
If a flag like
--strictArrayIndex
is not an option because flags are not designed to change thelib.d.ts
files. Maybe you guys can release strict versions oflib.d.ts
files like "lib": ['strict-es6']
?It could contain multiple improvements, not just strict array index. For example,
Object.keys
:Could be:
Thanks for your incredibly constructive input 👍
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
. ES6Map
is already defined with get asget(key: K): V | undefined;
.There’s always a way to invalidate the previous assumption, and that’s the problem. Analyzing every possible execution path would be way too costly.
This shouldn’t be an option, it should be the default.
That is, if TypeScript’s type system is to describe at compile-time the types that JavaScript values can take on at run-time:
Array
can of course returnundefined
in JavaScript (index out of bounds or sparse array), so the correct read signature would beT | undefined
.Array
to beundefined
is also possible via thedelete
keyword.As TypeScript 4 prevents
there is a good case for also preventing
a[1] = undefined
. This suggest thatArray<T>[number]
should indeed be different for reads and writes.It might be “complex” but it would allow to model run-time possibilities in JavaScript more accurately; Which is what TypeScript was made for, right?
I would say that the statement that it’s a problem for devs who use for loops is failing to take into account devs who use Array Destructuring. It is broken in Typescript and won’t be fixed either because of this issue. It provides inaccurate typings. IMO Array Destructuring being effected is a lot bigger problem than those still using for loops.
I’m not sure there’s that much
for(let i..)
iterating over tuples… If the tuple has a single type then The user would prob use an Array,In my experience tuples are great for mixed types and if that’s the general case then you are type checking anyway
Also assuming this proposed change happens what is stopping people from doing this?
you could use
keyof typeof aa
instead of0 | 1
when they fix this issueSure there wouldn’t be any type safety when doing
i
arithmetic, but it really allows people to choose this for type safety over the default “Not strictly type safe but convenient” arrayIn those cases where you do need to, you can use
Array#entries
:I personally don’t use
Array#forEach
all that much, and I never useArray#map
for iteration (I use it all the time for mapping though). I prefer to keep my code flat, and I appreciate the ability to break out of a for…of loop.Yeah, I also ended up using this approach when I was trying my experiment. For example, code that was previously written as:
had to be changed to something like:
Probably true. It might actually be easier to write a codemod for automatically re-writing code that depends on assertions about
.length
to use some other approach, than to make TypeScript smart enough to infer more about the types based on assertions about the.length
😛This is a good point. While including
undefined
in index types is a huge breaking change, going in the other direction is not a breaking change. Code that is able to handle gettingT | undefined
should also be able to handleT
without any changes. This means, for example, that libraries could be updated to handle theT | undefined
case if there was a compiler flag and still be used by projects that don’t have that compiler flag enabled.This is not a proposed solution, but just an experiment: I tried modifying
lib.es5.d.ts
inside mynode_modules
on an existing project that uses TypeScript just to see what it would be like if we did have a compiler option for this. I modifiedArray
andReadonlyArray
:Because this is a very large project, this caused several hundred type errors. I went through just a few of them to get an idea of what kind of issues I’d run into and how hard they would be to work around if there was a compiler option for this. Here’s a few of the issues I ran into:
This caused type errors not only in our codebase, but in one of our dependencies: io-ts. Since io-ts allows you to create types that you use in your own code, I don’t think it’s possible to only apply this option to your own codebase and not also apply it to io-ts’s types. This means that io-ts and probably some other libraries would still have to be updated to work with this option even if it were only introduced as a compiler option. At first I thought making this a compiler option would make this less controversial, but if people that do choose to use the option start complaining to a bunch of different library authors about incompatibilities, it may be even more controversial than it simply being a TS 4.0 breaking change.
Sometimes I had to add some extra type guards to eliminate the possibility of undefined. This is not necessarily a bad thing, and is kind of the whole point of this proposal, but I’m just mentioning it for completeness.
I had a type error inside a
for (let i = 0; i < array.length; i++)
loop that was looping over an arbitraryArray<T>
. I couldn’t just add a type guard to check forundefined
, becauseT
itself might includeundefined
. The only solutions I could think of were to A) use a type assertion or@ts-ignore
to silence the type error or B) use a for-of loop instead. Personally I don’t find this to be too bad (there are probably few cases where using an for-of loop to iterate over an array is not better anyway), but it may be controversial.There are many cases where the existing code was already making some assertion about the value of
.length
and then accessing an element in the array. These now caused type errors despite the.length
check, so I had to either change the code to not depend on a.length
check or just add a redundant!== undefined
check. It would be really nice if TypeScript could somehow allow using a.length
check to avoid the need for the!== undefined
check. I assume actually implementing that would not be trivial, though.Some code was using
A[number]
to get the type of the elements of a generic array type. However, this now returnedT | undefined
instead of justT
, causing type errors elsewhere. I made a helper to work around this:but even with this helper to ease the transition, this is still a large breaking change. Perhaps TypeScript could have some kind of special case for
A[number]
to avoid this issue, but it would be nice to avoid weird special cases like that if possible.I only went through a small handful of the hundreds of type errors, so this is probably a very incomplete list.
For anyone else interested in fixing this issue, it might be useful to try doing the same thing with some existing projects and share what other issues you run into. Hopefully the specific examples can provide some guidance to anyone who actually tries to implement this.
@danielnixon That’s not a solution, that’s a workaround. There’s nothing stopping you or another developer from mistakenly (if you can even call it that) using the language’s built-in tools for the same objectives. I mean, I use fp-ts for this stuff, but this issue is still valid and needs fixing.
@RyanCavanaugh Here are some of the cases from my code base where some runtime crashes are slumbering. Note that I already had cases where I had crashes in production because of this and needed to release a hotfix.
In this case it would be annoying as
ArticleIDs extends articleNames[]
includesundefined
in the resulting values, while it should just allow completely defined subsets. Easily fixable by usingReadonlyArray<articleNames>
instead ofarticleNames[]
.All in all I’d really like to have this extra type safety for preventing possible runtime crashes.
@buu700 Welcome to TypeScript 3.0: https://blogs.msdn.microsoft.com/typescript/2018/07/30/announcing-typescript-3-0/#richer-tuple-types
I think this is a very important point.
At the moment, I am experimenting with the following approach:
Define
const safelyAccessProperty = <T, K extends keyof T>(object: T, key: K): T[K] | undefined => object[key];
Then access properties like
safelyAccessProperty(myObject, myKey)
, instead ofmyObject[myKey]
.This seems like a really weak argument against this. Those cases are already broken - using them as an excuse not to fix other cases makes no sense. Nobody here is asking for either of those cases to be handled correctly nor for it to be 100% correct. We just want accurate types in a really common case. There are many cases in TypeScript that aren’t handled perfectly; if you’re doing something weird, types might be wrong. In this case we’re not doing anything weird and the types are wrong.
I’m curious to see a code example of where this is “adding ceremony to correct code”. As I understand it, a basic for loop over an array is easy to detect/narrow. What’s the real world code that isn’t a simple for loop where this becomes inconvenient which is not potentially a bug? (either now, or could be from a trivial change in the future). I’m not saying there isn’t any; I just can’t picture it and haven’t seen any examples (I’ve seen plenty of examples using for loops, but unless you’re saying these can’t be narrowed they seem irrelevant).
There have been examples of both working code that fails to compile because of this and code that throws at runtime because the types mislead the developer. Saying there is zero value in supporting this is nonsense.
thats intriguing, because the solution is obvious: both
T
andT | undefined
are wrong, the only right way is to make the index variable aware of the capacity of its container, either by picking from a set or by enclosing it in a known numeric rangeT | undefined
is correct for the general case, for reasons I just gave. Gonna ignore your nonsensical ramblings about dependent types, have a nice day.All well intentioned input is appreciated, don’t take it the wrong way @Timmmm. I think the downvotes only convey that this is already well-trodden ground in this thread at this point.
For me, adding a flag that improves type safety across the board is much lower cost than introducing new opt-in syntax.
Every flag you add is another configuration matrix that needs to be tested and supported. For that reason TypeScript wants to keep flags to a minimum.
That’s fair, but at the very least I would expect a long “lead in” period where it’s off by default, giving people a lot of time (6 months? a year? more?) to convert legacy code before it becomes on-by-default. Like, add the flag in v4.0, set it to on-by-default in v5.0, that sort of thing.
But that’s far future, at this point, because we don’t even have buy-in that the prerequisite feature (different types for setter/getter) is worth doing. So I stand by the statement that there’s no point discussing default behavior for a long, long time.
@RyanCavanaugh
Just curious, can you tell us what is the not pretty part about? Perhaps we can sort it out if you can share more about this.
Hmm what about object indexes? I never have run into the need for arrays but fairly certain it makes you check when using objects.
@bradennapier
Flow
doesn’t useT | undefined
either in the index signature forArray
s (so same as TypeScript, unfortunately.)I mean… seems like a good idea to have a
--strictIndexChecks
or similar that allows people to opt into this type of behavior. It then wont be a breaking change for everyone and it gives a more strict type environment.I do believe
Flow
works this way by default for indexes and doesn’t have the aforementioned issues but it has been awhile since I have used it so I could be mistaken.I think having the
??
and?.
operators means it is no longer overly cumbersome to do random access toArray
s. But also, it is probably far more common to.map
or.forEach
, where there’s no need for “T | undefined
”, the callback is never run at out-of-bounds indices) orArray
in loops, some of which could be statically determined to be safe (for...of
… almost, except for sparse arrays,for...in
I believe is safe).And also, a variable could be considered a “safe index” if it’s checked with
<index> in <array>
first.Clarification: I am talking about the original reason for
Array<T>
not having the index type[number]: T | undefined
(but instead just[number]: T
), which was that it would be too cumbersome to use. I mean that it is no longer the case, soundefined
could be added.Yep, same here. It forced us to rollout our own types that have (
| undefined
) to have type safety. Mostly for object access (probably it’s a separate opened issue), but same logic applies to accessing array indexes.This doesn’t need fixing in array.prototype functions. This is a massive issue for array destructuring!
It is more ugly, but it forces the developer to check whether the element is defined or not. I’ve tried to come up with more elegant solutions, but it seems that this is the one that has the least downsides. At least for our usecase.
Given that functions like
forEach
,map
,filter
, etc exist (and are IMHO much more preferable), I would not expect the TS team to greatly complicate their inference engine to support looping over an array with a regular for loop. I have a feeling that there is just too much unexpected complexity from trying to accomplish that. For instance, sincei
is not a constant, what happens if someone changes the value ofi
inside the loop? Sure, it’s an edge case, but it’s something they need to handle in a (hopefully) intuitive way.Fixing index signatures, however, should be relatively simple(ish), as the above comments have shown.
@nicu-chiciuc https://www.npmjs.com/package/patch-package Totally fits into a heretic’s toolbox alongside https://www.npmjs.com/package/yalc ✌️
I am suprised no one talked about
as const
yet.More generally, and not exactly related to the initial message of this issue I’ve been using the following defensive programming patterns for the past months, and it worked well (for me)
Even though there is not short notation for it (like
?
), I feel it’s fine. Telling the compiler yourself that you may not be sure the value is defined is imho fine.It really can’t.
Does this function pass an
undefined
tosomeFunc
on the second pass through the loop, or doesn’t it? There are a lot of things I could write insomeFunc
that would result in anundefined
showing up later.What about this?
Good workaround, I really hope this gets sherlocked by the TypeScript team.
When a programmer makes assumptions in written code, and the compiler cannot deduce that it is save, this should result in a compiler error unless silenced IMHO.
This behavior would also be very helpful in combination with the new optional chaining operator.
I came here from rejecting adding
| undefined
in the DT PR above, as it would break all the existing users of that API - could this be better looked at as allowing the user to pick how fussy they want to be, rather than the library?I’ll note optional properties add the
| undefined
as well, and that has bitten me a few times - essentially TS doesn’t distinguish between a missing property and a property set to undefined. I would just like{ foo?: T, bar?: T }
to be treated the same as{ [name: 'foo' | 'bar']: T }
, whichever way that goes (see also theprocess.env
comments above)Is TS against breaking the symmetry here on number and string indexers?
foo[bar] && foo[bar].baz()
is a very common JS pattern, it feels clumsy when it’s not supported by TS (in the sense of reminding you that you need to if you don’t add| undefined
, and warning when it’s obviously not required if you do).Regarding mutating arrays during iteration breaking the guard expression guarantee, that’s possible with the other guards, too:
but I guess that’s a lot less likely in real code that old loops mutating the iteratee.
I’m trying to collect my thoughts on this issue.
Objects are a mixed bag in JavaScript. They can be used for two purposes:
For objects used as records, index signatures do not need to be used. Instead, known keys are defined on the interface type. Valid key lookups return the corresponding type, invalid key lookups will fallback to the index signature. If there is no index signature defined (which there shouldn’t be for objects used as records), and
noImplicitAny
is enabled, this will error as desired.For objects used as dictionaries (aka maps) and arrays, index signatures are used, and we can choose to include
| undefined
in the value type. For example,{ [key: index]: string | undefined }
. All keys lookups are valid (because keys are not known at compile time), and all keys return the same type (in this example,T | undefined
).Seeing as index signatures should only be used for the dictionary objects pattern and arrays, it is desired that TypeScript should enforce
| undefined
in the index signature value type: if the keys are not known, and key lookups possibly returnundefined
.There are good examples of bugs that may appear invisible without this, such as
Array.prototype.find
returningundefined
, or key lookups such asarray[0]
returningundefined
. (Destructuring is just sugar syntax for key lookups.) It is possible to write functions likegetKey
to correct the return type, but we have to rely on discipline to enforce use of these functions across a codebase.If I understand correctly, the issue then becomes about reflection over dictionary objects and arrays, such that when mapping over the keys of a dictionary object or array, key lookups are known to be valid i.e. will not return
undefined
. In this case, it would be undesirable for value types to includeundefined
. It may be possible to use control flow analysis to fix this.Is this the outstanding issue, or do I misunderstand?
Which use cases and problems haven’t I mentioned?
and you will in N years, maybe, for now you can suffer or man up
Yes, I have to edit others’ typings all the time and I’d like to do it less.
And now we’re back to
Time is a flat circle.
Interesting, so I tried this:
Any ideas?
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.
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.
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 andforEach
properly.cross-linking to #13195, which also looks at differences and similarities between “there’s no property here” and “there is a property here but it’s
undefined
”I’d say that’s a bug. Basically if
keyof Type
only includes string/number literal types (as opposed to actually includingstring
ornumber
), thenType[Key]
whereKey extends keyof Type
should not includeundefined
, I’d think._Originally posted by @osyrisrblx in https://github.com/microsoft/TypeScript/issues/40435#issuecomment-690017567_
I didn’t mention it above, but a syntax for one-off overrides already exists:
if ((some_array[i] as MyType | undefined) === undefined)
. It’s not as terse as a new shorthand but hopefully it’s not a construct you’d have to use very often.I don’t want to get too far off from the original point of the issue, but you should really declare that example method using a tuple type for the function argument. If the function was declared as
([userId, duration, ...reason]: [string, number, ...string[]]) => {}
then you wouldn’t have to worry about this in the first place.@RDGthree not to be white knighting for Microsoft of all things, but you apparently missed this bit from mhegazy in the first reply:
and a bit later on RyanCavanaugh has:
So basically - they are perfectly aware of the fact that a flag is being asked for, and so far they have not been convinced it’s worth the work for them for the rather dubious benefit that’s been suggested people would get from this. Honestly, I’m not surprised, since everyone keeps coming in here with pretty much exactly the same suggestions and examples that were already addressed. Ideally, you should just use
for of
or methods for arrays, andMap
instead of objects (or less effectively, 'T | undefined` object values).I’m here because I maintain a popular DT package and people keep asking for
|undefined
to be added to things like http header maps, which is totally reasonable, except for the bit where it would break pretty much every existing usage, and the fact that it makes other safe usages likeObject.entries()
much worse to use. Other than complaining about how your opinion is better than that of Typescript’s creators, what actually is your contribution here?No, as typescript will (incorrectly, in my opinion) not treat it as optional (see #35139).
@caseyhoward As noted previously in this issue, this causes unwanted behavior with the various Array.prototype functions:
Ignoring arrays, and focusing on
Record<string, T>
for a moment, my personal wishlist is that writing only allowsT
but reading may beT|undefined
.The only way for the above to be ergonomic would be some kind of dependent typing, and dependent type guards. Since we don’t have those, adding this behavior would cause all kinds of problems.
Going back to arrays, the problem is that the index signature for arrays does not have the same intent as
Record<number, T>
.So, you would require entirely different dependent type guards like,
So, the index signature for arrays isn’t really
Record<number, T>
. It’s more likeRecord<(int & (0 <= i < this["length"]), T>
(a range and number-backed integer type)So, while the original post just talks about arrays, the title seems to suggest index signatures of “just”
string
or “just”number
. And they’re two completely different discussions.TL;DR Original post and title talk about different things, implementing either looks heavily dependent (ha) on dependent typing.
@riggs I think you’re talking about making
interface Array<T> { }
have[index: number]: T | undefined
, but @radix is likely talking about what seems to be the current recommendation, which is to useArray<T | undefined>
in your own code.The latter is bad for several reasons, not least of which is that you don’t control other packages’ types, but the former also has some issues, namely that you can assign
undefined
to the array, and that it givesundefined
even in known to be safe cases. 🤷♂️@radix For the actual functions on Array, I don’t think this will be a problem because they all have their own type definitions that output the right type: e.g.
map
: https://github.com/microsoft/TypeScript/blob/master/lib/lib.es5.d.ts#L1331The only common code usage that the
| undefined
construct poses an actual regression of experience is infor ... of
loops. Unfortunately (from the perspective of this issue), those are rather common constructs.@yawaramin For sparse arrays, the element type probably needs to include
undefined
unless TypeScript can deduce that it‘s used like a tuple. In the code that @danielnixon linked to (https://github.com/microsoft/TypeScript/issues/13778#issuecomment-536248028), tuples are also treated special and do not includeundefined
in the returned element type since the compiler guarantees that only set indices are accessed.@plul Good catch. The discussion is currently focused on read operations, but the indexer type definition is in fact two-fold, and adding
| undefined
would allow writingundefined
values.The
safelyAccessProperty
function you are experimenting with (mentioned above asgetKey
by @OliverJAsh) requires discipline and/or a linter rule to forbid indexing operations on all arrays and objects.This can be made scalable if the function is provided on all array and object instances (every type that provides indexer operations), like in C++
std::vector
has.at()
which throws an exception in runtime for OOB access, and an unchecked[]
operator which in best case crashes with SEGFAULT on OOB access, in worst case corrupts memory.I think the OOB access problem is not solvable in TypeScript/JavaScript at the type definition level alone, and requires language support to restrict potentially dangerous indexer operations if this strictness feature is enabled.
The two-fold nature of the indexer could be modeled as a property with
get
andset
operations as functions, but that would be a breaking change for all existing indexer type definitions.@radix “Suggestion Backlog Slog”
How about a direct key lookup? E.g.
Is there any way to enforce
T | undefined
instead ofT
here? Currently I use these helpers in my codebase everywhere, as type safe alternatives to index lookups on arrays and objects:@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.
the point is that neither
T | undefined
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?
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 yearsOne of the regularly skipped errors with the absence of
| undefined
in the array indexing is this pattern when used in place offind
: