docusaurus icon indicating copy to clipboard operation
docusaurus copied to clipboard

When using commonmark, `<var>` inside of `<code>` results in a codeblock

Open robert-j-webb opened this issue 1 year ago • 9 comments

Have you read the Contributing Guidelines on issues?

Prerequisites

  • [X] I'm using the latest version of Docusaurus.
  • [X] I have tried the npm run clear or yarn clear command.
  • [X] I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
  • [X] I have tried creating a repro with https://new.docusaurus.io.
  • [X] I have read the console error message carefully (if applicable).

Description

Heres the rerpo of the issue - you can see the demo here

This is the problem code:

# intro.md:

This is <code>inline <var>var</var></code>

Results in

image

However, it should not result in a code block, instead it should still be inline.

Note that if you remove commonmark support by editing docusaurus.config.ts to not have:

    markdown: {
      format: 'detect',
    },

Then it works with inlining.

I know commonmark support is experimental, and I am not expecting a quick fix. I am merely hoping my issue report will help improve support for commonmark when it does work. Thanks a ton for all of your work on this project :)

Self-service

  • [X] I'd be willing to fix this bug myself.

robert-j-webb avatar Sep 27 '24 23:09 robert-j-webb

is this issue open ?

krittika019 avatar Oct 01 '24 20:10 krittika019

is this issue open ?

Yes

robert-j-webb avatar Oct 01 '24 20:10 robert-j-webb

Hey @robert-j-webb, are you working on this issue ???

azamshaikh1103 avatar Oct 02 '24 03:10 azamshaikh1103

Hey @robert-j-webb, are you working on this issue ???

No, I'm not a maintainer though so I have no say in whether or not it should be worked on which we should wait for before we start doing anything.

robert-j-webb avatar Oct 02 '24 15:10 robert-j-webb

My hypothesis is that it's due to the logic here: https://github.com/facebook/docusaurus/blob/16500436f7b14a6801f9ae466ef405ab29a22b84/packages/docusaurus-theme-classic/src/theme/MDXComponents/Code.tsx

All <code> elements are rendered using this MDXCode, which determines whether it should be rendered as <code> or a <pre> by checking whether it has string contents or nested code. I don't remember the original intention for this but we have always treated <code> the same as <pre> to support things like:

<code>{`
function foo() {
  return 1;
}
`}</code>

I'm not familiar enough with our CommonMark infrastructure to tell why it only happens with CM, though. Investigation is welcome.

Josh-Cena avatar Oct 02 '24 16:10 Josh-Cena

we have always treated <code> the same as \<pre\> to support things like:

<code>{`
function foo() {
  return 1;
}
`}</code>

What's the reasoning to do that?

The <code> element is designed for inline code and the <pre> block is designed for block code. Changing <code> to become <pre> is a violation of the author's intention—it's like saying, "I know better than you what you want." And changing it reinforces bad behavior. People should be forced to learn the correct HTML semantics.

EDIT: @Josh-Cena sorry, you did say, "I don't remember the original intention," so my question is not addressed at you directly. But it's also mostly rhetorical... IMO, this is clearly a bug.

scottamain avatar Oct 02 '24 17:10 scottamain

I'm happy to always render <code> as inline regardless of what's inside it, given that it's compatible with what people expect and/or are already doing. We actually have testing for this: https://docusaurus.io/tests/pages/code-block-tests (which is actually broken atm)

And, blaming that test, I dug up some more context: https://github.com/facebook/docusaurus/pull/6767, https://github.com/facebook/docusaurus/pull/6177, https://github.com/facebook/docusaurus/pull/2857. It seems that this has been the case ever since https://github.com/facebook/docusaurus/pull/1555, and we don't know the intention anymore since the author is no longer there.

Josh-Cena avatar Oct 02 '24 17:10 Josh-Cena

😄 It's been a while.

We'll never know exactly but to me the goal is to be able to distinguish Markdown triple-backticks code blocks (that MDX automatically wraps in <pre>) from Markdown inline code blocks, that all use the CodeBlock component.

Note that things used to be different under MDX v1 and now MDX v2 also has a different behavior. I think we can get rid of some complexity here indeed.

For example these historical things:

 function MDXPre(props: Props): ReactNode | undefined {
  // With MDX 2, this element is only used for fenced code blocks
  // It always receives a MDXComponents/Code as children
  return <>{props.children}</>;
}
// Solution inspired by https://github.com/pomber/docusaurus-mdx-2/blob/main/packages/mdx-loader/src/remark/codeCompat/index.ts
// TODO after MDX 2 we probably don't need this - remove soon?
// Only fenced code blocks are swapped by pre/code MDX components
// Using <pre><code> in JSX shouldn't use our MDX components anymore

// To make theme-classic/src/theme/MDXComponents/Pre work
// we need to fill two properties that mdx v2 doesn't provide anymore
export default function codeCompatPlugin(this: Processor): Transformer {
  return async (root) => {
    const {visit} = await import('unist-util-visit');

    visit(root, 'code', (node: Code) => {
      node.data = node.data || {};

      node.data.hProperties = node.data.hProperties || {};
      node.data.hProperties.metastring = node.meta;

      // Retrocompatible support for live codeblock metastring
      // Not really the appropriate place to handle that :s
      node.data.hProperties.live = node.meta?.split(' ').includes('live');
    });
  };
}

Will need to study all that in depth to figure out what we should do, thanks for reporting.

slorber avatar Oct 03 '24 10:10 slorber

For future references, here's how MDX compiles:

CleanShot 2024-10-03 at 12 37 15

And here's how it compiles CommonMark:

CleanShot 2024-10-03 at 12 38 20

slorber avatar Oct 03 '24 10:10 slorber