Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Cli update and typescript conversion wip (new branch name) #69
base: master
Are you sure you want to change the base?
Cli update and typescript conversion wip (new branch name) #69
Changes from 7 commits
8499c72
6bf24f9
d29ca8e
cbd6dd6
f2aa8bd
d6056d0
200d78d
5e6d029
66bfde6
111683f
c817261
0e57e4b
040beae
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
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 probably avoid all the
// @ts-expect-error
directives by making this anObject.keys(svConfig).filter(() => /* etc */)
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.
Unfortunately, I don't believe
.filter()
acts as a type guard. There is a library,ts-reset
, that does this, but it's not recommended for libraries because it pollutes the global scope.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.
I agree, let's not pollute the global scope 😅
What I mean is:
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.
I understand your meaning, but what I'm saying is that - despite how one might expect .filter() and Object.keys() to work - the type of
key
remains asstring
throughout that pipeline. It is never narrowed to the union of literal strings that are keys of svConfig, which is what is needed to remove the type errors.You aren't the only one to expect this behavior, though. I certainly did, as did many others. That's why I bring up
ts-reset
- it was written with the purpose of making some less intuitive behaviors act how you would expect, and I think this is one of the things it addresses (again, not advocating for it here lol).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.
Wow - ignore me. I feel dumb now! I think I understand what you mean. I've refactored like so:
That about right?