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
NOTE
I don't like having to make a new function to properly narrow types for every union I create, is there another way around this (like manually overriding the variable's type within the scope of a code block without declaring a new variable?).
I think this is a bug. I guess the rule is that for typeof expr.id, if expr is a union type and id exists on at least one member, permit the check and narrow based on the property.
I definitely don't think this is bug (it's a feature request) because this is definitely how union type property accesses are intended to work (properties only exist if they're in all union members, and that's how they've worked forever)... however allowing accesses of nonexistant union member properties for the purposes of checking for their existence seems reasonableish? Only -ish, though... I think union excess property checking will catch most of the obvious problems that could have cropped up from allowing it nowadays, so... it may be fine? I can't convince myself it definitely is... Like, here's the deal, if I say I have type Union = {kind: "a", Foo: number} | {kind: "b", Foo: string} | {kind: "c"}, and a variable x of type Union, I cannot confidently say that x.Foo is string | number | undefined, because someone could have used {kind: "c", Foo: true} as a perfectly valid value for that variable; all I can say is any, really, which doesn't help. Unions aren't actually closed to subtypes, which muddles this - discrimination is only possible for discriminable unions precisely because a field exists on all union members and whose types give that field a finite domain of possible values into which each possible union member can be bucketed (which is also why we only discriminate on unit types).
Wesley Wigham (@weswigham) I don't understand why type narrowing by property existance has to function differently to type narrowing by property string literal?
falsandtru (@falsandtru) thanks! Ironically that's a bug that would have been caught by this "proposal".
By the logic that union types are meant to be extended, my argument is that so are interface types. I don't know why you'd cater for invalid assignment during member access on a union type and not also allow it on an interface type (by that logic all interfaces should implicitly have [param: string]:any). That seems extremely counterintuitive.
Actually, to further summarise, you don't think you should do this because you want to implicitly treat all unions as though they have [param: string]:any?
As someone who chooses to code with strict mode on I really dislike that line of logic.
Griffork No? The point is that when you ask for the type of value.now when value is type string | typeof Date, since now only exists on one union member, nothing is constraining its' type on the other member, meaning you could have potentially assigned a value of a conflicting type to it in the past. For example,
typeUnion={kind: "a",Foo: number}|{kind: "b",Foo: string}|{kind: "c"};leta: Union;functiongetVal(){return{kind: "c"as"c",Foo: true}}a=Math.random()>0.5 ? getVal() : {kind: "a",Foo: 12}if(typeofa.Foo!=="undefined"){// a.Foo exists, yes. What is a.Foo's type here? No idea. Can't say it's `number | string` -// we've clearly just constructed an expression above where it might not be! What's// `a`'s type? Don't know. Can't draw any conclusions on the type of `a` based on the// presence of `Foo`, since not all union members have a known value for it.}
So while property access won't immediately throw, it is not safe to use (and using it would likely be a code smell), since its type is indeterminate!
You should probably just cast inside your typeof. Alternatively, augment the String interface with an explicit property stating the type of the now member (undefined), to make the fields mutually exclusive (and prevent you from ever assigning something with a defined now type to a string):
FWIW Flow just changed their behavior from "Any property is OK in a conditional" to match the proposed behavior (a property is OK in a conditional if it exists in any union constituent): facebook/flow@c39aa44
Still not a huge fan of expression typechecking behaving differently depending on the surround context, e.g. this
Yeah, it's probably a good thing that they tightened their checking a bit, however I'm not a fan of loosening ours, given there's very little information we can safely derive from the check, and you can always just use a cast to perform the check anyway.
Related issue:
In javascript you can typeof check a value that hasn't been declared. In typescript this is an error. (I don't think it should be)
This issue:
In javascript you can typeof check a property of an object that hasn't been declared. In typescript this is an error. (I don't think it should be)
I see both of these issues as two sides of the same coin, and it bugs me.
For the first one, I like to do a test (within a shared library) for global properties that only exist on the server, but on the client this test errors if I don't declare the properties (making code sharing really hard).
Which I understand, and am OK with. We can do additional typeof checks to further narrow the scope.
I do think that typeof should still be possible on non-existing members of an object (although I don't know how to resolve this properly with spelling errors, maybe make it only available with string literals in the [] notation?).
Alternatively combining it with some syntax to retype the variable in the following codeblock, such as (example, not proposed syntax):
Which also checks whether or not a is actually assignable to the resulting type would be good.
Alternatively (again, and this one is a crazy idea) maybe make invalid/undefined properties a different colour in the syntax highlighting? That way it would be obvious why typeof works and nothing else does.
I do not like having to introduce <any> everywhere I use a union because I can't type narrow on property existance.
This happens everywhere.
I want type safety within my if statements and I can't have it. This is why I thought of this as a bug, and why I'm asking for it to be fixed.
You can use if ("queryname" in matchingoptions) { which narrows and doesn't error. One advantage of using it in a union position is that if you misspelll the left operand, the right operand isn't narrowed (which hopefully clues you in).
Reacted by falsandtru, Griffork, David Neil and timm-aym
I do have a question though; why does the in operator behave differently to the typeof operator in this context? Won't it have the same problems as Wesley Wigham (@weswigham)'s example with the getVal scenario?
In javascript aren't they essentially performing the same job?
The in operator is used for all kinds of dynamic tests and we never had a rule in place to disallow any left operand of it, so we blessed it with the narrowing behavior because why not. There's currently no special casing for property access anywhere.
Ryan Cavanaugh (@RyanCavanaugh) I'm trying to show this functionality to my team-mates but don't seem to be able to find in's behaviour anywhere in the docs. Am I missing something?
Support for exact (sealed/frozen) types in TypeScript, in case they're in the union being tested, would solve the issues of not being sure of the narrowed type after doing the typeof check.
I'd like it to be in the language docs somewhere.
It's a pain getting new people up to speed on typescript because a lot of useful features are not covered at all in the language docs but are halfway down a random blog post (and you can't find it unless you know the exact name for it).
Activity
DanielRosenwasser commentedon Mar 15, 2018
I think this is a bug. I guess the rule is that for
typeof expr.id, ifexpris a union type andidexists on at least one member, permit the check and narrow based on the property.Kevin Verdieck (@nomcopter), in #22094 you did something similar.
weswigham commentedon Mar 15, 2018
I definitely don't think this is bug (it's a feature request) because this is definitely how union type property accesses are intended to work (properties only exist if they're in all union members, and that's how they've worked forever)... however allowing accesses of nonexistant union member properties for the purposes of checking for their existence seems reasonableish? Only -ish, though... I think union excess property checking will catch most of the obvious problems that could have cropped up from allowing it nowadays, so... it may be fine? I can't convince myself it definitely is... Like, here's the deal, if I say I have
type Union = {kind: "a", Foo: number} | {kind: "b", Foo: string} | {kind: "c"}, and a variablexof typeUnion, I cannot confidently say thatx.Fooisstring | number | undefined, because someone could have used{kind: "c", Foo: true}as a perfectly valid value for that variable; all I can say isany, really, which doesn't help. Unions aren't actually closed to subtypes, which muddles this - discrimination is only possible for discriminable unions precisely because a field exists on all union members and whose types give that field a finite domain of possible values into which each possible union member can be bucketed (which is also why we only discriminate on unit types).falsandtru commentedon Mar 15, 2018
I agree with Wesley Wigham (@weswigham) . Additionally, this example is originally wrong.
typeofis missing:Griffork commentedon Mar 15, 2018
Wesley Wigham (@weswigham) I don't understand why type narrowing by property existance has to function differently to type narrowing by property string literal?
falsandtru (@falsandtru) thanks! Ironically that's a bug that would have been caught by this "proposal".
By the logic that union types are meant to be extended, my argument is that so are interface types. I don't know why you'd cater for invalid assignment during member access on a union type and not also allow it on an interface type (by that logic all interfaces should implicitly have
[param: string]:any). That seems extremely counterintuitive.Actually, to further summarise, you don't think you should do this because you want to implicitly treat all unions as though they have
[param: string]:any?As someone who chooses to code with strict mode on I really dislike that line of logic.
weswigham commentedon Mar 15, 2018
Griffork No? The point is that when you ask for the type of
value.nowwhenvalueis typestring | typeof Date, sincenowonly exists on one union member, nothing is constraining its' type on the other member, meaning you could have potentially assigned a value of a conflicting type to it in the past. For example,So while property access won't immediately throw, it is not safe to use (and using it would likely be a code smell), since its type is indeterminate!
You should probably just cast inside your
typeof. Alternatively, augment theStringinterface with an explicit property stating the type of thenowmember (undefined), to make the fields mutually exclusive (and prevent you from ever assigning something with a definednowtype to astring):RyanCavanaugh commentedon Mar 16, 2018
FWIW Flow just changed their behavior from "Any property is OK in a conditional" to match the proposed behavior (a property is OK in a conditional if it exists in any union constituent): facebook/flow@c39aa44
Still not a huge fan of expression typechecking behaving differently depending on the surround context, e.g. this
weswigham commentedon Mar 16, 2018
Yeah, it's probably a good thing that they tightened their checking a bit, however I'm not a fan of loosening ours, given there's very little information we can safely derive from the check, and you can always just use a cast to perform the check anyway.
Griffork commentedon Mar 16, 2018
Ryan Cavanaugh (@RyanCavanaugh) I agree (thus the comment about type narrowing).
Related issue:
In javascript you can
typeofcheck a value that hasn't been declared. In typescript this is an error. (I don't think it should be)This issue:
In javascript you can
typeofcheck a property of an object that hasn't been declared. In typescript this is an error. (I don't think it should be)I see both of these issues as two sides of the same coin, and it bugs me.
For the first one, I like to do a test (within a shared library) for global properties that only exist on the server, but on the client this test errors if I don't declare the properties (making code sharing really hard).
Wesley Wigham (@weswigham) by your argument the following should also be valid, and return an any type.
Which I understand, and am OK with. We can do additional
typeofchecks to further narrow the scope.I do think that
typeofshould still be possible on non-existing members of an object (although I don't know how to resolve this properly with spelling errors, maybe make it only available with string literals in the [] notation?).Alternatively combining it with some syntax to retype the variable in the following codeblock, such as (example, not proposed syntax):
Which also checks whether or not
ais actually assignable to the resulting type would be good.Alternatively (again, and this one is a crazy idea) maybe make invalid/undefined properties a different colour in the syntax highlighting? That way it would be obvious why typeof works and nothing else does.
Griffork commentedon Mar 16, 2018
At the moment, all over our code we've had to do things like this:
I do not like having to introduce
<any>everywhere I use a union because I can't type narrow on property existance.This happens everywhere.
I want type safety within my if statements and I can't have it. This is why I thought of this as a bug, and why I'm asking for it to be fixed.
RyanCavanaugh commentedon Mar 16, 2018
You can use
if ("queryname" in matchingoptions) {which narrows and doesn't error. One advantage of using it in a union position is that if you misspelll the left operand, the right operand isn't narrowed (which hopefully clues you in).Griffork commentedon Mar 16, 2018
Ryan Cavanaugh (@RyanCavanaugh) I like that answer and will use that.
I do have a question though; why does the
inoperator behave differently to thetypeofoperator in this context? Won't it have the same problems as Wesley Wigham (@weswigham)'s example with thegetValscenario?In javascript aren't they essentially performing the same job?
RyanCavanaugh commentedon Mar 16, 2018
We could have at most one consistency.
The
inoperator is used for all kinds of dynamic tests and we never had a rule in place to disallow any left operand of it, so we blessed it with the narrowing behavior because why not. There's currently no special casing for property access anywhere.1 remaining item
Griffork commentedon Mar 16, 2018
Actually, I think what I suggested would work better with this proposal:
#14094
Griffork commentedon Apr 4, 2018
Ryan Cavanaugh (@RyanCavanaugh) I'm trying to show this functionality to my team-mates but don't seem to be able to find
in's behaviour anywhere in the docs. Am I missing something?mhegazy commentedon Apr 4, 2018
https://github.com/Microsoft/TypeScript/wiki/What's-new-in-TypeScript#type-guards-inferred-from--in-operator
RyanCavanaugh commentedon Aug 13, 2018
Griffork do you consider this use case adequately solved by
in, or should we keep this on the backlog of things to review?niieani commentedon Aug 13, 2018
Support for exact (sealed/frozen) types in TypeScript, in case they're in the union being tested, would solve the issues of not being sure of the narrowed type after doing the
typeofcheck.Griffork commentedon Aug 13, 2018
Ryan Cavanaugh (@RyanCavanaugh) it's enough for me.
I'd like it to be in the language docs somewhere.
It's a pain getting new people up to speed on typescript because a lot of useful features are not covered at all in the language docs but are halfway down a random blog post (and you can't find it unless you know the exact name for it).
RyanCavanaugh commentedon Aug 14, 2018
Thanks! I agree the docs need some love.