-
Notifications
You must be signed in to change notification settings - Fork 27
Usage without gatsby #157
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
Usage without gatsby #157
Conversation
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.
Thanks so much for taking this on! This looks pretty uncontroversial to me. Code sharing between this and the gatsby equivalent could definitely be better—are there opportunities where it makes sense to extract some stuff, or are there subtle difference interleaved throughout the plugin body?
I think longer term it would make sense if this were in a separate package that gatsby-remark-vscode depended on (it does have “gatsby” in its name after all), but I’m totally happy to try this out here for now, especially given that I haven’t had time to spend maintaining this myself lately. Just happy for someone else to keep it alive.
CI is only failing due to prettier. npm run format
should do the trick.
FYI I’m going to be on a road trip off and on for the next week, so I might be slow to respond, but will try to check in and get this thing merged. Do you want to add something to the README as well?
There are a few subtle differences interleaved and I also made one or two minor adjustments to be working in an ESM package (require.main.filename not being defined). I agree that code sharing between this and the gatsby equivalent could be way better, but I don't really know what the best approach would be. Maybe, if you'll have the time in the future, we can pair on this.
I would like that aswell! For the time being, I'm happy that I managed to get this working at all, though 😁
Will do.
I'll add a tiny bit to the README and would love to get this merged/released. Enjoy your road trip! |
Is @wooorm’s feedback in #99 actionable here, @codepunkt? |
Probably, but that sounds like a general improvement to the library that’s not specific to this PR. |
@andrewbranch Not sure whether or not you're still on your roadtrip - I hope everything's fine! I added documenteation to the |
That might be true, but it would be news to me. The impression I got when writing this originally is that if I wanted to inject HTML into a Markdown file, I just had to stringify that HTML and put it in an HTML node—in other words, that I couldn't embed an HTML AST inside the Markdown AST. If that's wrong, I would have built this super differently 😅 Agreed that probably shouldn't be part of this PR, but I wanted to get a better understanding of the problem and the suggestion before committing to these two plugins living in the same place. If the Remark plugin is going to be permanently held back due to the gatsby-remark plugin needing stringified HTML, this project seems a little less worthwhile to me. |
I believe Gatsby has also always supported the embedded AST, so I don’t think there is any holding back! |
Fixes #99