Skip to content
This repository has been archived by the owner on Feb 16, 2021. It is now read-only.

Exposing node types as constants #2

Closed
kt3k opened this issue Jan 22, 2018 · 9 comments
Closed

Exposing node types as constants #2

kt3k opened this issue Jan 22, 2018 · 9 comments
Labels
👀 area/external This makes more sense somewhere else help wanted 🙏 This could use your insight or help 🧘 status/waiting This may go somewhere but needs more information

Comments

@kt3k
Copy link

kt3k commented Jan 22, 2018

How about exposing node types (strings) as constants of remark module?

const { parse, HEADING, PARAGRAPH } = require('remark')

parse(markdown).children.forEach(node => {
  if (node.type === HEADING) {
    // do something for headings
  } else if (node.type === PARAGRAPH) {
    // do something for paragraphs
  }
})

It would increase the readability of dependent modules.

@wooorm
Copy link
Member

wooorm commented Jan 22, 2018

I’m not sure about your argument for readability. How is your example more readable than the following?

const { parse } = require('remark')

parse(markdown).children.forEach(node => {
  if (node.type === 'heading') {
    // do something for headings
  } else if (node.type === 'paragraph') {
    // do something for paragraphs
  }
})

I have been thinking about using the mdast module similar to hastscript, but for markdown. As a wrapper for unist-builder. Maybe that could expose types as well?

@kt3k
Copy link
Author

kt3k commented Jan 22, 2018

The word readability was a little misleading. Sorry for that. But it is sometimes considered as an antipattern to put string literals directly in source code, especially when the string literal works like a symbol or enum. For example, see this.

So what I do in my source code for now is like the below.

const { parse } = require('remark')
const { HEADING, PARAGRAPH } = require('./my-constant-module')

parse(markdown).children.forEach(node => {
  if (node.type === HEADING) {
    // do something for headings
  } else if (node.type === PARAGRAPH) {
    // do something for paragraphs
  }
})

If remark provides the type strings officially, I don't need define types by myself and the code would look more legitimate.

I have been thinking about using the mdast module similar to hastscript, but for markdown. As a wrapper for unist-builder. Maybe that could expose types as well?

That would be great!

@wooorm
Copy link
Member

wooorm commented Jan 26, 2018

@kt3k Would you like to work on an mdast / mdastscript package?

@kt3k
Copy link
Author

kt3k commented Jan 27, 2018

@wooorm Yes! I'm glad to contribute what I've suggested.

@wooorm
Copy link
Member

wooorm commented Jan 27, 2018

Alright! I added you to the syntax-tree org, would you like to start work in a repo called mdastscript there?
Let me know if I can help, and we can discuss about the name before publishing, potentially switching to mdast?!

@kt3k
Copy link
Author

kt3k commented Jan 28, 2018

Thanks! I joined @syntax-tree org.

I started working on mdastscript for now. https://github.com/syntax-tree/mdastscript

Please review it when I finished scaffolding the test settings.

@wooorm
Copy link
Member

wooorm commented Jan 28, 2018

Super! Looks like a good start!
Let me know if you get a bit further and if there’s more to review! 👍

@kt3k
Copy link
Author

kt3k commented Jan 29, 2018

I added bundling (microbundle), testing (tape & babel), lint (xo), coverage(nyc & codecov) and ci (travis). I put the type constants in index.js and confirmed they exist in test cases.

I think I've done what I can do for now! 😄

I used microbundle because it seemed easier to configure than other tools, but I found that it only supports node >= 6.x. If that's a problem for mdastscript, then I'll switch bundling tool to another one.

I guessed the description of mdastscript as DSL for creating virtual MDAST trees, based on hastscript's description. But I'm not sure if this is correct. Please update it with more correct one.

@wooorm wooorm added help wanted 🙏 This could use your insight or help 👀 area/external This makes more sense somewhere else 🧘 status/waiting This may go somewhere but needs more information labels Aug 14, 2019
@ChristianMurphy
Copy link
Member

Thanks for starting the discussion @kt3k !
We're in the process unifying ideas in with discussions unifiedjs/collective#44
If you'd like to continue this thread, or start a new one https://github.com/remarkjs/remark/discussions/categories/ideas will be the home for ideas going forward.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
👀 area/external This makes more sense somewhere else help wanted 🙏 This could use your insight or help 🧘 status/waiting This may go somewhere but needs more information
Development

No branches or pull requests

3 participants