storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84.55k stars 9.3k forks source link

[Bug]: Wrapping stories inside HTML elements breaks MDX Docs page in Storybook 7 #24158

Closed msanzv closed 1 year ago

msanzv commented 1 year ago

Describe the bug

In my project, we have most stories already defined in tsx files, but some are still in mdx files that combine docs + stories. In all of those, we have this structure that was working with Storybook 6.5

// Imports

// Template

// ArgsTable

<details>
  <summary>Show interaction tests</summary>

  // Some stories
</details>

We did this to hide our interaction tests inside a collapsible element, at the bottom of the Docs page.

After following the Storybook 7 migration guide, my tsx files work fine, but the mdx files throw an error like this, mentioning the name of the first test inside the <details> block:

image



It took me a while until I noticed that removing the details block fixed the errors, but then the tests are not collapsed anymore if I do so. At first I thought it was related to MDX2 somehow not accepting some specific HTML elements, but it also happens changing it to a <div>. In fact the <details> element works fine if it doesn't have stories inside it. What causes the error is having stories inside any HTML block.

<!-- Doesn't work -->
<details>
  <summary>Show interaction tests</summary>
  // Some stories
</details>

<!-- Doesn't work -->
<div>
  // Some stories
</div>

<!-- Works (but changes the result) -->
<details>
  <summary>Show interaction tests</summary>
  Lorem ipsum dolor sit amet
</details>
// Some stories

It's not a blocker for me, I don't mind just removing those details blocks since we will migrate those mdx stories to tsx files soon. But I am curious, what is causing this? Why it was working in v6.5 but doesn't in v7.4? I'm guessing it's related to the move to MDX2 and the update of the parser and the addon-docs, but I googled for hours and looked through issues here and in MDX2 repos and couldn't find an issue similar to mine.

If there's an existing issue/question addressing this that I couldn't find, I'd appreciate a link. Thanks in advance!

To Reproduce

No response

System

Storybook 7.4.0 (also tried 7.4.1) + all related packages installed via automigration

Additional context

No response

shilman commented 1 year ago

@msanzv Do you a have a reproduction repo you can share? If not, can you create one? Go to https://storybook.new or see repro docs.

The code that handles the MDX-based story definition is different between MDX1 and MDX2, so it's possible that there was a regression for stories whose definitions are nested inside other DOM elements. Looking through the tests I only see tests for <Story> inside <Canvas> blocks https://github.com/storybookjs/mdx2-csf/blob/next/src/index.test.ts

msanzv commented 1 year ago

Hi @shilman, thanks for your reply. I tried replicating with Stackblitz here but I couldn't figure out how to create a simple story there in the only .mdx file I saw (Configure.mdx). I tried creating a basic template and story but that already throws the error I'm seeing in my project ("No story found with that name"), without the need of any wrapping html. You can check my changes there in lines 38-44. Is there anything I'm missing the cause it's not working? Sorry, I'm not very experienced with Storybook configs.

Also tried, in my project, to change .stories.mdx files to just .mdx (and of course the appropriate main.ts config), as it is the new recommendation, but that didn't seem to make any difference. And also, I'm not sure how to manually downgrade to storybook 6 so I can try an example of what was working in my setup before the migration.


Edit, forgot to reply to this:

Looking through the tests I only see tests for inside blocks

Some of my failing mdx stories are wrapped in <Canvas> and some are not, so doesn't sound like that is the cause of the issue.

github-actions[bot] commented 1 year ago

Hi there! Thank you for opening this issue, but it has been marked as stale because we need more information to move forward. Could you please provide us with the requested reproduction or additional information that could help us better understand the problem? We'd love to resolve this issue, but we can't do it without your help!

github-actions[bot] commented 1 year ago

I'm afraid we need to close this issue for now, since we can't take any action without the requested reproduction or additional information. But please don't hesitate to open a new issue if the problem persists – we're always happy to help. Thanks so much for your understanding.

msanzv commented 11 months ago

This still happens, but as mentioned before, it's not a blocker for us since we're migrating all components to CSF3 anyway. I opened the issue mostly out of curiosity for what was happening. Probably not an issue affecting many people.

Thanks anyway, and feel free to close it if it's not worth investigating.