-
Notifications
You must be signed in to change notification settings - Fork 9
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
Implement config file support #223
Conversation
eb2c917
to
63fa0ae
Compare
d50c8fc
to
aa54b4e
Compare
// TODO: use TypeScript 4.9 satisfies keyword ([key in OPTION_TYPE]...) to ensure all options are included without losing type information. | ||
export const OPTION_DEFAULTS = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use Record
instead. You will get type errors if you the values aren't in sync with OPTION_TYPE
// TODO: use TypeScript 4.9 satisfies keyword ([key in OPTION_TYPE]...) to ensure all options are included without losing type information. | |
export const OPTION_DEFAULTS = { | |
export const OPTION_DEFAULTS: Record<OPTION_TYPE, string> = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion, just tried it and it looks like it's equivalent to the type I was originally trying to use { [key in OPTION_TYPE]: string | string[] | boolean | undefined }
and has the same problem. While it does correctly ensure all options are included in the object, it loses type information since accessing a single option from the object now could have any of those possible types instead of what should be a known type. The new satisfies feature in TypeScript 4.9 looks like it will solve this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate on why you need the exact type for each of them?
It doesn't seem like it gives extra value 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's required actually. This object stores default values for each config option. These default values are used throughout the codebase. When they have inexact types (i.e. could be one of several types), they won't be able to be used easily due to type mismatches. For example, for an option with an array default value, we won't be able to handle/iterate the default value properly if the type says it could also be a string, boolean, etc.
Fixes #95.
README documentation located here.
TODO:
--config-emoji