-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
typeof union.membername errors. #22598
Comments
I think this is a bug. I guess the rule is that for @nomcopter, in #22094 you did something similar. |
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 |
I agree with @weswigham . Additionally, this example is originally wrong. declare var value: string | typeof Date;
if (typeof value.now /*error!*/ !== "undefined") {} |
@weswigham I don't understand why type narrowing by property existance has to function differently to type narrowing by property string literal? @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 Actually, to further summarise, you don't think you should do this because you want to implicitly treat all unions as though they have 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 type Union = {kind: "a", Foo: number} | {kind: "b", Foo: string} | {kind: "c"};
let a: Union;
function getVal() {
return { kind: "c" as "c", Foo: true }
}
a = Math.random() > 0.5 ? getVal() : { kind: "a", Foo: 12 }
if (typeof a.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 interface String {
now: undefined;
}
declare var value: string | typeof Date;
if (typeof value.now !== "undefined") {
value.now();
} |
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. |
@RyanCavanaugh I agree (thus the comment about type narrowing). Related issue: This issue: I see both of these issues as two sides of the same coin, and it bugs me. @weswigham by your argument the following should also be valid, and return an any type. interface Union { kind: "c" };
let a: Union;
function getVal() {
return { kind: "c" as "c", Foo: true }
}
a = getVal();
if (typeof a.Foo /*error*/ !== "undefined") Which I understand, and am OK with. We can do additional I do think that Alternatively combining it with some syntax to retype the variable in the following codeblock, such as (example, not proposed syntax): if (typeof a.Foo !== "undefined") a is {kind: "a", Foo: number} | {kind: "b", Foo: string}
{
} Which also checks whether or not 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. |
At the moment, all over our code we've had to do things like this: var matchingoptions = <{value: WriteQuery<any>, queryname: string} | {value: WriteQuery<any>, query: ReadQuery<any>}> write[operator];
if (typeof (<any>matchingoptions).queryname !== "undefined") {
arraypath = path + ".$[" + (<any>matchingoptions).queryname + "]";
} I do not like having to introduce |
You can use |
@RyanCavanaugh I like that answer and will use that. I do have a question though; why does the In javascript aren't they essentially performing the same job? |
We could have at most one consistency. The |
IIRC, the slight unsoundness of it's narrowing behavior when it was implemented and just kinda shrugged off, since it provides an option for people like @Griffork who want an unsound way to make a membership check and narrow, and we didn't think people really used |
Actually, I think what I suggested would work better with this proposal: |
@RyanCavanaugh I'm trying to show this functionality to my team-mates but don't seem to be able to find |
@Griffork do you consider this use case adequately solved by |
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 |
@RyanCavanaugh it's enough for me. I'd like it to be in the language docs somewhere. |
Thanks! I agree the docs need some love. |
TypeScript Version: 2.8.0-dev.20180308
Search Terms:
typeof union parameter undefined.
Code
Expected behavior:
No error and appropriate type narrowing.
Actual behavior:
Playground Link
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?).
The text was updated successfully, but these errors were encountered: