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

MDX2: newline character is added visually, and extra <p> tag is getting inserted in the DOM #18921

Closed
melanieseltzer opened this issue Aug 11, 2022 · 6 comments

Comments

@melanieseltzer
Copy link
Contributor

Describe the bug

I've come across two rendering issues working with MDX 2 / previewMdx2: true is set.

  • When there's no newline between jsx in the story source code, "\n" gets inserted visually in the UI.

  • When the jsx is multi-line in the source code, then the content gets wrapped in a <p> tag in the DOM. ONLY occurs if the source code itself (the jsx) is multi-line, not if the content itself is rendered on multiple lines and wraps in the UI.

I can't seem to upload any screenshots along with this issue report to help visually show this (GitHub says "something went wrong and we can't process that file" 😕), so please check out my link below to reproduce this.

To Reproduce
https://github.com/melanieseltzer/mdx2-rendering-bugs-reproduction

System

  System:
    OS: macOS 11.2.3
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
  Binaries:
    Node: 16.16.0 - ~/.nvm/versions/node/v16.16.0/bin/node
    Yarn: 1.22.17 - /usr/local/bin/yarn
    npm: 8.11.0 - ~/.nvm/versions/node/v16.16.0/bin/npm
  Browsers:
    Chrome: 104.0.5112.79
    Firefox: 102.0.1
    Safari: 14.0.3
  npmPackages:
    @storybook/addon-docs: ^6.5.10 => 6.5.10 
    @storybook/addon-essentials: ^6.5.10 => 6.5.10 
    @storybook/builder-webpack5: ^6.5.10 => 6.5.10 
    @storybook/manager-webpack5: ^6.5.10 => 6.5.10 
    @storybook/mdx2-csf: ^0.0.3 => 0.0.3 
    @storybook/react: ^6.5.10 => 6.5.10

Additional context
Add any other context about the problem here.

@zhyd1997
Copy link
Contributor

This is mdx library issue.

SHORT ANSWER

The bug occurred at fromMarkdown fn.

LONG EXPLANATION

For mdx example from repro repo:
https://github.com/melanieseltzer/mdx2-rendering-bugs-reproduction/blob/47f402b136f7603016cb7e186086cd25515b2fba/stories/Heading.stories.mdx

to demonstrate the explanation, I set up a new repo: debug-mdx

the call stack is:

compile(file, options?) <- process(file) <- parse(doc)

for line 263, the file result a normal VFile object:

VFile {
  data: {},
  messages: [],
  history: [],
  cwd: '/Users/zhyd1997/workspace/mdx',
  value: "import { Canvas, Meta, Story, ArgsTable } from '@storybook/addon-docs';\n" +
    "import { Heading } from './Heading';\n" +
    '\n' +
    '<Meta title="Components/Heading" component={Heading} />\n' +
    '\n' +
    '# MDX 2 Rendering Bugs\n' +
    '\n' +
    `When there's no linebreak between jsx in the story source code, "\n` +
    '" gets inserted visually in the UI.\n' +
    '\n' +
    '<Canvas>\n' +
    '  <Story name="one">\n' +
    '    <Heading>Lorem ipsum dolor sit amet</Heading>\n' +
    '    <Heading>Lorem ipsum dolor sit amet</Heading>\n' +
    '  </Story>\n' +
    '</Canvas>\n' +
    '\n' +
    `When there is a linebreak between jsx in the story source code, then there's no "\n` +
    '" in the UI, and things look normal.\n' +
    '\n' +
    '<Canvas>\n' +
    '  <Story name="two">\n' +
    '    <Heading>Lorem ipsum dolor sit amet</Heading>\n' +
    '\n' +
    '    <Heading>Lorem ipsum dolor sit amet</Heading>\n' +
    '\n' +
    '  </Story>\n' +
    '</Canvas>\n' +
    '\n' +
    'When the jsx is multi-line in the source code, then the content gets wrapped in a \\<p\\> tag when rendered.\n' +
    '\n' +
    'ONLY occurs if the source code itself (the jsx) is multi-line, not if the content itself is rendered on multiple lines and wraps in the UI.\n' +
    '\n' +
    '<Canvas>\n' +
    '  <Story name="three">\n' +
    '    <Heading>I am very long, but am kept on one line in the source code, so there is no wrapper paragraph tag inserted around me. I am very long, but am kept on one line in the source code, so there is no wrapper paragraph tag inserted around me.</Heading>\n' +
    '\n' +
    '    <Heading>\n' +
    '      Multi-line in the source code, wrapped in p tag.\n' +
    '    </Heading>\n' +
    '\n' +
    '  </Story>\n' +
    '</Canvas>\n' +
    '\n' +
    '## Props\n' +
    '\n' +
    '<ArgsTable of={Heading} />'
}

but when running the return statement, the result is strange (you can try to run console.log(JSON.stringify(Parser(String(file), file))), and format it on a online JSON viewer.):

  1. \n issue:

Screen Shot 2022-08-16 at 16 47 13

Screen Shot 2022-08-16 at 16 51 54

  1. <p> issue:

Screen Shot 2022-08-16 at 16 36 48

Screen Shot 2022-08-16 at 16 34 02

After logging the Parser:

console.log(Parser.toString())

I found it uses remark-parse parser.

so continuing the call stack:

remarkParse(options) <- fromMarkdown

the definition of fromMarkdown is from mdast-util-from-markdown:

https://github.com/syntax-tree/mdast-util-from-markdown/blob/77208c8d1bfaf33f4024ce0b90e640178322e7d1/dev/lib/index.js#L105

That's all I found until now.

@zhyd1997
Copy link
Contributor

Update: fromMarkdown relies on parse, maybe the deeper cause is related to this issue from micromark.

@wooorm
Copy link

wooorm commented Aug 20, 2022

I don’t know enough about everything here, but the main thing on the <p> being there sounds like the rule of interleaving is not respected: https://mdxjs.com/docs/what-is-mdx/#interleaving. Follow that rule, and it is good.

@melanieseltzer
Copy link
Contributor Author

Thanks @wooorm, make sense 👍

In my case, it was because I have my IDE set to run Prettier on save, and text was getting forced onto a new line due to props bloating the line length.

Getting around it in my stories with curly braces:

<Heading prop="asdf" anotherProp="asdf">
  {"Some text"}
</Heading>

@shilman
Copy link
Member

shilman commented Jun 9, 2023

We’re cleaning house! Storybook has changed a lot since this issue was created and we don’t know if it’s still valid. Please open a new issue referencing this one if:

@shilman shilman closed this as not planned Won't fix, can't repro, duplicate, stale Jun 9, 2023
@Katerina198b
Copy link

This is still a problem

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

No branches or pull requests

5 participants