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

Conditional on property not propagated #59125

Closed
Sainan opened this issue Jul 3, 2024 · 10 comments
Closed

Conditional on property not propagated #59125

Sainan opened this issue Jul 3, 2024 · 10 comments
Labels
Duplicate An existing issue was already created

Comments

@Sainan
Copy link

Sainan commented Jul 3, 2024

πŸ”Ž Search Terms

property undefined propagation

πŸ•— Version & Regression Information

Not a regression as far as I can tell.

⏯ Playground Link

https://www.typescriptlang.org/play/?ts=5.6.0-dev.20240702#code/JYOwLgpgTgZghgYwgAgJICUIHc5QCbIDeAUMmcmAJ4AOEAXMgM5hSgDmA3KedVAPYAjOAOAAbYFQD8DEAFcAtgOhdyyKLgmVpTFuy4BfYsVCRYiFBhBtMjWaLBFuZKrQbNWVlT35CR4qjIKSlAGRgh8IMxq2Lh4AAp8fKIAKnw2dmAJScgAvMgAFFAx+IwMGMV4ANoAugCUZehW6fY1uQB8jqoA9F1oeBBwoqKUADQUABbAjMhYfHYEs1AA1nROyD1FYLJQINE4JQB0MGKm+QAe7chnB7yCwmKayACEOXmyIP3HIBB4tV5kPWQACFZGAxhAAG4QXagCYod78IY-ZAIOCMCBjCTIUDhKBFBBgYbIaD8KClNbhSIOaiJUQNJoQWwtaq5ZA1f7IGB8KAFSlRIr7Ah8GB7WKMWqdVTkYAiwoVG4+e7+SjPV7Id6fUA-CUkKV65A0pI3WSMcZywV-NaqQzWtabba7Q2iAxcIA

πŸ’» Code

interface IReward {
    type: string;
    probability?: number;
    rarity?: string;
}

interface IRngResult {
    type: string;
    probability: number;
}

const rewardPoolToResultPool = (rewards: IReward[]): IRngResult[] => {
    // Ideally, this would work:
    //return rewards.filter(x => x.probability !== undefined);
    // But, even in the unrolled case, it incorrectly errors:
    const pool: IRngResult[] = [];
    for (const reward of rewards) {
        if (reward.probability !== undefined) {
            pool.push(reward);
        }
    }
    return pool;
};

πŸ™ Actual behavior

It complains that the probability property may be undefined, even tho it was explicitly checked for that.

πŸ™‚ Expected behavior

It accepts the push as valid.

Additional information about the issue

No response

@yaKsirhC
Copy link

yaKsirhC commented Jul 3, 2024

You can convert the type on your own pool.push(reward as IRngResult);

@Sainan
Copy link
Author

Sainan commented Jul 3, 2024

I know that casting is an option, but it's generally more desirable to prove than promise something to a compiler.

@MartinJohns
Copy link
Contributor

Duplicate of #42384.

@Sainan
Copy link
Author

Sainan commented Jul 3, 2024

I don't think these are quite the same issue, since in that case it's about inferring which variant of a union type is active in the given branch, whereas in this case, there is no union type, only an object and its properties.

@MartinJohns
Copy link
Contributor

It's about narrowing a property propagating to the parent type. You expect by checking the property probability that your IRewad type suddenly is compatible with IRngResult, but that's not how narrowing works. Checking the property does not change the parent type.

@Sainan
Copy link
Author

Sainan commented Jul 3, 2024

Inheritance is involved nowhere here. Please don't speak if you have nothing of value to add.

@MartinJohns
Copy link
Contributor

I did not mention inheritance anywhere. Please don't say I have nothing of value to add when you don't understand the problem at hand. πŸ™ƒ

Checking a property like probability can be used to narrow the type of the object (what I referred to as parent) when it's a union type. It does not change the type of the parent from { probability?: number } to { probability: number }, that's not supported.

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.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Jul 9, 2024
@RyanCavanaugh
Copy link
Member

The duplicate link is correct.

Please don't speak if you have nothing of value to add

Not an acceptable way to interact with someone here. Please refer to the code of conduct:

Disagreements, both social and technical, are useful learning opportunities. Seek to understand the other viewpoints and resolve differences constructively.

@Sainan
Copy link
Author

Sainan commented Jul 9, 2024

I still fail to see how that issue is the same, but okay.

Also, I think "Please don't speak if you have nothing of value to add." is good advice sometimes, especially to eager noobies, which this person appeared to me as.

@MartinJohns
Copy link
Contributor

Yep, it's my first day using TypeScript, I'm such a noobie.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants