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

Using Omit on an Object Union applies on the merged type instead of the union members #57085

Closed
james-yeoman opened this issue Jan 18, 2024 · 6 comments
Labels
Duplicate An existing issue was already created

Comments

@james-yeoman
Copy link

🔎 Search Terms

"omit", "omit union"

#54371 is related to this issue, but is specific to tuples. This issue applies to objects in general

🕗 Version & Regression Information

  • This is the behavior in every version I tried, and there are no applicable FAQ pages

⏯ Playground Link

https://www.typescriptlang.org/play?#code/C4TwDgpgBAMhBmwoF4oG8BQUqkgLigCIAbBYQgbiygGNiB7AZwgIAoBKFAPigDd6AlgBMq2RgFcARgFsBwNmACGIBoqEFGwAE4CAdgHNOyHgAUt9WcwA8-YVyoBfDBlzQASgP0ALJKkzZXAkIdb3JRWgZmNiMeWxFqCRk5BWVVdShdcWlJCC0YqDMLAWs4+2p6EL1FYg1tPX1HZ1coAFVdAXpdFFgyKAAfKA9QqgwaTs0oeHp6AjaOrr9qQKIQn0IAGnLK3WqgqfpJRS0N6jomFigObnQHTbEpWXkoJRV6NWvCywgAOi0IRnoxF4EA41AA9GCAHoAfgwDhGzQA8o9gBAhN1kXIrHNOusiGdmIQyqNxkhDloCJjgKj0YsAuALsFPGs7lAKp4qjUiPtySd7kkni80h9zF9fv9AcDQdgITC4c4IVAfMAwIw8BD9HIvFJvmNpGDZDRzADEGCACoMgDKRoEYGAYOKEn+YIArAAWADMAHYAIwAYkd4ggeukEF0wAAtD6XQA2AAcca9AAY4x6Yy4GVAABIWXNaMBeAQ0KlWM14gDSPD8UAA2iYoHooABrCAgejwKBmqCKRgFKAQAAeqN0Ql75ag0IyEGBWigBBMAF0CGa6wuoPCmpmAHIQADuVJp3Rz0jzBaLJZxujxhAJECJIzGugmhwAXgQd-uUWjuv4cAygqs5Csuymo7FyhA8kcfJQIkjwEEKby0qYorFD8fwAkCILsOCUKwvKQA

💻 Code

type Left = {
  type: "left";
  close: () => void;
  submit: (payload: string) => Promise<void>;
}

type Right = {
  type: "right";
  close: () => void;
  submit: (payload: number) => Promise<void>;
  original: string;
}

type Union = Left | Right;

const foo: Union = {
  type: "right",
  original: "foobar",
  close: () => {},
  submit: payload => Promise.resolve()
  //^? - payload is typed as `number`
};

type Omitted = Omit<Union, "close">;

const bar: Omitted = {
  type: "right",
  original: "foobar",
  submit: payload => Promise.resolve()
  //^? - a union of the two submit types
}

// https://github.com/microsoft/TypeScript/issues/54371#issuecomment-1568870836
type HomomorphicOmit<T, K> = { [P in keyof T as P extends K ? never : P]: T[P] };

type NewOmitted = HomomorphicOmit<Union, "close">;

const baz: NewOmitted = {
  type: "right",
  original: "foobar",
  submit: payload => Promise.resolve()
  //^? - payload is typed as `number`
}

🙁 Actual behavior

Omit consolidates the union into a single type

🙂 Expected behavior

I expected Omit to operate on all members of a union

Additional information about the issue

In #54371 (comment) Ryan mentions a modified version of Omit, that I've verified in my Playground link works for object unions.

type MappedOmit<T, K> = { [P in keyof T as P extends K ? never : P]: T[P] };
@MartinJohns
Copy link
Contributor

This is working as intended. See also #54451, and many many many other issues (search for Omit in:title).

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Jan 18, 2024
@james-yeoman
Copy link
Author

Ah, my bad. Guess I should've been more diligent with my issue searching :/

Still not too keen on the push into userland, but #53169 was mentioned in #54451 and the points outlined make the MappedOmit userland definition an easier pill to swallow.

Would it be possible to either add to the JSDoc of Omit to mention the incompatibility with union types, add an entry to the FAQ about the incompatibility with union types, or both? At least until a solution can be found for the problems with Omit?

@fatcerberus
Copy link

Yeah, I don't think the TS team really wanted to push it into userland either, but it was kind of a catch-22: Omit already exists but with an inconvenient definition, we can't change it without breaking code in the wild, and since there are multiple competing userland definitions of Omit, adding a second variant to the compiler builtins is just opening the floodgates for people to ask for built-in support for other variants. Especially since (in this hypothetical situation) they've already tacitly admitted that the "canonical" Omit is inadequate by implementing MappedOmit.

@typescript-bot
Copy link
Collaborator

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

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 22, 2024
@james-yeoman
Copy link
Author

we can't change it without breaking code in the wild

@fatcerberus Is this not what Semver Major versions are for? At the very least, it'd be great to get some sort of an official commitment to fix Omit in Typescript v6

@MartinJohns
Copy link
Contributor

TypeScript does not follow SemVer (for good reasons). And Omit is such a widely used type, I think even announced ahead and with a new major version it would be a very very bad idea to break it.

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

5 participants