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

Constraints of SetOptional may make you write bad code #509

Closed
kasperpeulen opened this issue Nov 14, 2022 · 2 comments
Closed

Constraints of SetOptional may make you write bad code #509

kasperpeulen opened this issue Nov 14, 2022 · 2 comments

Comments

@kasperpeulen
Copy link

We have some code like this.

type OptionalDefaults<T, Default extends Partial<T>> = SetOptional<T, keyof Default>;

However, SetOptional would complain, as it is defined like this:

export type SetOptional<BaseType, Keys extends keyof BaseType> =

Basically, keyof Default may contain different keys than T. So anyway, I started writing this code to please the SetOptional signature (although it doesn't make any difference in practice):

type OptionalDefaults<T, Default extends Partial<T>> = SetOptional<T, Extract<keyof T, keyof Default>>;

Now I actually found this code to lead to bugs.

interface Props {
  a: string;
  b: string;
  [key: string]: unknown;
}

const defaults = { a: 'bla' };

const props: OptionalDefaults<Props, typeof defaults> = {
  // complains that a is a required key
  b: 'bla',
};

My gut feeling is that the constraint on key of SetOptional is not really worth it.
Or that we need at least also a variant without the constraint.

@papb
Copy link
Contributor

papb commented Nov 14, 2022

Hello! Actually you have a mistake in your implementation.

Basically, keyof Default may contain different keys than T.

To be more precise: Default may contain more keys than the ones in T.

Therefore, instead of Extract<keyof T, keyof Default>, you should have used Extract<keyof Default, keyof T> (or keyof T & keyof Default).

That said, maybe your OptionalDefaults idea could be even included in type-fest!

Playground

@kasperpeulen
Copy link
Author

Oh, that actually makes sense! Thanks for the detailed playground, which makes it all a lot clearer.

So you can always silence the constraint of SetOptional by adding & keyof T:

SetOptional<T, Whatever & keyof T>;

So in that case, adding an unconstraint SetOptional may not be necessary indeed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants