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
chore: add typescript definitions #346
Conversation
Codecov Report
@@ Coverage Diff @@
## master #346 +/- ##
==========================================
+ Coverage 94.97% 95.48% +0.51%
==========================================
Files 1 1
Lines 1134 554 -580
==========================================
- Hits 1077 529 -548
+ Misses 57 25 -32
Continue to review full report at Codecov.
|
Thanks @connor4312 for these types, which are more complete than the one published on DefinitelyTyped: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/astring/index.d.ts |
* However, if an `output` stream is provided in the `options`, it writes to that stream and returns it. | ||
*/ | ||
export function generate(node: Node, options?: Options<null>): string | ||
export function generate(node: Node, options?: Options<Writable>): Writable |
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.
This overload looks slightly questionable to me. It works because of the order of declarations, but because output is optional {}
satisfies both Options<null>
and Options<Writable>
. To reduce ambiguity I would remove output
from the options and then write this as:
export function generate(node: Node, options?: Options): string
export function generate(node: Node, options: Options & { output: Writable }): Writable
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.
How about this?
export interface Options<Output = null | undefined> {
// …
export function generate(node: Node, options?: Options): string
export function generate(node: Node, options: Options<Writable>): Writable
Thanks for the merge! |
Adding TypeScript definitions will make astring easier to use by people writing their apps in TypeScript.
If you don't want these in the package, they can also be published to DefinitelyTyped, but having them bundled makes life easier for consumers.