You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Suggestion: option to include undefined in index signatures #13778
With strictNullChecks enabled, TypeScript does not include undefined in index signatures (e.g. on an object or array). This is a well known caveat and discussed in several issues, namely #9235, #13161, #12287, and #7140 (comment).
Example:
constxs: 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:
constxs: 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.
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.
Reacted by Marin Marinov, Jacob, Agustín De Luca and Lasath Fernando
Reacted by Danny Tuppeny, Jan Melcher, Anton Medvedev, Summer Knight, Robbie Speed, Abraham White, Tom Medema, Anand Nimkar, Babak B., Daniel Bankhead and 100 more
{// https://github.com/Microsoft/TypeScript/blob/1f92bacdc81e7ae6706ad8776121e1db986a8b27/lib/lib.d.ts#L1300declare global {interfaceArray<T>{[n: number]: T|undefined;}}constxs=[1,2,3]constx=xs[100]x// still number :-(}
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.
Reacted by SlurpTheo, Paul Koerbitz, Markus Johnsson, Hermann Rolfes and Prabhakar Poudel
Reacted by Danny Tuppeny, Zheeeng, Anton Medvedev, Frank Li, Alexandre Galays, ulrichb, Junyoung/"Clare" Jang, Jens Duttke, Kevin Montag, Robby Emmert and 49 more
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.
Reacted by Ciaran McGhie, Paul Sonnentag, Robert Penner, Kirill Agalakov, Gábor Balogh, WJH, Shota Senga, Ian Edington, Francesc Rosas, Łukasz Jagodziński and 20 more
this is a conscious decision. it would be very annoying for this code to be an error:
vara=[];for(vari=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
vara=[];for(vari=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.
Reacted by olegdunkan, Thomas Mur, Josh Ghoulberg 👻, Khải, Denis Zhbankov, Kyle Bebak, Eric Ferreira, Brian Kim, Dave Hulme, richardspence and 6 more
Reacted by Arnau Orriols, Piotr Witek, Robbie Speed, Jianrong Yu, Jeremy Banks, tom, Chad Nickell, Kevin Montag, Michał Miszczyszyn, hktonylee and 74 more
Reacted by Misha Kaletsky, Christopher Armstrong, Tom Crockett, Danny Tuppeny, Ethan Resnick, Anton Medvedev, S. B. Tam, Chad Nickell, Kevin Montag, Robby Emmert and 23 more
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
Reacted by Greg Zapp, shlomiborovitz, SlurpTheo, Tom Crockett, Danny Tuppeny, Bradley Ayers, Arnau Orriols, Ingvar Stepanyan, Gezim Hoxha, Piotr Witek and 103 more
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.
Reacted by Thomas Mur, Gustav Bylund, Summer Knight, George MacKerron, Kyle Bebak, Ivan Kashtanov, Joe Percy, John Leidegren, Wolfgang Hobmaier, Lasath Fernando and 1 more
Reacted by WJH, Pablo Blanco Celdrán, Timur Seitosmanov, Ian Edington, emilio-bitwala, Daniel Corner, Dapper Mink, Mikhail Zhuravlev, Laurin, Łukasz Jagodziński and 39 more
Reacted by SlurpTheo, Eugene Lubarsky, Gustav Bylund, Abraham White, Tadhg McDonald-Jensen, Brendan Grady, Matt Mazzola, Ian Edington, Steven, Joe Percy and 1 more
Reacted by Daniel Nixon, Dapper Mink, Kirill Groshkov, Eric Ferreira, Tauyekel Kunzhol, Simon Hessner, Roger Graham, Carlo Moretti, Tom Bonnike, chpio and 1 more
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.
Reacted by SlurpTheo, James Bromwell, Michał Adamczyk, Gezim Hoxha, Robert K. Bell, Kyle Corbelli, defenestrant, Stefano J. Attardi, Augusto Moura, Anders Lassen and 144 more
Reacted by Anton Ivanov, Mikolaj Torz, Grzegorz Szeliga, Alexey Lyakhov, scratchyone, Radovan Bednár, dominictobias, Tyler Laprade, CFA and Maksym Kobieliev
Reacted by Anton Ivanov, Mikolaj Torz, Grzegorz Szeliga, Alexey Lyakhov, Christopher Lord and dominictobias
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:
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.
Reacted by Thomas Mur, Memeplexx, Alec Larson, S. B. Tam, Robby Emmert, Dmitrijs Balcers, Kirill Agalakov, Pablo Blanco Celdrán, Mikhail Zhuravlev, Laurin and 11 more
In the process of enabling this rule on my project, I came across this interesting error:
typeMyRecord={a: number;b: string};declareconstmyRecord: MyRecord;declareconstkey: 'a'|'b';constvalue=myRecord[key];// string | number ✅// ❌ Unexpected error// Type 'MyRecord[Key] | undefined' is not assignable to type 'MyRecord[Key]'constfn=<KeyextendskeyofMyRecord>(key: Key): MyRecord[Key]=>myRecord[key];
In this case I did not expect myRecord[key] to return type MyRecord[Key] | undefined, because the key is constrained to keyof MyRecord.
I'd say that's a bug. Basically if keyof Type only includes string/number literal types (as opposed to actually including string or number), then Type[Key] where Key extends keyof Type should not include undefined, I'd think.
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"
FYI: I caught two bugs in my codebase thanks to this flag, so big thank you!
It's a bit annoying that checking arr.length > 0 is not sufficient to guard arr[0], but that's a minor inconvenience (I can rewrite the check to make tsc happy) compared to the extra safety, IMO.
But there are a few places where we do if (arr.length > 2) or more, which are a bit awkward. However I don't think the checking .length would be totally type safe anyway since you can just modify it:
That would essentially be a sparse array, which is out of scope for this sort of thing. There's always non-standard or sneaky things that could be done to get around the type checker.
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.
Reacted by Fabian Keller, Simon Chan, Adrien Turiot, Tyler Laprade, CFA and AMORE
It seems to me that Perfectoff just wants to use a language that isn't Javascript. JS arrays have never had a concept of "out of bounds", Arrays have always been sparse, and Objects have always allowed arbitrary indexing. That's the whole reason undefined exists! If you want an array that can't be sparse, write one, but you're much better off learning to use iterators and forEach properly.
It seems to me that Perfectoff just wants to use a language that isn't Javascript. JS arrays have never had a concept of "out of bounds", Arrays have always been sparse, and Objects have always allowed arbitrary indexing. That's the whole reason undefined exists! If you want an array that can't be sparse, write one, but you're much better off learning to use iterators and forEach properly.
I see your point but almost nobody use the optional sparse capabilities of Arrays in their production applications. If types can help alleviate the historicaly poor technical choices of JS, why not do it.
If types can help alleviate the historicaly poor technical choices of JS, why not do it.
That's exactly what this issue was about! One could argue that sparse arrays are "one of the bad parts", sure. The fix for that, IMHO, is forcing you to check your indexed access, or avoid it altogether.
Oliver Joseph Ash (@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.
Reacted by Oliver Joseph Ash, Ardeshir Izadi, Sebastian Rogers, Oskar, Richard Waller, Jiří Cerhan, Ilija Tovilo, Hatzen, Serhan Coşkun, 0x-DLN and 2 more
Activity
mhegazy commentedon Feb 1, 2017
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 commentedon Feb 1, 2017
Mohamed Hegazy (@mhegazy) That's an interesting idea. Any guidance on how to override the type signatures for array/object?
gcnew commentedon Feb 1, 2017
There is
interface Array<T>inlib.d.ts. I searched by the regexp\[\w+: (string|number)\]to find other indexing signatures as well.OliverJAsh commentedon Feb 2, 2017
Interesting, so I tried this:
Any ideas?
mhegazy commentedon Feb 2, 2017
copy
lib.d.tslocally, 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.OliverJAsh commentedon Feb 2, 2017
Cool, thanks for that.
The issue with the suggested fix here is it requires forking and maintaining a separate
libfile.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
getmethod on ES6 collections (Map/Set) returnsT | undefinedwhenArray/Objectindex signatures do not.mhegazy commentedon Feb 2, 2017
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
| undefinedon all your index signatures, and you will get the expected behavior. but forArrayit 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 commentedon Feb 3, 2017
Mohamed Hegazy (@mhegazy) but for arrays with holes
a[i]is actually possibly undefined:Output is:
RyanCavanaugh commentedon Mar 14, 2017
We remain quite skeptical that anyone would get any benefit from this flag in practice. Maps and maplike things can already opt in to
| undefinedat 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 commentedon Mar 15, 2017
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
strictNullCheckson 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| undefinedfeels like TypeScript is failing to do what people actually want it for.kitsonk commentedon Mar 15, 2017
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.
234 remaining items
ericbf commentedon Sep 21, 2020
I'd say that's a bug. Basically if
keyof Typeonly includes string/number literal types (as opposed to actually includingstringornumber), thenType[Key]whereKey extends keyof Typeshould not includeundefined, I'd think.jcalz commentedon Oct 23, 2020
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"RyanCavanaugh commentedon Nov 20, 2020
Tracking some issues related to design limitations here
forloop should be OKlengthchecks should allow further destructuringrubenlg commentedon Nov 25, 2020
FYI: I caught two bugs in my codebase thanks to this flag, so big thank you!
It's a bit annoying that checking
arr.length > 0is not sufficient to guardarr[0], but that's a minor inconvenience (I can rewrite the check to make tsc happy) compared to the extra safety, IMO.Timmmm commentedon Nov 26, 2020
Rubén López (@rubenlg) I agree about
arr.length. For just checking the first element I've rewritten our code likeBut there are a few places where we do
if (arr.length > 2)or more, which are a bit awkward. However I don't think the checking.lengthwould be totally type safe anyway since you can just modify it:Prints
undefined.ericbf commentedon Nov 26, 2020
That would essentially be a sparse array, which is out of scope for this sort of thing. There's always non-standard or sneaky things that could be done to get around the type checker.
MartinJohns commentedon Nov 26, 2020
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.
Document that index signatures do not include undefined
Perfectoff commentedon May 27, 2021
I think the best solution would be to throw an exception when trying to access a non-existent element. This is in line with normal programming practice in statically typed languages. And if you want to get a nullable value without throwing an exception, then a method like "tryGet" is used.
As I see, this behavior can be implemented by replacing the code during the compilation to the javascript file, like this:
object [index] --> (object[index] ?? throw ("index out of range"))
object.tryGet [index] --> object[index]
And, accordingly, add the tryGet method to the Array, Map and other interfaces with indexer.
rubenlg commentedon May 27, 2021
Perfectoff As a principle, TypeScript never changes the runtime behavior of JavaScript code:
https://www.typescriptlang.org/docs/handbook/typescript-from-scratch.html#runtime-behavior
What you are proposing are run-time assertions, and that could be implemented as a separate library.
thw0rted commentedon May 28, 2021
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
undefinedexists! If you want an array that can't be sparse, write one, but you're much better off learning to use iterators andforEachproperly.AlexGalays commentedon May 28, 2021
I see your point but almost nobody use the optional sparse capabilities of Arrays in their production applications. If types can help alleviate the historicaly poor technical choices of JS, why not do it.
thw0rted commentedon May 28, 2021
That's exactly what this issue was about! One could argue that sparse arrays are "one of the bad parts", sure. The fix for that, IMHO, is forcing you to check your indexed access, or avoid it altogether.
nh2 commentedon Oct 23, 2021
Oliver Joseph Ash (@OliverJAsh) Coul dyou edit the issue description to mention that
--noUncheckedIndexedAccessin TypeScript 4.1 fixes this?The thread is so long and still continuously edited that the solution is hard to find.
FE: Configure typescript to warn on undefined index access