Skip to content

Conditional spread results in weird union type, with duplicates #41386

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
DanielRosenwasser opened this issue Nov 3, 2020 · 8 comments · Fixed by #42233
Closed

Conditional spread results in weird union type, with duplicates #41386

DanielRosenwasser opened this issue Nov 3, 2020 · 8 comments · Fixed by #42233
Assignees
Labels
Experience Enhancement Noncontroversial enhancements Fix Available A PR has been opened for this issue Suggestion An idea for TypeScript

Comments

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Nov 3, 2020

interface Animal {
    name: string;
    kind: string;
    age: number;
    location: string;
    owner: object;
}

function clonePet(pet: Animal, fullCopy?: boolean) {
    return {
        name: pet.name,
        kind: pet.kind,
        ...(fullCopy && pet),
    }
}

Expected: clonePet returns type { name: string, kind: string, age?: number, location?: string, owner?: object }

Actual: clonePet returns the following type:

{
    name: string;
    kind: string;
} | {
    name: string;
    kind: string;
} | {
    name: string;
    kind: string;
    age: number;
    location: string;
    owner: object;
}
@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented Nov 3, 2020

Any clue if this is working as intended @ahejlsberg @weswigham @amcasey?

@DanielRosenwasser
Copy link
Member Author

Another example that @sandersn had come up with

interface Animal {
    name: string;
    owner?: string;
}
function billOwner(pet: Animal) {
    return {
        ...(pet.owner && pet),
        paid: false
    }
}

@ahejlsberg
Copy link
Member

I'm a bit surprised the same type is in there twice, but otherwise it's an effect of us not performing union subtype reduction in object literal spreads.

@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented Nov 3, 2020

Right - I actually thought that that was where the slowdown originally came from: creating every combination of types of spreads, followed by subtype reduction on extremely similar types. I guess it was just creating each possibility, which I think is still exponential.

@weswigham
Copy link
Member

The multiple-of-the-same-object-type thing is something that's always been an unfortunate possibility in a spread. That example you have there gives the same output in older TS, too.

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Nov 3, 2020

The second output seems more correct to me (ignoring the duplicate union element).

Because I doubt this is desired,

const clone = clonePet(pet, true)
clone.location = undefined //probably not desired?

I just read the 4.1 RC release notes. Didn't realize the optional props are now expected behaviour.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Nov 11, 2020
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 4.1.2 milestone Nov 11, 2020
@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented Nov 11, 2020

The duplicate is just "unfortunate" but it's a distraction. I think this behavior is inconsistent otherwise because the real thing I'm reporting is that we're not getting optional properties, and I can't coherently explain this behavior.

@weswigham
Copy link
Member

weswigham commented Nov 11, 2020

The duplicate is directly the cause of the behavior (which also exists in older TS versions provided you use single-property additions). This is because our spread object-combining logic only permits combining exactly two types, so if you have three types (even if two are identical), we skip the whole simplification process (which is the only thing that produces optional properties) and do the big union-all-the-possibilities plan.

@weswigham weswigham added Experience Enhancement Noncontroversial enhancements Fix Available A PR has been opened for this issue Suggestion An idea for TypeScript and removed Needs Investigation This issue needs a team member to investigate its status. labels Jan 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experience Enhancement Noncontroversial enhancements Fix Available A PR has been opened for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants