Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add remark-steps to list of plugins #1281

Closed
wants to merge 1 commit into from
Closed

Conversation

lituidev
Copy link

@lituidev lituidev commented Jan 23, 2024

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and couldn’t find anything (or linked relevant results below)
  • If applicable, I’ve added docs and tests

Description of changes

ADD

Signed-off-by: William Lee <154324507+lituidev@users.noreply.github.com>
@github-actions github-actions bot added the 👋 phase/new Post is being triaged automatically label Jan 23, 2024

This comment has been minimized.

@github-actions github-actions bot added 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Jan 23, 2024
@ChristianMurphy
Copy link
Member

ChristianMurphy commented Jan 23, 2024

Welcome @lituidev! 👋
Thanks for sharing!
A few thoughts:

  1. This plugin appears to be a specific kind of directive, it would be good to note both in the description here and in the package itself that directives are required (source: https://github.com/lit-ui/remark-steps/blob/ad235a49c593a5b57e3308552844c89f1d3b2d6e/src/index.ts#L23)
  2. There is a types only package for MDAST nodes https://www.npmjs.com/package/@types/mdast (avoid hard coding your own node type, source: https://github.com/lit-ui/remark-steps/blob/ad235a49c593a5b57e3308552844c89f1d3b2d6e/src/index.ts#L12-L19)
  3. HAST and MDAST are not the same, avoid pushing HAST nodes as children of MDAST, instead use data.hChildren https://github.com/syntax-tree/mdast-util-to-hast#fields-on-nodes (source: https://github.com/lit-ui/remark-steps/blob/ad235a49c593a5b57e3308552844c89f1d3b2d6e/src/index.ts#L32-L42)
  4. You probably want to target node16 module resolution for ESM https://www.typescriptlang.org/docs/handbook/modules/reference.html#node16-nodenext (source: https://github.com/lit-ui/remark-steps/blob/ad235a49c593a5b57e3308552844c89f1d3b2d6e/tsconfig.json#L4-L6)
  5. You probably want to publish somewhat readable source, let downstream adopters decide how they want to transpile, polyfill, or minify code (source: https://github.com/lit-ui/remark-steps/blob/ad235a49c593a5b57e3308552844c89f1d3b2d6e/rollup.config.mjs#L17)
  6. Avoid using any in TypeScript and especially in the public interface, it largely defeats the purpose of having type checking in the first place (source: https://github.com/lit-ui/remark-steps/blob/ad235a49c593a5b57e3308552844c89f1d3b2d6e/src/index.ts#L21)
  7. There are no tests currently, it can help ensure the library is working and make it easier for others to contribute by including tests. https://nodejs.org/api/test.html is very light weight and fast. https://vitest.dev/ is more feature rich similar to Jest, but with ESM support.

There are some good examples at https://unifiedjs.com/learn/ of how to traverse ASTs, narrow types, and construct ASTs in a type safe way.

@wooorm
Copy link
Member

wooorm commented Sep 27, 2024

Closing this as it seems abandoned. This also makes more sense as a rehype plugin I think!

@wooorm wooorm closed this Sep 27, 2024
@wooorm wooorm added the 🤷 no/invalid This cannot be acted upon label Sep 27, 2024

This comment has been minimized.

@github-actions github-actions bot added 👎 phase/no Post cannot or will not be acted on and removed 🤞 phase/open Post is being triaged manually labels Sep 27, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🤷 no/invalid This cannot be acted upon 👎 phase/no Post cannot or will not be acted on
Development

Successfully merging this pull request may close these issues.

3 participants