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

Function return typing not working well when depth is higher than 1 #52893

Closed
carlo23666 opened this issue Feb 21, 2023 · 8 comments
Closed

Function return typing not working well when depth is higher than 1 #52893

carlo23666 opened this issue Feb 21, 2023 · 8 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@carlo23666
Copy link

carlo23666 commented Feb 21, 2023

Bug Report

πŸ”Ž Search Terms

Function Return, Optionality, Optional

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about Return Optionality

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

class Example {
    main(param: boolean) {
        const result = this.getMethodCase1(param) // result.c is not visible here (Bad)
        //    ^?
        const result2 = this.getMethodCase2(param) // result.c is optional here (OK)
        //    ^?
        const result3 = this.getMethodCase3(param) // result.c is optional here (OK)
        //    ^?
    }

    getMethodCase1(param: boolean) {
        if(param) {
            return this.method1()
        }

        if(!param) {
            return this.method2()
        }

        throw new Error('Not controlled')
    }


    getMethodCase2(param: boolean) {
        if(param) {
            const result = this.method1()
            return {...result}
        }

        if(!param) {
            const result = this.method2()
            return {...result}
        }

        throw new Error('Not controlled')
    }

    getMethodCase3(param: boolean) {
        if(param) {
            return {b:1, c:2}
        }

        if(!param) {
            return {b: 1}
        }

        throw new Error('Not controlled')
    }


    method1() {
        return {b: 1, c: 2}
    }

    method2() {
        return {b: 1}
    }
}

πŸ™ Actual behavior

Result 1 doesn't have optional attribute c

πŸ™‚ Expected behavior

All results should have optional attribute c

@whzx5byb
Copy link

Duplicate of #46449

@Andarist
Copy link
Contributor

The problem is related to the linked issue (as the problem is related to the union reduction) - but it still feels to me that those 3 code samples reduce differently.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Feb 21, 2023
@RyanCavanaugh
Copy link
Member

Optional inference only happens when all input candidates are fresh object literals. See #19513

@Andarist
Copy link
Contributor

I was expecting that freshness is at play here but the thing that still surprises me is that getMethodCase1 and getMethodCase2. It's only the getMethodCase3 that is explained by the freshness (I think?).

@RyanCavanaugh
Copy link
Member

I guess you could argue that literals with spreads shouldn't be considered fresh, since the sorts of guarantees that literals provide don't usually apply when a spread is present.

@Andarist
Copy link
Contributor

I think that even with spreads the object literal should be treated as fresh if all of its "spread sources" are object literals. This might matter for conditional object creation like in those patterns:

const obj1 = {
  a: 1,
  ...(bool ? { b: 2 } : { c: 3 })
}

const obj2 = {
  a: 1,
  ...(bool && { b: 2 })
}

That being said, I'm still wondering what the rules are here :P If I understand you correctly then you are saying that the spread case is actually fresh today - but then, what is the rule that makes the getMethodCase2 and getMethodCase3 different? My guess is that in the non-spread case, we truly see object literals of "undetermined" types - so the compiler figures them out in place and this involves "optional properties inference" from #19513 . The spread case already deals with inferred return types from other methods - but since those spreads are in object literals the compiler assumes freshness there and that makes it to infer unions (but the union members are not subject to any extra "unification" - like in the "undetermined" case)

@RyanCavanaugh
Copy link
Member

Sorry, I got something backwards above - the one with spreads is working correctly.

The difference between 2 and 3 is just whether subtype reduction has happened yet. I'm not really sure why one happens and not the other; subtype reduction is allowed to happen anywhere, but is not required to do so at any time, so either behavior is considered correct from our perspective.

@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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

5 participants