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

RequireAtLeastOne: Make other given keys optional #142

Merged
merged 1 commit into from
Nov 16, 2020

Conversation

ulken
Copy link
Contributor

@ulken ulken commented Nov 11, 2020

For each mapped type with given required key, make all other given keys optional.

Fixes #59

For each mapped type with given required key,
make all other given keys optional.
@ulken ulken changed the title RequireAtLeastOne: Make other keys optional RequireAtLeastOne: Make other given keys optional Nov 12, 2020
@sindresorhus
Copy link
Owner

From the description:

The remaining keys are kept as is.

That's not really true anymore, right?

@ulken
Copy link
Contributor Author

ulken commented Nov 12, 2020

That's not really true anymore, right?

I get the confusion, but I believe it still holds. It gets a little hairy with the terminology, but I'll try to explain the differences, as I understand it:

  • Remaining keys: keys not given as parameters, i.e. keys of ObjectType not present in KeysType. Note: if no keys are given, then KeysType includes all keys of ObjectType.
  • Other given keys: all keys of KeysType except the one we're currently mapping over.

Example

type SystemMessages = {
  default: string;
  
  macos?: string;
  linux?: string;
  windows?: string;

  optional?: string;
}

type ValidMessages = RequireAtLeastOne<
  SystemMessages,
  "macos" | "linux" | "windows"
>

In this case, default and optional are remaining keys and they are left intact/as is.

When we're creating the type where "macos" is required, "linux" and "windows" are the other given keys.

Does that make sense?

The change this PR introduces is making the other given keys optional.

@ulken
Copy link
Contributor Author

ulken commented Nov 15, 2020

@sindresorhus do you feel me?

@sindresorhus
Copy link
Owner

That makes sense. Thanks for explaining.

@sindresorhus sindresorhus merged commit 6110607 into sindresorhus:master Nov 16, 2020
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

Successfully merging this pull request may close these issues.

RequireAtLeaseOne issues
2 participants