Skip to content
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

Add types #1

Merged
merged 9 commits into from
Feb 10, 2021
Merged

Add types #1

merged 9 commits into from
Feb 10, 2021

Conversation

mikaelkaron
Copy link
Contributor

This PR is an attempt to add TypeScript types for this package. I saw in a related ticket that it's preferred to add types directly to the package in question rather than @types so that's what I did.

JSDoc was taken from the README.md

@ChristianMurphy
Copy link
Member

Thanks @mikaelkaron!
Could you include some type tests with dtslint?
There are some example of how this looks in some recent typing PRs across unified remarkjs/remark-external-links#18, remarkjs/remark-footnotes#4, remarkjs/remark-html#32

@ChristianMurphy ChristianMurphy added ☂️ area/types This affects typings 🦋 type/enhancement This is great to have 🧑 semver/major This is a change labels Feb 7, 2021
@mikaelkaron
Copy link
Contributor Author

Stole some tests from test.js and updated the definitions to work with that.

types/index.d.ts Show resolved Hide resolved
types/vfile-rename-tests.ts Outdated Show resolved Hide resolved
types/vfile-rename-tests.ts Outdated Show resolved Hide resolved
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.

Excellent, thanks @mikaelkaron!
Let's get a test case in for rename.convert as well, and this will LGTM

@wooorm wooorm merged commit 278ebb9 into vfile:main Feb 10, 2021
@wooorm wooorm added 🧒 semver/minor This is backwards-compatible change and removed 🧑 semver/major This is a change labels Feb 10, 2021
@wooorm
Copy link
Member

wooorm commented Feb 10, 2021

Released, thanks @mikaelkaron!

P.S. @ChristianMurphy I’ve been tagging things that add types as minor recently in the case of a package that’s likely not deep down in ancient versions of Node, and it’s going pretty well (as in, I haven‘t seen any issues)

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/minor This is backwards-compatible change 🦋 type/enhancement This is great to have
Development

Successfully merging this pull request may close these issues.

3 participants