Skip to content
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

Another plugin usage example #35

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

msakrejda
Copy link

Similar to #31, but for remark-frontmatter and remark-mdx-frontmatter, which are plugins I'm interested in getting working (and since the main README claims remark-frontmatter as a working plugin, it'd be nice to document how to set that up). #31 should be merged first and then I can rebase on top of it--for now the history of the branch is ugly because I just forked that example (thanks for the project structure @icew4ll!), but I can clean that up (probably by integrating the two versions of the example into one).

I actually set out to document how this does not work right now, but with some help from remark-mdx-frontmatter maintainers, I got it working.

TODO:

See also #30.

@brillout
Copy link
Owner

@ajstrand @aleclarson Anyone up for reviewing this? I'm swamped with work for Vike :-).

@msakrejda
Copy link
Author

No need to review anything yet--I created the PR in draft mode because I think #31 should land first (though I guess it's been a while--if you think that PR is abandoned by the author, I can merge their changes into my PR and we can just continue here instead.)

@ajstrand
Copy link
Collaborator

@brillout i can review whenever @uhoh-itsmaciek says the PR is ready for review.

@brillout
Copy link
Owner

I can merge their changes into my PR and we can just continue here instead.

Yes, I just closed #31.

@brillout i can review whenever @uhoh-itsmaciek says the PR is ready for review.

Neat 😊

Copy link
Collaborator

@silvenon silvenon left a comment

Choose a reason for hiding this comment

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

To make the start scripts more robust, I'd add --force to yarn install just in case yarn skips install if everything is up to date, which can happen in case someone ran install before running this script, as you already experienced (and me too when reviewing this PR).


This text is written in Markdown.

MDX allows Rich React components to be used directly in Markdown: <Counter/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the reason for using a .md extension over .mdx?

yarn add remark-frontmatter@^3.0.0 remark-mdx-frontmatter
```

Note that remark-frontmatter has some breaking changes after version 3; pin to that version.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Best to be descriptive here because we don't want to discourage people from upgrading, so something like:

Suggested change
Note that remark-frontmatter has some breaking changes after version 3; pin to that version.
We're using remark-frontmatter 3 here because versions 4 and higher are ESM-only.


dt:not(:first-child) {
margin-top: 6px;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd make <code> tag inside <pre> block, to highlight code blocks better:

pre code {
  display: block;
}

@@ -47,7 +47,7 @@ Features:
const options = {
// See https://mdxjs.com/advanced/plugins
remarkPlugins: [
// E.g. `remark-frontmatter`
// E.g. `require("remark-prism")`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mixing import and require is slightly confusing, I'd import remark-prism at the top of the config and pass the result here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants