Allowed DeepOmit to support partial

[Joja81]

Linked to issue: #12391

Proposed solution to bug in DeepOmit function.

When deep commit is used on a type with optional keys, the optional key(s) is removed and replaced with the orignial type union undefined.

This leads to situations like:

type OptionalString = {
  maybeString?: string;
  __typename: 'X-type';
};

becoming

{
    maybeString: string | undefined;
}

when DeepOmit is used to remove ___typename

To solve this I've added logic to deal with Optional and Required keys seperately.

[apollo-cla]

@Joja81: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

[netlify]

👷 Deploy request for apollo-client-docs pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	6892462

[changeset-bot]

🦋 Changeset detected

Latest commit: 6892462

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@apollo/client	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[svc-apollo-docs]

✅ Docs preview has no changes

The preview was not built because there were no changes.

Build ID: 944a5d9f7313135559e58ba6

[jerelmiller]

After looking at this some more, it looks like the Exclude<keyof T, K> is to blame for the optional getting removed from the output type. If we use switch to use key remapping instead, this seems to fix the issue:

- [P in Exclude<keyof T, K>]: T[P] extends infer TP ?
+ [P in keyof T as P extends K ? never : P]: T[P] extends infer TP ?

Here is a test I did with only this change:

type C = {
  a: boolean;
  b?: number;
  c?: string;
  d?: { a: string; c: boolean; e: { a: string; c: boolean } };
};
type D = DeepOmit<C, "c">;
//   ^?
// type D = {
//   a: boolean;
//   b?: number | undefined;
//   d?: {
//     a: string;
//     e: {
//       a: string;
//     };
//   } | undefined;
// }

Can we simplify to this instead?

[Joja81]

Ah cool. Yes that's much neater!

I've just updated my PR with your version.

Sorry, I'm fairly new to complicated TypeScript stuff but felt like trying to find a solution.

[jerelmiller]

No problem at all! That's what code reviews are for 🙂

[jerelmiller]

Great catch on the optional getting removed. Let's use the simpler fix though if we can 🙂

[jerelmiller]

Since this technically touches two publicly exported utilities, last thing I'll ask for is to add a changeset here. You can generate a changeset with npx changeset and fill out the change. I'd say something along the lines of:

Fixes an issue where the DeepOmit type would turn optional properties into required properties. This should only affect you if you were using the omitDeep or stripTypename utilities exported by Apollo Client.

By all means, make it your own, but I think mentioning which utilities are affected help. Adding a changeset will make sure we can publish a version bump with this change.

I'll get this approved and merged after that 🎉

[Joja81]

No worries! I've just added the changest. Thanks for your help!

[jerelmiller]

Thanks a bunch!