Add type checking to lang objects #1462
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
camden11
requested review from
brandenrodgers,
joe-yeager and
kemmerle
as code owners
joe-yeager
reviewed
Apr 24, 2025
lang/en.ts
Outdated
Comment on lines
4
to
9
| type LangFunction = (...args: string[] | number[]) => string; | ||
|
|
||
| type LangObject = { | ||
| [key: string]: string | LangFunction | LangObject; | ||
| }; | ||
|
|
joe-yeager
reviewed
Apr 24, 2025
lang/en.ts
Outdated
| // @ts-nocheck | ||
| import chalk from 'chalk'; | ||
|
|
||
| type LangFunction = (...args: string[] | number[]) => string; |
Contributor
There was a problem hiding this comment.
should this be ...args: (string | number)[]?
Contributor
Author
There was a problem hiding this comment.
Yeah actually... you'd think so, but for some reason this works as expected (typing each individual arg as string or number). But I think what you suggested is clearer towards we actually want so I'll update it
joe-yeager
previously approved these changes
Apr 24, 2025
Contributor
joe-yeager
left a comment
joe-yeager
left a comment
There was a problem hiding this comment.
LGTM, just a question about one of the types
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description and Context
Was working with the new lang objects and found another opportunity to do some type checking and reduce copy errors. With the
satisfiesoperator, we can default every function in the lang objects to expect only strings or numbers as arguments. This will help reduce accidentally passing inundefined, objects, or anything else unexpected. We can override lang functions on a case by case basis if they need to accept other types (seenoAppsElgible) - I think this is rare enough that the extra checks will be worth the slightly higher effort in those cases.Who to Notify
@brandenrodgers @joe-yeager @kemmerle