Skip to content
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

'in' expression should be a type guard #1427

Closed
JsonFreeman opened this issue Dec 10, 2014 · 6 comments
Closed

'in' expression should be a type guard #1427

JsonFreeman opened this issue Dec 10, 2014 · 6 comments
Labels
Duplicate An existing issue was already created Suggestion An idea for TypeScript

Comments

@JsonFreeman
Copy link
Contributor

The in operator provides an opportunity for us to narrow a type. There are two ways that this could work:

  1. We can select particular constituents of a union type based on which members are present:
interface I1 {
    p: string;
}
interface I2 {
    q: string;
}

var v: I1 | I2;
if ("p" in v) {
    v.p; // Error
}
  1. If we have a general type like Object, we can clone the type and add a specific property to it when narrowing:
var v: Object;
if ("p" in v) {
    v.p; // Type should be { p: any }
}
@JsonFreeman JsonFreeman added the Suggestion An idea for TypeScript label Dec 10, 2014
@jeffreymorlan
Copy link
Contributor

There's a hidden trap inherent in using property existence to distinguish the types in a union: imagine if elsewhere in the code there is a type interface I2Ext extends I2 { p: number; }, and an I2Ext makes its way into the function that uses p to distinguish I1 | I2. Any time you extend a type and add a property you'd have to worry about whether the name you picked will cause it to be misidentified somewhere...

This could be made safe if there were a way to specify that an object lacks a certain property, and that were used to determine which parts of a union get narrowed away when in evaluates to true.

@JsonFreeman
Copy link
Contributor Author

Yes, that is a great point. I think the crux of it is that you can't eliminate a constituent based on the fact that it lacks a property you observe to be present. But as you alluded to, if there were an else block, it would be safe to narrow there based on the observation that a property is absent.

@JsonFreeman
Copy link
Contributor Author

@danquirk This problem that @jeffreymorlan points out applies to property accesses as well.

@RyanCavanaugh RyanCavanaugh added the Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. label Dec 17, 2014
@RyanCavanaugh
Copy link
Member

We should look at how prevalent this "assume X based on presence of Y" pattern is in real code.

@awerlang
Copy link

Today we would have:

interface I1 {
    prop1: string;
    doSomething(): string;
}
interface I2 {
    prop2: string;
    doAnotherThing(): string;
}
class MyObject implements I2 {
    prop1: string = "";
    prop2: string = "";
    doAnotherThing() {
        return 'doAnotherThing';
    }
    doSomething() {
        return 'doSomething';
    }
}
function test(obj: I1 | I2) {
    if ('prop1' in obj) {
        alert('doSomething() = ' + (<I1> obj).doSomething());
    } else {
        alert('doAnotherThing() = ' + (<I2> obj).doAnotherThing());
    }
}

test(new MyObject);

As there's no type guard, I just type cast. I believe having 'in' as type guard is no more dangerous than a typecast. It's even a little better, perharps .

@mhegazy
Copy link
Contributor

mhegazy commented Sep 20, 2016

closing in favor of #10485

@mhegazy mhegazy closed this as completed Sep 20, 2016
@mhegazy mhegazy added Duplicate An existing issue was already created and removed Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Sep 20, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants