Skip to content

User-defined type guard behaves differently inside Array.filter #30240

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
OliverJAsh opened this issue Mar 6, 2019 · 8 comments
Closed

User-defined type guard behaves differently inside Array.filter #30240

OliverJAsh opened this issue Mar 6, 2019 · 8 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@OliverJAsh
Copy link
Contributor

OliverJAsh commented Mar 6, 2019

TypeScript Version: 3.3.3333

Search Terms: user defined type guard array filter narrow

Code

type BasicUserData = { name: string };
type UserStats = { stats: {} };

type User = BasicUserData | UserStats | (BasicUserData & UserStats);

declare const checkHasUserStats: (user: User) => user is UserStats;

declare const user: BasicUserData | (BasicUserData & UserStats);

// no type error, good
user.name;
// type error, good
user.stats;

if (checkHasUserStats(user)) {
    // Expected and actual `typeof user`: BasicUserData & UserStats

    // no type error, good
    user.name;
    // no type error, good
    user.stats;
}

declare const users: Array<BasicUserData | (BasicUserData & UserStats)>;

users.filter(checkHasUserStats).map(user => {
    // Expected `typeof user`: BasicUserData & UserStats
    // Actual: BasicUserData

    // no type error, good
    user.name;
    // unexpected type error, bad (!)
    user.stats;
});
@jack-williams
Copy link
Collaborator

Related: #29501

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Mar 7, 2019
@RyanCavanaugh
Copy link
Member

The problem is here: BasicUserData | (BasicUserData & UserStats) - this is a subtype-reducing union that is going to evaporate if poked.

Declaring this as

declare const users: Array<BasicUserData & Partial<UserStats>>;

causes the expected behavior.

@OliverJAsh
Copy link
Contributor Author

this is a subtype-reducing union that is going to evaporate if poked.

Can you elaborate on why? It seems there's a caveat in the type system here that I need to be aware of, so any information you can share would be appreciated!

Also, I'm still not sure I understand why this behaviour isn't consistent between filter and if usages of the type guard.

Array<BasicUserData & Partial<UserStats>>;

IIUC, those types aren't entirely accurate, as Partial allows any of the properties in UserStats to be undefined, whereas I want to say "either no A or all of A".

In my contrived example this doesn't matter much because UserStats has a single property, but that might not always be true. (Indeed it isn't in my real world example where I initially discovered this issue.)

@jack-williams
Copy link
Collaborator

jack-williams commented Mar 7, 2019

The issue here is that your filter predicate is not a valid type guard when instantiated. The refinement of a predicate needs to be a assignable to the input. You can see the issue here:

type BasicUserData = { name: string };
type UserStats = { stats: {} };
declare const users: Array<BasicUserData | (BasicUserData & UserStats)>;
declare const checkHasUserStats: (user: typeof users[number]) => user is UserStats; // error

The type guard does not typecheck if it's written as it would be used in the filter.

TLDR: UserStats is not assignable to BasicUserData | (BasicUserData & UserStats), so the type guard overload is not selected by filter.

Using @RyanCavanaugh's solution the type guard overload is still not selected, but the type BasicUserData & Partial<UserStats> is more permissive and lets you assign to property stats.

Writing your typeguard like this will give the desired behaviour I think.

declare const checkHasUserStats: <T extends User>(user: T) => user is T & UserStats;

@OliverJAsh
Copy link
Contributor Author

OliverJAsh commented Mar 7, 2019

@jack-williams That explains a lot I think, thank you.

One question I have:

filter predicate is not a valid type guard when instantiated

I instantiate the type guard with exactly the same type when using if, and it seems to work OK there—so I'm still not entirely clear why this behaviour isn't consistent between if and filter. 🤔

@jack-williams
Copy link
Collaborator

jack-williams commented Mar 7, 2019

The difference (I believe) is that in the if you deal with the concrete instantiation of the type guard immediately.

EDIT: My previous explanation was terrible. The input type to the type guard is fixed by the array type, which is BasicUserData | (BasicUserData & UserStats). Once that is fixed it will only check that that arguments are assignable to that type. What you really need is reasoning that lets you restrict the expected type of the argument, but there is no facility to do that I think.

@OliverJAsh
Copy link
Contributor Author

@jack-williams Do you think any suggestions or bugs need to be filed as a result of what you're describing? I'm certain the behaviour I described is going to confuse users, especially when the behaviour differs between if and filter.

@OliverJAsh
Copy link
Contributor Author

OliverJAsh commented Nov 19, 2019

@jack-williams

Writing your typeguard like this will give the desired behaviour I think.

declare const checkHasUserStats: <T extends User>(user: T) => user is T & UserStats;

That achieves the desired behaviour, however it creates another problem: if we use a constrained generic then it seems we lose the ability to validate the type guard (our workaround for #29980):

const checkHasUserStats = (user: User): user is UserStats => {
    if ('stats' in user) {
        // No error, good! :-)
        const _test: UserStats = user;
        return true;
    } else {
        return false;
    }
};
const checkHasUserStatsGeneric = <T extends User>(user: T): user is T & UserStats => {
    if ('stats' in user) {
        // Error, bad! :-(
        const _test: UserStats = user;
        return true;
    } else {
        return false;
    }
};

Do you have any suggestions for that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

3 participants