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

RequiredOptions.ts duplicates PickRequired.ts #109

Open
Tracked by #129
pixelzoom opened this issue Mar 3, 2022 · 2 comments
Open
Tracked by #129

RequiredOptions.ts duplicates PickRequired.ts #109

pixelzoom opened this issue Mar 3, 2022 · 2 comments
Assignees

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Mar 3, 2022

RequiredOption.ts looks like it duplicates PickRequired.ts. We had a long discussion about PickRequired.ts (and its name) in https://github.com/phetsims/phet-io/issues/1843, and decided on PickRequired.ts and PickOptional.ts.

Recommended to delete RequiredOption.ts.

pixelzoom added a commit that referenced this issue Mar 3, 2022
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Mar 3, 2022

We discussed this in 3/3/2022 dev meeting, and they are in fact different. I updated the doc in RequiredOption.ts to clarify.

But the name is still misleading. It returns the type of an option, and it doesn't even need to be a required option (as demonstrated by the example in the doc.) Should it be renamed to something like TypeOfOption or OptionType?

@pixelzoom
Copy link
Contributor Author

It looks like there's another problem with RequiredOption.ts. You can list multiple option names and it doesn't complain. Example:

type SelfOptions = {
  name: string
  age: number
};

// No complaint about this?
something: RequiredOption<SelfOptions, 'name' | 'age'>;

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