storybookjs / storybook

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

[Bug]: The `Primary` doc block in an attached MDX file shows invalid code preview #21125

Closed elisehein closed 1 year ago

elisehein commented 1 year ago

Describe the bug

I'm seeing invalid code previews when using the Primary doc block and clicking "Show code":

import {
  Meta,
  Primary,
} from "@storybook/blocks";

import * as ButtonStories from "./Button.stories.js";

<Meta of={ButtonStories} />

<Primary />
CleanShot 2023-02-16 at 15 52 42@2x

When relying on autodocs, the docs page generated for each component shows the correct JSX in the code preview. But when I add an MDX file and attach that file to a component using the Meta block, none of the code previews work (the example above is for the Primary block, but I'm also seeing the same error on the Canvas block when using <Canvas of={ButtonStories.Primary} />).

To Reproduce

See reproduction repo: https://github.com/elisehein/storybook7-primary-block-code-preview

See README for steps to reproduce, or have a look at the problematic file https://github.com/elisehein/storybook7-primary-block-code-preview/blob/main/src/stories/Button.mdx?plain=1

The repro was created using npx sb@next init

System

Environment Info:

  System:
    OS: macOS 12.6.3
    CPU: (10) arm64 Apple M1 Pro
  Binaries:
    Node: 16.17.1 - /opt/homebrew/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 8.15.0 - /opt/homebrew/bin/npm
  Browsers:
    Chrome: 110.0.5481.96
    Firefox: 109.0.1
    Safari: 15.6.1
  npmPackages:
    @storybook/addon-essentials: ^7.0.0-beta.47 => 7.0.0-beta.47
    @storybook/addon-interactions: ^7.0.0-beta.47 => 7.0.0-beta.47
    @storybook/addon-links: ^7.0.0-beta.47 => 7.0.0-beta.47
    @storybook/blocks: ^7.0.0-beta.47 => 7.0.0-beta.47
    @storybook/react: ^7.0.0-beta.47 => 7.0.0-beta.47
    @storybook/react-webpack5: ^7.0.0-beta.47 => 7.0.0-beta.47
    @storybook/testing-library: ^0.0.14-next.1 => 0.0.14-next.1

Additional context

In my own codebase I was able to fix one particular instance of this by replacing const Default = {}; export { Default as ComponentName } in my stories file with export const Default = {}. (See Discord) I wasn't able to reproduce this here.

tmeasday commented 1 year ago

@ndelangen I think this is a bundling issue. I see two copies of the blocks package (try searching for Source2 in the below two files) in the webpack build, in files:

I think the first file is accessed when you import @storybook/blocks from an MDX file, but the first is what the DocsContainer uses. I think this means there are two SourceContainers, which means the Source block gets the wrong one (and thus it is empty).

ndelangen commented 1 year ago

@elisehein I think you likely haven't pushed all files to the remote. Your repro won't start for me.

(main) % npm install                                                                                                                                                                                                                 ~/Projects/GItHub/repros/storybook7-primary-block-code-preview

added 1037 packages, and audited 1038 packages in 37s

176 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
(main)⚡ % npm run storybook                                                                                                                                                                                                         ~/Projects/GItHub/repros/storybook7-primary-block-code-preview

> test-storybook-init@1.0.0 storybook
> storybook dev -p 6006

@storybook/cli v7.0.0-beta.47

Error: No configuration files have been found in your configDir (/Users/me/Projects/GitHub/repros/storybook7-primary-block-code-preview/.storybook).
Storybook needs "main.js" file, please add it.
    at validateConfigurationFiles (/Users/me/Projects/GitHub/repros/storybook7-primary-block-code-preview/node_modules/@storybook/core-common/dist/index.js:9:30)
(main)⚡ [1] %                                                                                                                                                                                                                       ~/Projects/GItHub/repros/storybook7-primary-block-code-preview

There is no .storybook dir..

elisehein commented 1 year ago

@ndelangen apologies, should be fixed now.

ndelangen commented 1 year ago

this is strange: vendors-node_modules_mdx-js_react_lib_index_js-node_modules_storybook_blocks_dist_index_mjs.iframe.…

because @mdx-js/react doesn't have a dependency on @storybook/block..

I was also able to reproduce this whilst ensuring there's only 1 version of the package installed.

ndelangen commented 1 year ago

I found the bug! Will open a PR soon 🎉

shilman commented 1 year ago

Ooh-la-la!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-beta.53 containing PR #21161 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb@next upgrade --prerelease
elisehein commented 1 year ago

Confirming that this is fixed in 7.0.0-rc3 when I upgrade in the reproduction repo. Unfortunately this hasn't fixed the issue for me in my actual codebase, and I'm unable to figure out what is causing it 🤔

tmeasday commented 1 year ago

@elisehein, hmm, that's frustrating! Maybe can you see if the same root issue is going on in your repo? If you open Dev tools and search for SourceContainer, do you get more than one file?

image
elisehein commented 1 year ago

@tmeasday Thanks for the tip, this is what my search results show:

CleanShot 2023-03-15 at 09 14 07@2x

It's all index.mjs from @storybook/blocks like yours but I don't know why I get that three times 🤔

This is after purging node_modules, yarn.lock, and a fresh install, the version is definitely rc3.

tmeasday commented 1 year ago

Ok, looks like it likely is the same symptom @elisehein. Can you run yarn why (or look in node_modules) and see if @storybook/blocks is installed more than once? It looks like each of those (3!) entries is an .mjs file, so if I am reading it correctly it isn't the same problem as the one we had fixed in https://github.com/storybookjs/storybook/pull/21161, even if the symptom was the same.

elisehein commented 1 year ago

@tmeasday Here's the output from yarn why

yarn why v1.22.19
[1/4] 🤔  Why do we have the module "@storybook/blocks"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "@storybook/blocks@7.0.0-rc.3"
info Has been hoisted to "@storybook/blocks"
info Reasons this module exists
   - Specified in "devDependencies"
   - Hoisted from "@storybook#addon-essentials#@storybook#addon-controls#@storybook#blocks"
   - Hoisted from "@storybook#addon-essentials#@storybook#addon-docs#@storybook#blocks"
info Disk size without dependencies: "976KB"
info Disk size with unique dependencies: "28.01MB"
info Disk size with transitive dependencies: "79.8MB"
info Number of shared dependencies: 143
✨  Done in 0.58s.
tmeasday commented 1 year ago

Hmm, I'm puzzled by that bundling then. Do you have a custom webpackFinal in main.js?

@ndelangen do you have any ideas here? TLDR: @elisehein has only a single copy of @storybook/blocks but it seems to be in 3 seperate webpack chunks.

elisehein commented 1 year ago

@tmeasday I do in fact have a custom webpackFinal, apologies, I should have included that straight away:

  webpackFinal: async (config, { configType }) => {
    config.stats = "none";

    // Needed to build Iconoir
    // See https://github.com/storybookjs/storybook/issues/16690#issuecomment-971579785
    config.module.rules.push({
      test: /\.mjs$/,
      include: /node_modules/,
      type: "javascript/auto",
    });

    return config;
  },

But removing it does not fix the code preview issue still :( And removing it doesn't change the output of yarn why.

To clarify, on beta47 code previews are working fine in auto-generated docs pages, it's only when I have a custom .mdx file and use Primary or Canvas that they break. The Primary block just shows a copy of the story in CSF3 format:

CleanShot 2023-03-17 at 10 29 08@2x

BUT trying out rc4 just now I'm seeing that code previews also break on auto-generated docs pages where they were working fine on beta47. I get the dreaded <No Display Name />, so this is a regression for me.

CleanShot 2023-03-17 at 10 33 30@2x

In custom mdx files on rc4 I still get the copy of the story definition in the code preview, same as beta47.

tmeasday commented 1 year ago

@tmeasday I do in fact have a custom webpackFinal, apologies, I should have included that straight away:

No worries, I don't think it's the problem here.

BUT trying out rc4 just now I'm seeing that code previews also break on auto-generated docs pages where they were working fine on beta47. I get the dreaded , so this is a regression for me.

The <No Display Name /> issue is something else: https://github.com/storybookjs/storybook/issues/21649, I don't think it's related to the problem you are seeing with "raw" snippets here. We'll sort it out soon but I imagine the other problem will recur.

I'm hoping @ndelangen might have some ideas as to why @storybook/blocks get rendered more than once.

ndelangen commented 1 year ago

@elisehein is this repro repo up to date with those changes? elisehein/storybook7-primary-block-code-preview

I can investigate why you seem to be getting the find the same code 3 times.

elisehein commented 1 year ago

@ndelangen unfortunately not, as upgrading to rc3 fixes it in the repro. I haven't been able to reproduce the issue in isolation, but I'm seeing it on my own codebase with both beta47 and rc3 (and 4).

ndelangen commented 1 year ago

@elisehein and you're sure that in that situation it's only installed once?

Might there be anything going on with symlinks or anything like that? Can you find out what file-paths the multiple instances are from?

elisehein commented 1 year ago

@ndelangen apologies for the delay.

I've just upgraded to the stable release of 7 and this issue is fixed for me 🥳