Skip to content

Conversation

@Murderlon
Copy link
Member

Closes #4

Wanted to know the process of adding types to an existing lib so here we are. I followed Microsoft's advice on using tsd instead of dtslint. Am I missing a reason dtslint is always used even for simple tests?

@Murderlon Murderlon added 🦋 type/enhancement This is great to have 🧑 semver/major This is a change ☂️ area/types This affects typings 🙆 yes/confirmed This is confirmed and ready to be worked on labels Mar 13, 2020
@ChristianMurphy
Copy link
Member

ChristianMurphy commented Mar 13, 2020

Am I missing a reason dtslint is always used even for simple tests?

Good question.
dtslint is type checking, plus a range of lint checks.
For developers that only want type checks tsd is an excellent option.
At unified (so far), there has been a preference to have both the type checks and the lint checks, to keep the code following a consistent standard.

I followed Microsoft's advice on using tsd

This isn't advise, so much as offering an alternative for developers who don't want the lint checks.

Copy link
Member

@ChristianMurphy ChristianMurphy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Murderlon this looks pretty good!
Would it be possible to use dtslint to get the additional lint checks provided?
And could a test case be added verifying the util can be used in a Unified plugin/tranform? (similar to https://github.com/syntax-tree/mdast-util-toc/blob/94cbac8b65c16209537d863f092642cb6a45bcf4/types/mdast-util-toc-tests.ts#L55-L64)

Copy link
Member

@ChristianMurphy ChristianMurphy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @Murderlon! 🙇‍♂️

@ChristianMurphy
Copy link
Member

/cc @Rokt33r

@Murderlon
Copy link
Member Author

And could a test case be added verifying the util can be used in a Unified plugin/tranform? (similar to syntax-tree/mdast-util-toc:types/mdast-util-toc-tests.ts@94cbac8#L55-L64)

I added a unified plugin example. I’m a bit confused on what it tests in this context. To me, it seems I don’t explicitly assert anything?

@ChristianMurphy
Copy link
Member

I added a unified plugin example. I’m a bit confused on what it tests in this context. To me, it seems I don’t explicitly assert anything?

Mainly it ensures that tree as it is passed from unified matches the signature of modify, if those were to conflict dtslint will error.
It also offers a test case that shows usage in the most common scenario, being used in a plugin.

@Murderlon Murderlon merged commit 968e97c into master Mar 19, 2020
@Murderlon Murderlon deleted the murderlon/#4 branch March 19, 2020 18:38
@wooorm
Copy link
Member

wooorm commented Mar 19, 2020

Released in 2.0.0!

@wooorm wooorm added ⛵️ status/released and removed 🙆 yes/confirmed This is confirmed and ready to be worked on labels Mar 19, 2020
@wooorm wooorm added the 💪 phase/solved Post is done label Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

☂️ area/types This affects typings 💪 phase/solved Post is done 🧑 semver/major This is a change 🦋 type/enhancement This is great to have

Development

Successfully merging this pull request may close these issues.

Would love TypeScript types for this package

4 participants