Skip to content

typeof union.membername errors. #22598

Closed
Closed
@Griffork

Description

@Griffork

TypeScript Version: 2.8.0-dev.20180308

Search Terms:
typeof union parameter undefined.

Code

declare var value: string | Date;
if (typeof value.now /*error!*/ !== "undefined") {}

Expected behavior:
No error and appropriate type narrowing.

Actual behavior:

[ts]
Property 'now' does not exist on type 'string | Date'.
  Property 'now' does not exist on type 'string'.

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?).

Activity

DanielRosenwasser

DanielRosenwasser commented on Mar 15, 2018

@DanielRosenwasser
Member

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.

@nomcopter, in #22094 you did something similar.

weswigham

weswigham commented on Mar 15, 2018

@weswigham
Member

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).

falsandtru

falsandtru commented on Mar 15, 2018

@falsandtru
Contributor

I agree with @weswigham . Additionally, this example is originally wrong. typeof is missing:

declare var value: string | typeof Date;
if (typeof value.now /*error!*/ !== "undefined") {}
Griffork

Griffork commented on Mar 15, 2018

@Griffork
Author

@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 [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

weswigham commented on Mar 15, 2018

@weswigham
Member

@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,

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 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):

interface String {
  now: undefined;
}
declare var value: string | typeof Date;
if (typeof value.now !== "undefined") {
  value.now();
}
RyanCavanaugh

RyanCavanaugh commented on Mar 16, 2018

@RyanCavanaugh
Member

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

weswigham commented on Mar 16, 2018

@weswigham
Member

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

Griffork commented on Mar 16, 2018

@Griffork
Author

@RyanCavanaugh I agree (thus the comment about type narrowing).

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).

@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 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):

if (typeof a.Foo !== "undefined") a is {kind: "a", Foo: number} | {kind: "b", Foo: string} 
{

}

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.

Griffork

Griffork commented on Mar 16, 2018

@Griffork
Author

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 <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

RyanCavanaugh commented on Mar 16, 2018

@RyanCavanaugh
Member

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

Griffork commented on Mar 16, 2018

@Griffork
Author

@RyanCavanaugh I like that answer and will use that.

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 @weswigham's example with the getVal scenario?

In javascript aren't they essentially performing the same job?

RyanCavanaugh

RyanCavanaugh commented on Mar 16, 2018

@RyanCavanaugh
Member

We could have at most one consistency.

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.

4 remaining items

added
SuggestionAn idea for TypeScript
and removed
BugA bug in TypeScript
on Mar 29, 2018
Griffork

Griffork commented on Apr 4, 2018

@Griffork
Author

@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?

RyanCavanaugh

RyanCavanaugh commented on Aug 13, 2018

@RyanCavanaugh
Member

@Griffork do you consider this use case adequately solved by in, or should we keep this on the backlog of things to review?

niieani

niieani commented on Aug 13, 2018

@niieani

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.

Griffork

Griffork commented on Aug 13, 2018

@Griffork
Author

@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).

added
DeclinedThe issue was declined as something which matches the TypeScript vision
and removed on Aug 14, 2018
RyanCavanaugh

RyanCavanaugh commented on Aug 14, 2018

@RyanCavanaugh
Member

Thanks! I agree the docs need some love.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    DeclinedThe issue was declined as something which matches the TypeScript visionSuggestionAn idea for TypeScript

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @niieani@DanielRosenwasser@weswigham@falsandtru@Griffork

        Issue actions

          typeof union.membername errors. · Issue #22598 · microsoft/TypeScript