-
-
Notifications
You must be signed in to change notification settings - Fork 150
Update the Range type to labeled tuple for improved type hints
#640
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
base: main
Are you sure you want to change the base?
Update the Range type to labeled tuple for improved type hints
#640
Conversation
eemeli
left a comment
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.
Sounds like a good idea in general, though the current compatibility is with TS 3.9 and later:
Lines 17 to 19 in 92821f2
| The minimum supported TypeScript version of the included typings is 3.9; | |
| for use in earlier versions you may need to set `skipLibCheck: true` in your config. | |
| This requirement may be updated between minor versions of the library. |
Note the last line there; this can change in a minor release. So I'm tempted to accept this, provided that the readme and the CI tests are also updated, but hold off on putting out a release that includes this for a short bit, in case I do actually start properly working on the 3.0 release -- if that advances, introducing this sort-of-breaking-ish change into 2.x is not worthwhile.
d1e2f02 to
7873591
Compare
|
I understand your concern, especially considering that I just updated the branch to match the current TypeScript tests and ensure CI passes. Feel free to pick this change whenever you feel the time is right :) |
eemeli
left a comment
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.
The main branch now contains other breaking changes, including an update to depending on TS 5.9 or later, so this fix can and should be merged.
See inline for a small remaining fix though, as the code comment is now unnecessary.
| @@ -43,7 +43,7 @@ export type ParsedNode = | |||
| | YAMLSeq.Parsed | |||
|
|
|||
| /** `[start, value-end, node-end]` */ | |||
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.
| /** `[start, value-end, node-end]` */ |
Labeled tuple is introduced in TypeScript 4.0, which was released in 2020. Compatibility should be fine.
References:
Rangetype with labeled tuple on TypeScript Playground: link