Skip to content

Control flow analysis: T|T[] case, Array.isArray and else case #8935

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

Closed
Strate opened this issue Jun 2, 2016 · 5 comments
Closed

Control flow analysis: T|T[] case, Array.isArray and else case #8935

Strate opened this issue Jun 2, 2016 · 5 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@Strate
Copy link

Strate commented Jun 2, 2016

TypeScript Version:

1.9.0-dev (nightly)

Code

// A self-contained demonstration of the problem follows...
function <T>(arg: T|T[]) {
  if (Array.isArray(arg)) {
    // arg expected to be T[] here, and it has type T[] (works as expected)
  } else {
    // arg expected to be T here, but it is has type T|T[] (unexpected behaviour)
  }
}
@mhegazy
Copy link
Contributor

mhegazy commented Jun 2, 2016

do not think this is accurate. T can be an array.

foo<number[]>([[1],[2]]);

@mhegazy mhegazy added the Working as Intended The behavior described is the intended behavior; this is not a bug label Jun 2, 2016
@Strate
Copy link
Author

Strate commented Jun 2, 2016

Same thing about non generic functions:

arg: string|string[]

@mhegazy
Copy link
Contributor

mhegazy commented Jun 2, 2016

animation

@Strate
Copy link
Author

Strate commented Jun 3, 2016

@mhegazy Sorry for inconvience, there should be a little more complex case.

interface Struct {
    type: string
    [key: string]: any
}
function handle(object: Struct | Struct []) {
    if (Array.isArray(object)) {
        for (let single of object) {
            handle(single)
        }
    } else {
        Object.keys(object).forEach(key => {
            let value = object[key];
            if (Array.isArray(value)) {
                console.log(value.length);
            } else {
                console.log(object.type);
            }
        })
    }
}

This code raises compilation error on typescript 1.9.0-dev.20160602-1.0 on line console.log(object.type):

 error TS2339: Property 'type' does not exist on type 'Struct | Struct[]'. 

But with typescript 1.8 everything is ok.

@mhegazy
Copy link
Contributor

mhegazy commented Jun 3, 2016

The issue is a change in how narowing works. in TS 1.8 narrowing worked cross function expression/lambda expression, which was not correct; the compiler has no guarantee that forEach will call the lambda expression immediately, so making assumptions about the type of object is not correct. you can find more information about this in #7719 (comment)

if object was a const, the compiler will narrow correctly the type through the function expressions, as it knows it can not be modified once captured. so:

function handle(obj: Struct | Struct []) {
    const object = obj;  // capture it in a const
    if (Array.isArray(object)) {
        for (let single of object) {
            handle(single)
        }
    } else {
        Object.keys(object).forEach(key => {
            let value = object[key];
            if (Array.isArray(value)) {
                console.log(value.length);
            } else {
                console.log(object.type);
            }
        })
    }
}

You would probably like to get the same behavior for function arguments, except that there is no way to tell the compiler that these are "constant" like variables or "readonly" like members. We have recently discussed allowing a const modifier on function arguments, that will allow the compiler to make these assumptions without giving up on safety. you can find more about this in #8908.

@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
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

3 participants