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

using a transformer to add a wrapping element results in extra duplicative nodes #811

Open
3 of 5 tasks
olets opened this issue Oct 17, 2024 · 5 comments
Open
3 of 5 tasks

Comments

@olets
Copy link
Contributor

olets commented Oct 17, 2024

Validations

Describe the bug

I'm working on a transformer which adds a root wrapper. Minimal goal is

<div data-my-attr="val">
  <pre class="shiki <theme>" style="" tabindex="0">
    <code class="language-js"></code>
  </pre>
</div>

I'm using @shikijs/markdown-it.

root hook

My first thought was to use the root hook:

        root(hast) {
          return [
            {
              type: 'element',
              tagName: 'div',
              properties: {
                'data-my-attr': 'val',
              },
              children: hast.children,
            },
          ];
        },

Given

```js
```

that transformer results in this HTML (I added the comments)

<pre> <!-- unexpected wrapper 1 -->
  <code class="language-js"> <!-- unexpected wrapper 2 -->
    <div data-my-attr="val">
      <pre class="shiki <theme>" style="" tabindex="0">
        <code class="language-js">
          <span class="line"></span>
        </code>
      </pre>
    </div>
  </code>
</pre>

postprocess hook

Using a postprocess hook is a better fit for my need, because I'd like to use options.meta.

That doesn't work either:

        postprocess(html) {
          return `<div data-my-attr="val">${html}</div>`;
        },

Given the same markdown as in the root hook example above, I get the same problematic result as above.

Note that if we console log html in this hook, it does already have the pre > code wrappers.

Reproduction

https://stackblitz.com/edit/shikijs-shiki-issue-811

Contributes

  • I am willing to submit a PR to fix this issue
  • I am willing to submit a PR with failing tests
@fuma-nama
Copy link
Contributor

Make sure to return a Root element:

function transformerTab(): ShikiTransformer {
  return {
    name: 'rehype-code:tab',
    root(root) {
      return {
        type: 'root',
        children: [
          {
            // anything you want ;)
            children: root.children,
          },
        ],
      };
    },
  };
}

@olets
Copy link
Contributor Author

olets commented Oct 28, 2024

Thanks for the correction. I see now that that's made clear in the types. Still looks like there isn't a way to get this to work with a root transformer https://stackblitz.com/edit/shikijs-shiki-issue-811-bwecfk?file=main.js. Feels like a bug but maybe I'm misunderstanding "root".

And again a postprocess hook is really what I need, to have access to the rest of the fence. If anyone has a solution for that please share 🙏

(And if anyone finds this issue because you're trying to migrate from shikijs/twoslash/remark-shiki-twoslash or shikijs/twoslash/eleventy-plugin-shiki-twoslash with a custom transform to @shikijs/markdown-it to get Shiki v1: for now I'm using my forks https://github.com/olets/shiki, which has v1's languages and themes)

@fuma-nama
Copy link
Contributor

fuma-nama commented Oct 29, 2024

oh you're right, I overlooked that. Markdown-it wraps the result from highlight with <pre> and <code> if the result does not.

See markdown-it/markdown-it#269, this can be disabled with some configurations.

Or the jsdoc of markdown-it:

    /**
     * Highlighter function for fenced code blocks.
     * Highlighter `function (str, lang, attrs)` should return escaped HTML. It can
     * also return empty string if the source was not changed and should be escaped
     * externally. If result starts with <pre... internal wrapper is skipped.
     * @default null
     */
    highlight?: ((str: string, lang: string, attrs: string) => string) | null | undefined;
}

@olets
Copy link
Contributor Author

olets commented Oct 29, 2024

Great find!

Okay now I have working implementation of @shikijs/markdown-it with a postprocess transformer where the outermost element isn't a <pre>. As mentioned in the issue you linked, it requires copypasting markdown-it's source. https://stackblitz.com/edit/shikijs-shiki-issue-811-solution-for-markdown-it-14-1-0?file=main.js

I've proposed a change to markdown-it markdown-it/markdown-it#269 (comment), but the maintainer has said no to related ideas before.

Maybe as a bandaid we could add my stackblitz's fence rule to the @shikijs/markdown-it exports? Then users using a root transform or postprocess transformer to add a wrapper around the <pre> could

import shikiMarkdownItPlugin, { wrapperlessMarkdownItFenceRule } from '@shikijs/markdown-it';
import MarkdownIt from 'markdown-it';

const md = MarkdownIt();

md.renderer.rules.fence = wrapperlessMarkdownItFenceRule;

md.use(await shikiMarkdownItPlugin({
  // …
  transformers: // …,
}));

@olets
Copy link
Contributor Author

olets commented Nov 22, 2024

I packaged up the wrapperless fence rule as a markdown-it plugin:

https://github.com/olets/markdown-it-wrapperless-fence-rule

The README has some Stackblitz links

Would be happy to open a PR mentioning this plugin in the docs if maintainers are into it.

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

No branches or pull requests

2 participants