Skip to content

Add types #5

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

Merged
merged 5 commits into from
Oct 11, 2020
Merged

Add types #5

merged 5 commits into from
Oct 11, 2020

Conversation

ocavue
Copy link
Contributor

@ocavue ocavue commented Oct 10, 2020

Some questions or TODOs:

  • This PR is pending the release of Add types micromark/micromark#25
  • Is there any difference between micromark's Token and mdast-util-from-markdown's Token?
  • Is it okay to just import Buffer from micromark? Or do we need to re-define our own interface for Node's Buffer?

@wooorm
Copy link
Member

wooorm commented Oct 10, 2020

  1. from-markdown returns a root. Types for mdast are available in @types/mdast
  2. As this project builds on the parts that micromark’s parse halve also takes, I believe it’s fine to use the types defined there here too!

An example of a typical extension is https://github.com/syntax-tree/mdast-util-gfm-table/blob/main/from-markdown.js. So, also enter/exit as keys, which then are objects mapping token types to functions

Copy link
Member

@wooorm wooorm left a comment

Choose a reason for hiding this comment

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

one nit but otherwise good, @ChristianMurphy?

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 @ocavue!
Could we add a type tests for this to verify usage?
similar to https://github.com/micromark/micromark/blob/main/micromark.test.ts

@wooorm
Copy link
Member

wooorm commented Oct 11, 2020

micromark 2.10 has types! (edit: you’re fast)

@ocavue
Copy link
Contributor Author

ocavue commented Oct 11, 2020

😂. I was just doing some experiments between export default and export = and I saw the GitHub notification about the new version of micromark.

@wooorm
Copy link
Member

wooorm commented Oct 11, 2020

Nice! ;)

And, can you apply that export = suggestion, or what did you find?

@ocavue
Copy link
Contributor Author

ocavue commented Oct 11, 2020

Just added export =.

@wooorm
Copy link
Member

wooorm commented Oct 11, 2020

Does it make sense to put all these files in a types/ directory? That's what we've been doing to most other stuff in unified

@ocavue
Copy link
Contributor Author

ocavue commented Oct 11, 2020

Just moved them.

ocavue and others added 2 commits October 11, 2020 21:45
Co-authored-by: Christian Murphy <christian.murphy.42@gmail.com>
Co-authored-by: Christian Murphy <christian.murphy.42@gmail.com>
@wooorm wooorm merged commit 4ff4f0f into syntax-tree:main Oct 11, 2020
@wooorm wooorm added ☂️ area/types This affects typings ⛵️ status/released 🦋 type/enhancement This is great to have 🧑 semver/major This is a change labels Oct 11, 2020
@wooorm
Copy link
Member

wooorm commented Oct 11, 2020

Released, thanks @ocavue!

@ocavue
Copy link
Contributor Author

ocavue commented Oct 11, 2020

You're welcome. It's an honor to be able to contribute to such an awesome project. @wooorm

@wooorm
Copy link
Member

wooorm commented Oct 12, 2020

@ocavue are you also interested in doing to-markdown?

@ocavue
Copy link
Contributor Author

ocavue commented Oct 12, 2020

I can do that. I should be able to submit a PR sometime this week. @wooorm

@wooorm
Copy link
Member

wooorm commented Oct 13, 2020

@ocavue alight! I tried it myself (I didn't want to rush you but also want to publish remark soon): syntax-tree/mdast-util-to-markdown#7

@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.

3 participants