Skip to content

Type narrowing on a property does not narrow the type of the parent #50651

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
Fryuni opened this issue Sep 6, 2022 · 5 comments
Closed

Type narrowing on a property does not narrow the type of the parent #50651

Fryuni opened this issue Sep 6, 2022 · 5 comments

Comments

@Fryuni
Copy link

Fryuni commented Sep 6, 2022

Bug Report

🔎 Search Terms

type narrowing
narrowing property
narrowing nested

🕗 Version & Regression Information

When did you start seeing this bug occur?

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about narrowing, guards and common bugs

Locally I noticed this on versions 4.7.4 and 4.8.2.
It also happens on every version available on the playground, from 3.3.3 up to nightly (4.9.0-dev.20220905).

⏯ Playground Link

Playground link with relevant code

💻 Code

type Foo = {
    bar: string,
    baz?: string,
};

type FooWithBaz = Foo & {
    baz: NonNullable<Foo['baz']>,
};

function takesFoo(f: Foo) {
    if (f.baz === undefined) {
        return;
    }

    // This should work
    takesFooWithBaz(f);

    // This works, but creates a new object
    takesFooWithBaz({
        ...f,
        baz: f.baz,
    });
}

function takesFooWithBaz(_: FooWithBaz) {}

🙁 Actual behavior

Type narrowing should happen on the property and the parent variable, allowing f to be passed to takesFooWithBaz

🙂 Expected behavior

The property f.baz is narrowed to not be undefined, but the property baz of the type of f remains as string | undefined.

@MartinJohns
Copy link
Contributor

Quoting @jack-williams:

Type guards do not propagate type narrowings to parent objects. The narrowing is only applied upon access of the narrowed property which is why the destructing function works, but the reference function does not. Narrowing the parent would involve synthesizing new types which would be expensive. More detail in this comment.

Used search terms: type guard property parent

Duplicate of #42384.

@Fryuni
Copy link
Author

Fryuni commented Sep 6, 2022

Looks like it's not a bug then, thanks @MartinJohns

@Fryuni Fryuni closed this as not planned Won't fix, can't repro, duplicate, stale Sep 6, 2022
@fatcerberus
Copy link

Yeah, if you need to narrow-by-property, you should use a discriminated union. Those exist as a trade-off, if this kind of thing worked in the general case, you wouldn't need them 😉

https://www.typescriptlang.org/docs/handbook/typescript-in-5-minutes-func.html#discriminated-unions
^ in case you're not familiar with them.

@Fryuni
Copy link
Author

Fryuni commented Sep 6, 2022

My use case here was for a single type that has an optional property, so there is no need to make a discriminated union using a different property.

For a single property this can actually be solved somewhat easily, but I thought this was supposed to work out of the box.

@ericbf
Copy link
Contributor

ericbf commented Mar 20, 2023

Though not a bug, seems like a valid request still. Related to #42384, but slightly different. In the other issue, it’s talking about inferring (discriminating) the type of the parent based on properties of the parent, while this one is talking about narrowing the type of the parent's properties.

Seems a perfectly valid use case that should work out of the box:

type Nest = {
    nested?: {}
}

function doSomething(n: Nest & { nested: object }) {}

declare const n: Nest

if (n.nested) {
    doSomething(n) // Error
    doSomething({ ...n }) // Error
    doSomething({ nested: n.nested }) // Works, but loses object identity
}

Playground link

It seems obvious that inside the if statement, n.nested is defined, so the call to doSomething should not be flagged as an error.

Whether this is working as intended or not, there’s obviously a gap there that at least can be explored.

Should this issue be reopened? Another issue created to track this as a feature request instead?

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