Consider property access a form of type guards

[danquirk]

This is a proposal for using property access as another form of type guards (see #900) to narrow union types. While we're investigating expanding the power of type guards (#1007) this feature would support the natural style that JavaScript programmers have in their code today.

Using property access to narrow union types

var x: string|number;
if (x.length) { // no error even though number lacks a length property
    var r = x.charAt(3); // x is of type string inside this block
}

var r2 = x.length || x * x; // no error, x narrowed to number on right hand side, r3 is number

var y: string[]|number[]|string;
if (y.length && y.push) {
    // y is string[]|number[] now
    var first = y[0]; // first is string|number
    if (first.length) {
        // first is string in here
    } else {
        // first is number in here
    }
}

We do not expand the situations in which types are narrowed, but we do expand the known type guard patterns to include basic property access. In these narrowing contexts it would be an error to access a property that does not exist in at least one of the constituent types of a union type (as it is today). However, it would now be valid to access a property that exists in at least one, but not all, constituent types. Any such property access will then narrow the type of the operand to only those constituent types in the union which do contain the accessed property. In any other context property access is unchanged.

Invalid property access

var x: string|number;

var r = x.length; // error, not a type guard, normal property access rules apply

if (x.len) { } // error, len does not exist on type string|number 

var r3 = !x.len && x.length; // error, len does not exist on type string|number 

var r4 = x.length && x.len; // error, len does not exist on type string

Issues/Questions

• Should the language service behave differently in these type guard contexts? Should dotting off a union type in a type guard list all members on all types rather than only those that exist on all of them?
• Need to understand performance implications

I have a sample implementation and tests in a branch here: https://github.com/Microsoft/TypeScript/tree/typeGuardsViaPropertyAccess. There're a couple bugs remaining but examples like the above all work and no existing behavior was changed.

[DanielRosenwasser]

👍 for this.

For the existing issues/questions:

Should the language service behave differently in these type guard contexts? Should dotting off a union type in a type guard list all members on all types rather than only those that exist on all of them?

I assume that solely in these typeguard contexts, you're allowing a property access for properties that exist on any of the constituent types. If so, I'd think it makes sense to extend this sort of logic to the language service. After all, I assume we'll still give errors in the appropriate contexts. For instance,

interface A { bar: { bizzle: any } };
interface B { }
var foo: A | B;
if (foo.bar

is fine, but as soon as you have something like

var foo: A | B;
if (foo.bar.bizzle

the LS will complain that bar does not exist on type A | B, and hopefully that bizzle is a ridiculous property name. The biggest problem is that that's not a terribly helpful error message, which we could work on.

Need to understand performance implications

Shouldn't be terrible if user-defined type tags supposedly won't affect perf either, but it'll be good to keep an eye on it.

[AbubakerB]

Any update on this?

if i can weigh in:

Should the language service behave differently in these type guard contexts? Should dotting off a union type in a type guard list all members on all types rather than only those that exist on all of them?

In addition to what Daniel said, what would be cool is when getCompletion is called, intellisense shows an info/warning symbol near the properties that don't belong to the union type and maybe some text indicating that this property exists in only TypeA or TypeA | TypeB or TypeC etc...

Also, in the case of

var foo: A | B;
if (foo.bar.bizzle

if bar doesn't exist in the union type, maybe
if (foo.bar.bizzle)
could be emitted to
if (foo.bar && foo.bar.bizzle)
and that way, we could allow completion.

[mykohsu]

Since this is still open, it seems that the original example here is no longer valid?

if (x.length) { // no error even though number lacks a length property
    var r = x.charAt(3); // x is of type string inside this block
}

The code above now errors at x.length and seems this is the case per https://github.com/Microsoft/TypeScript-Handbook/blob/master/pages/Advanced%20Types.md

Can anyone confirm?

[DanielRosenwasser]

The example was invalid at the time this was written as well. The idea was that at certain locations where you want to check a member on the type, you shouldn't need to cast, and that if certain types in a union lacked that member, they'd be "narrowed out".

[rcollette]

+1

I hate noise +1'ing. Some repos seem to have a separate thumbs up icon you can click on under the original issue post and it will tally. Absent that, I'm indicating this is of value.

[weswigham]

@rcollette There's an emoji smiley in the upper right hand corner of all comments - use that to add a 👍 (or other reaction), as you just described.

[rcollette]

Ah. The you can click on it to add to it once its there. I've never been first I guess. (gee that reminds me of the days when you'd see "first" as the only response to a post).

[EliSnow]

I would love to see this implemented. User-defined type guard functions which are currently required in lieu of this proposal, litter the code with functions which would be unnecessary in vanilla javascript. These functions exist only to support the type system, yet they remain when compiled down to javascript.

[EliSnow]

Another benefit of implementing property access type guards is that user-defined type guards are often unsafe. For example:

interface Foo {
  a: string;
}

interface Bar {
  b: string;
}

function isFoo (o : Foo | Bar): o is Foo {
  return (<Foo> o).a !== undefined;
}

This works as intended. But if for some reason, later on Bar is expanded to:

interface Bar {
  b: string;
  a: string;
}

Now we have a bug that is uncaught by the type checker because the type guard essentially bypasses it with the type assertion. It would seem that type assertions are almost always present in user-defined type guards.

[AlCalzone]

Since this is tagged "Awaiting more feedback", I'll just link to #36194 as a potential use case.

[jsejcksn]

Just curious: Is this simply a yet-to-be-implemented feature request or has it not been implemented due to some kind of technical cost or differing opinion?

[RyanCavanaugh]

Just curious: Is this simply a yet-to-be-implemented feature request or has it not been implemented due to some kind of technical cost or differing opinion?

"Awaiting More Feedback" generally means that this makes sense, and would presumably be OK to add to the language, but the quantity and quality of feedback heard so far has not indicated that this is what we should spend churn, complexity, and risk on.

This particular change request is very problematic from a soundness perspective, especially compared to the much clearer intent of using in (which already works), so the bar is set rather high in that regard.

[thorn0]

Feedback for you. Prettier is a formatter that works with ASTs that come from different parsers. In the case of JS-based languages, these AST formats are all based on the same ESTree base format, but also they have differences. It'd be great to be able to use a union type for this, and it looks like it might work (simplified code for two parsers only):

import * as Babel from "@babel/types";
import { TSESTree } from "@typescript-eslint/types";
type Node = Babel.Node | TSESTree.Node;

but in practice the resulting Node type is a pain to work with.

E.g., this already quite complex condition

    node.type === "MethodDefinition" &&
    node.value &&
    node.value.type === "FunctionExpression" &&
    getFunctionParameters(node.value).length === 0 &&
    !node.value.returnType &&

needs to be rewritten this way to make TS happy:

    node.type === "MethodDefinition" &&
    node.value &&
    node.value.type === "FunctionExpression" &&
    getFunctionParameters(node.value).length === 0 &&
    (!("returnType" in node.value) || !node.value.returnType)

The added in check doesn't make code any clearer and only bloats it. The entire code base needs adding a lot of such checks, which is why any is used almost everywhere instead of Node.

[jsejcksn]

"Awaiting More Feedback" generally means that this makes sense, and would presumably be OK to add to the language, but the quantity and quality of feedback heard so far has not indicated that this is what we should spend churn, complexity, and risk on.

@RyanCavanaugh Can you link me to an explainer that includes this and other information about how the TS team uses labels and other meta-features for issues in this repo? I'd like to be able to link other people who haven't been informed yet, and I imagine it will keep other similar replies DRYer.

[therynamo]

Just wanted to check in and see if there were any updates on this - this would be a fantastic feature! Let me know if there is anything I can do to help.

[Telokis]

The example from prettier in this comment is very representative of real-life use-cases.
From my understanding, one of the goals of TypeScript is to embrace current JavaScript practices and checking non-existing properties in ifs is an often used idiom. I know it's not as safe as using in but it's so much more convenient that it ends up being the default solution most of the time.
This issue is now even more relevant when we consider Optional chaining because using in becomes a nightmare in conditions and really bloats the code (like in the Prettier example above).

One of the solutions I'm considering is to virtually add all properties to all members of my unions and set some of them to prop?: undefined;. This will let my users migrate from JavaScript to TypeScript in a way more effective and comfortable way.

Example:
Instead of the following code

interface A {
  a: string;
};

interface B {
  b: string;
};

type U = A | B;

const val: U = getValueFromSomewhere();

if ("a" in val) {
  // Ok, val is of type A
}

I will do something equivalent to the following:

interface A {
  a: string;

  // Fake properties to make TS happy
  b?: undefined;
};

interface B {
  b: string;

  // Fake properties to make TS happy
  a?: undefined;
};

type U = A | B;

const val: U = getValueFromSomewhere();

if (val.a) {
  // Ok, val is of type A
}

In my opinion, it's a tradeoff. I know this will reduce the type-safety but since it's easier to use and reason with, it will lead to more people moving from JS to TS without having to change their code too much and in a too annoying way which actually increases type-safety.
Throughout the years, I've come to this issue several times and this, in my opinion, is a proof that this is a really missing feature.

Edit: Wrapper helping with the UX issue

I've created a type to help solve the problem. Here is the things you need:

// Creates a union of all keys of all objects in the Terface union
type AllKeys<Terface> = Terface extends any ? (keyof Terface & (string | number | symbol)) : never

// Creates a new interface adding the missing keys to Terface
type Wrap<Terface, Keys extends string | number | symbol> = (Terface & {
    [K in Exclude<Keys, keyof Terface>]?: undefined;
})

// Distributes the union and automatically add the missing keys
type BetterUXWrapper<Terface, Keys extends AllKeys<Terface> = AllKeys<Terface>> = Terface extends any ? Wrap<Terface, Keys> : never;

Here is an example usage:

interface A {
  a: string;
};

interface B {
  b: string;
};

type URaw = A | B;
type U = BetterUXWrapper<URaw >;

declare const val: U;

if (val.a) {
  // Ok, val is of type A
}

Typescript playground

[forivall]

The BetterUxWrapper has also been described as ExclusifyUnion in this StackOverflow answer, and I called it UnionExpand when i independently figured it out (before finding that StackOverflow answer)