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

add remark-embed-tag plugins.md #1305

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

Reimirno
Copy link

@Reimirno Reimirno commented Apr 8, 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 remark-embed-tag to the plugin list.

Signed-off-by: Yu Long <82516689+Reimirno@users.noreply.github.com>
@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Apr 8, 2024
@remcohaszing
Copy link
Member

remcohaszing commented Apr 8, 2024

Thanks @Reimirno!

Some tips:

  1. Your goal is to emit HTML. This is generally best done in a rehype plugin, not a remark plugin.
  2. Regardless what you think of the former suggestion, I suggest adding a test for emitting with rehype.
  3. Jest has issues when used with ESM. I strongly suggest using another test runner, for example node:test.
  4. Change the type of ast to Root. This way you get full type safety within visit.
  5. You’re missing a dependey on @types/mdast.
  6. You’re not supposed to change a node’s type. Instead, replace it with a new node.
  7. Your transformer does not need to be async. In some contexts only synchronous plugins can be used.
import type { Root } from "mdast"
import { visit } from "unist-util-visit";
// …

export default function remarkTagEmbed(configs: Config = {} as Config) {
  configs = { ...defaultConfig, ...configs };
  const transformer = (ast: Root) => {
    visit(ast, "text", (node, index, parent) => {
      if (index == null || !parent) {
        return
      }
      const { value } = node;
      if (configs.youtube) {
        const youtube = parseYouTube(value);
        if (youtube) {
          parent[index] = {
            type: 'html',
            value: createsYouTubeWidget(youtube)
          }
        }
      }
      // …
    });
  };
  return transformer;
}

Or you could use unified’s Plugin type to get full TypeScript support based on one type annotation. This is what I generally recommend.

import type { Root } from "mdast"
import type { Plugin } from "unified"
import { visit } from "unist-util-visit";
// …

const remarkTagEmbed: Plugin<[Configs?], Root> = (configs) => {
  configs = { ...defaultConfig, ...configs };
  return (ast) => {
    visit(ast, "text", (node, index, parent) => {
      if (index == null || !parent) {
        return
      }
      const { value } = node;
      if (configs.youtube) {
        const youtube = parseYouTube(value);
        if (youtube) {
          parent[index] = {
            type: 'html',
            value: createsYouTubeWidget(youtube)
          }
        }
      }
      // …
    });
  };
}

export default remarkTagEmbed

@Reimirno
Copy link
Author

Reimirno commented Apr 9, 2024

@remcohaszing Thank you so much for the detailed and prompt review!

  1. I see... I was searching through remark plugins for html embeds and didn't come across anything suits my need and hence I made this though. I saw many plugins that transform md->html too so I just assumed that is the intended use case :) Plus, I thought rehype is for transformation from html->html, while remark is md->md or md->html.
  2. Ah ok. Any example I could refer to?
  3. Yeah Jest is ESM hell... but I made it work for this project. Will consider switching if situation demands.
  4. Yes.
  5. @types/mdast is a dependency of remark and I already have remark. I think that is ok?
  6. Okie.
  7. Okie. I now use the Plugin type as you recommended. This is indeed a lot more cleaner.

@remcohaszing
Copy link
Member

  1. I see... I was searching through remark plugins for html embeds and didn't come across anything suits my need and hence I made this though. I saw many plugins that transform md->html too so I just assumed that is the intended use case :) Plus, I thought rehype is for transformation from html->html, while remark is md->md or md->html.

remark is for mdast (markdown) to mdast. remark-rehype does mdast → hast (HTML). rehype is for hast to hast.

export default remarkTagEmbed

  1. Ah ok. Any example I could refer to?

Just like in the remark plugin, you will probably need to iterate over all text nodes. But instead of creating a string of HTML, you should create hast elements.

You can have a look at the rehype plugins list. If you’re interested in a TypeScript implementation specifically, you can have a look at rehype-mermaid.

  1. @types/mdast is a dependency of remark and I already have remark. I think that is ok?

npm does not guarantee transitive dependencies are available or the same version unless they are in dependencies. pnpm explicitly makes sure it doesn’t work. Everything you import in your generated output, should be in your dependencies. So this includes @types/mdast and unified, but not remark.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤞 phase/open Post is being triaged manually
Development

Successfully merging this pull request may close these issues.

None yet

2 participants