-
-
Notifications
You must be signed in to change notification settings - Fork 352
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 remark-d2
to list of plugins
#1197
Conversation
Codecov ReportPatch and project coverage have no change.
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## main #1197 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 10 10
Lines 874 874
=========================================
Hits 874 874 ☔ View full report in Codecov by Sentry. |
@@ -88,6 +88,8 @@ The list of plugins: | |||
* ⚠️ [`remark-custom-blocks`](https://github.com/zestedesavoir/zmarkdown/tree/HEAD/packages/remark-custom-blocks#readme) | |||
— new syntax for custom blocks (new node types, rehype compatible) | |||
(👉 **note**: [`remark-directive`][d] is similar and up to date) | |||
* 🟢 [`remark-d2`](https://github.com/mech-a/remark-d2) | |||
— turn [d2](https://d2lang.com/) diagram code blocks into images |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! Some tips on how to improve your plugin:
- You don’t need
'use strict'
in ESM, they are always strict - Instead of using
node:path
, I recommend using URLs, they work across platforms (unix vs windows) - Throw errors instead of printing to
console.error
when things are misconfigured - You don’t need to
path.parse(file.path)
,file
already hasdirename
and other fields - replace
process.cwd()
withfile.cwd
, files can theoretically exist with multiple, different, “cwd
s” - Use async versions of
fs
calls, the transformer can be async - an
exists
call before amkdir
orwrite
call is an anti-pattern, instead, try tomkdir
orwrite
, and swallow the error if it already exists - do not change
node.type
, instead, replace one node with another, some more info here: https://unifiedjs.com/learn/recipe/remove-node/#removing-a-node - It is probably better to do async work instead of
spawn
, so that when many files are processed, or many code blocks exist, work can be done at the same time - What is the benefit of the HTML version? What can it do more than the markdown version? If there are no benefits, I recommend removing it, because raw strings of HTML don’t always work well with markdown, particularly when dealing with VDOM frameworks like React, such as MDX. There are also perhaps openings for XSS attacks, or at least bugs, when building a string of HTML like you do now, instead of using something like hastscript and hast-util-to-html!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this feedback. I'll integrate your feedback where applicable and reply back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how’s it going with the feedback?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ping! :)
Closing for now. Feel free to PR again, thanks! |
Initial checklist
Description of changes
I've made a plugin, remark-d2, that makes it so you can turn
d2
code blocks into their compiled images.d2
is a fast-to-develop and easy diagramming language, so I think it would be a good addition to the list. They also have a website where you can compare d2 code to other diagramming tools like Mermaid, PlantUML, and others.I'm not affiliated with d2 in any way, in case that is of concern.