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

Destructured parameter does not result in most specific type #12144

Closed
tp opened this issue Nov 10, 2016 · 3 comments
Closed

Destructured parameter does not result in most specific type #12144

tp opened this issue Nov 10, 2016 · 3 comments
Labels
Duplicate An existing issue was already created

Comments

@tp
Copy link

tp commented Nov 10, 2016

TypeScript Version: 2.0.3
Code

type X = { type: 'number', value: number } | { type: 'string', value: string };

const f = ({ type, value }: X) => {
    if (type === 'number') {
        value; // <--
    }
};

Expected behavior:

Value should be of type number.

Actual behavior:

Value is of type string | number.

(When not using parameter destructuring, and then using an if / switch on the object, this works as expected.)

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label May 24, 2017
@bfricka
Copy link

bfricka commented Jul 23, 2017

Here's an example showing the discrepancy, and it also highlights something not particularly intuitive which I'll refer to after:

const ids = { X: 'x' as 'x', Y: 'y' as 'y' };
type ids = (typeof ids)[keyof typeof ids];

class X {
    id = ids.X;
    value = 42;
}

class Y {
    id = ids.Y;
    value = 'Why';
}

type XY = X | Y;
// Normal params
function normalParams(v: XY) {
    switch (v.id) {
        case ids.X:
            // (property) X.value: number
            return v.value;
        case ids.Y:
            // (property) Y.value: number
            return v.value;
    }
}

// const normalX: string | number
const normalX = normalParams(new X);

// Destructured
function destructuredParams({ id, value }: XY) {
    switch (id) {
        case ids.X:
            // var value: string | number
            return value;
        case ids.Y:
            // var value: string | number
            return value;
    }
}

// const destructuredX: string | number
const destructuredX = destructuredParams(new X);

So two things:

  1. The discrepancy means that, in the destructuring case, you'd have to type cast usage of value in cases where string | number is not accepted.
  2. Even though the normal case correctly understands X.value, it doesn't change the return type.

We can solve this somewhat using generics:

class Z<T> {
    constructor(public id: ids, public value: T) {}
}

// Normal params
function normalParams<T>(v: Z<T>): T {
    switch (v.id) {
        case ids.X:
            // (property) Z<T>.value: T
            return v.value;
        case ids.Y:
            // (property) Z<T>.value: T
            return v.value;
    }
}

// const normalZ: number
const normalZ = normalParams(new Z(ids.X, 42));

// Destructured
function destructuredParams<T>({ id, value }: Z<T>): T {
    switch (id) {
        case ids.X:
            // var value: T
            return value;
        case ids.Y:
            // var value: T
            return value;
    }
}

// const destructuredZ: string
const destructuredZ = destructuredParams(new Z(ids.Y, 'Why?'));

This approach isn't always possible though.

@mhegazy mhegazy added Duplicate An existing issue was already created and removed Needs Investigation This issue needs a team member to investigate its status. labels Nov 9, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Nov 9, 2017

once an object is destructed, the compiler can no longer make any assumptions about the relationships between the parts. Doing so requires data-flow analysis and alias tracking which is not trivial tasks.

Duplicate of #10976

@typescript-bot
Copy link
Collaborator

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 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
Projects
None yet
Development

No branches or pull requests

5 participants