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

Partial type is not narrowed even when its only property is successfully narrowed as being defined #29827

Closed
anandthakker opened this issue Feb 8, 2019 · 5 comments

Comments

@anandthakker
Copy link

TypeScript Version: 3.4.0-dev.20190208

Search Terms: Partial narrow

Code

type T = { content: string[] };

declare function getData(): Partial<T>;

const partial = getData();

let data: T;
if (partial.content) {
    data = partial; // causes an error (unexpected)
    data = { content: partial.content }; // does not cause an error (expected)
}

Expected behavior: No errors. Specifically, the if (partial.content) check narrows partial from Partial<T> to T.

Actual behavior: The following error:

bug.ts:9:5 - error TS2322: Type 'Partial<T>' is not assignable to type 'T'.
  Property 'content' is optional in type 'Partial<T>' but required in type 'T'.

9     data = partial;
      ~~~~


Found 1 error.

Playground Link: playground link

Related Issues: #29496

@dragomirtitian
Copy link
Contributor

The check will only narrow the content field. partial is in no way a union so narrowing will be done.

This for example works:

type T = { content: string[] };
declare function getData(): T | { content?: undefined};

const partial = getData();

let data: T;
if (partial.content) {
    data = partial; // ok
    data = { content: partial.content }; //also ok
}

I am pretty sure this is the current designed behavior, but I have seen similar questions come up on SO. The basic expectation being that if a field is narrowed, then, on assignment of the object, that field narrowing is taken into account for compatibility.

This also fails, perhaps better illustrating what I am trying to explain:

type T = { content: "A" | "B" };
declare let data: T;
if (data.content == "A") {
    let justA : { content: "A" } = data // also an error
}

This information should already be available and the assignment seems safe.

@jack-williams
Copy link
Collaborator

The latter example isn't safe because of aliasing, which is non-trivial to track.

type T = { content: "A" | "B" };
declare let data: T;
if (data.content == "A") {
    let justA : { content: "A" } = data;
    data = { content: "B" };
}

A type { content: "A" | "B"} gives you permission to read and write "A" and "B". Witnessing an "A" gives you a snapshot of information about the current state of the field, but it cannot affect the permissions you, or anyone else, holds on the object.

@dragomirtitian
Copy link
Contributor

@jack-williams Fair enough. This doesn't work either and it feels like it should:

type T = { content: "A" | "B" };
declare let data: T;
if (data.content == "A") {
    let justA : { content: "A" } = { ...data }; // still an err
}

Here we are copying the data to a new object so changing the value should not be an issue.

@anandthakker
Copy link
Author

Thanks @jack-williams, that makes it pretty clear why the behavior I was expecting can't be possible... it seems so obvious in retrospect!

type T = { content: string[] };
declare let partial: Partial<T>;
if (partial.content) {
    let data: T = partial;
    delete partial.content; // `data` is now mistyped
}

@jack-williams
Copy link
Collaborator

jack-williams commented Feb 8, 2019

@dragomirtitian Yep, operationally that should work. I think type-checking that is non-trivial under the current infrastructure. You would need to synthesize property access nodes to get the narrowed type of every field and then reconstruct that on the spread, I think.

I'm also not sure what would happen in the case of generics:

function foo<X extends T>(x: X): X {
    if (x.content === "A") {
        return { ...x };
    }
    return x;
}

Doing anything funny with the spread might break existing code that expects the spread to return the exact same type back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants
@jack-williams @anandthakker @dragomirtitian and others