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

Storybook comes up empty with 6.4.0-alpha.20 and later (it's ok up to 6.4.0-alpha.19) #15746

Closed
dcwarwick opened this issue Aug 3, 2021 · 6 comments

Comments

@dcwarwick
Copy link

dcwarwick commented Aug 3, 2021

Describe the bug
When I update the version of @storybook/react and related @storybook packages to 6.4.0-alpha.20 or above, my storybook comes up empty. With previous versions of storybook, up to and including 6.4.0-alpha.19, it comes up fine with stories in. I wonder if there is a regression in #15399. NB our storybook is produced with a list of explicit files, not a glob.

To Reproduce
Here is a cut-down version of our repo:
https://github.com/dcwarwick/ibm-cloud-cognitive/tree/storybook-640alpha20

To reproduce:

  • clone that branch
  • run yarn and yarn:storybook
  • Storybook comes up empty ("Oh no! Your Storybook is empty.")

If the @storybook versions in /packages/core/package.json are pulled back to 6.4.0-alpha.19 or below, yarn:storybook will produce a storybook with stories in as expected.

Here is a running instance of the above branch:
https://deploy-preview-1108--ibm-cloud-cognitive.netlify.app/

Here is a running instance of the full repo using @storybook version 6.3.6:
https://ibm-cloud-cognitive.netlify.app/?path=/story/cloud-cognitive-released-aboutmodal--about-modal

System
Please paste the results of npx sb@next info here.

$ npx sb@next info

Environment Info:

  System:
    OS: macOS 11.4
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
  Binaries:
    Node: 14.16.0 - ~/.nvm/versions/node/v14.16.0/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 6.14.11 - ~/.nvm/versions/node/v14.16.0/bin/npm
  Browsers:
    Chrome: 92.0.4515.131
    Firefox: 88.0.1
    Safari: 14.1.1

Additional context
The storybook configuration files are all in /packages/core.

I've produced a cut-down version of our real-life case rather than attempting to make a minimal reproduction, as I suspect there is something significant in the way we have configured storybook and it might be difficult to recreate that from scratch.

Here is the module.exports from our main.js:

{
  core: { builder: 'webpack5' },
  addons: [
    '@storybook/addon-actions',
    '@storybook/addon-docs',
    '@storybook/addon-controls',
    '@storybook/addon-knobs',
    '@storybook/addon-storysource',
    '@storybook/addon-viewport',
    '@carbon/storybook-addon-theme/register'
  ],
  reactOptions: { strictMode: true },
  stories: [
    '/Users/dave/git/ibm-cloud-cognitive/packages/cloud-cognitive/src/components/AboutModal/AboutModal.stories.js',
    '/Users/dave/git/ibm-cloud-cognitive/packages/core/docs/getting-started.stories.mdx'
  ],
  webpackFinal: [AsyncFunction: webpackFinal]
}
@dcwarwick dcwarwick changed the title Storybook comes up empty with 6.4.0-alpha.20 and later (ok up to 6.4.0-alpha.19) Storybook comes up empty with 6.4.0-alpha.20 and later (it's ok up to 6.4.0-alpha.19) Aug 3, 2021
@dcwarwick
Copy link
Author

dcwarwick commented Aug 4, 2021

ok, @shilman I've taken a bit of a look through the code, and this definitely looks like a regression in #15399. I think the problem is in /lib/core-common/src/utils/to-require-context.ts, here:
image

  • For example, suppose one of the elements in the stories array is something like '/a/b/c.js'.
  • When the above code executes, fixedInput is NOT a glob (in this case), so in line 35 base is set to just the dirname from the fixedInput, which is the path not including the final /, i.e. '/a/b'.
  • In line 36 globFallback is set to fixedInput excluding base -- this is the filename PLUS the leading /, i.e. '/c.js'. I think this might be a mistake.
  • In line 39, regex is set to a lookahead term for the filename STILL INCLUDING the leading /, something like '^(?:\/c\.js)$'
  • In line 51, because glob DOES include a / (the leading / in '/c.js') the recursive flag is set to true, which is probably not what is intended.
  • In line 53, the following structure is returned:
{
  path: '/a/b',
  recursive: true,
  match: '^\\.\\/(?:\\/c\\.js)$'
}

and this does not produce any matches even though the file exists.

I tried changing line 36, fixedInput.substr(base.length) to fixedInput.substr(base.length+1). Now:

  • As before, fixedInput is NOT a glob, so in line 35 base is set to '/a/b'.
  • This time, in line 36 globFallback is set to 'c.js' -- i.e. NOT including the leading /.
  • In line 39, regex is set to '^(?:c\.js)$'
  • In line 51, because glob now does NOT include a / the recursive flag is set to false.
  • In line 53, the following structure is returned:
{
  path: '/a/b',
  recursive: false,
  match: '^\\.\\/(?:c\\.js)$'
}

and this works just fine.

In fact, with the above change to line 36, our whole storybook works just fine on 6.4.0-alpha.23. However, I do not feel confident enough in my understanding of all the possible paths through this glob code to propose this as a PR -- I'd much prefer someone who understands the code takes a look first! However, I hope the above helps.

@shilman
Copy link
Member

shilman commented Aug 5, 2021

@dcwarwick thanks for tracking this down!

@lkuechler any chance you can follow up on this? ☝️

@lkuechler
Copy link
Contributor

@shilman I will have a look at this today

@lkuechler
Copy link
Contributor

@shilman I have created a PR that should solve this issue. #15775

@dcwarwick thank you for the detailed analysis and the suggestion on how to solve this. Could you check if the test really matches with the issue that you where having?

@shilman
Copy link
Member

shilman commented Aug 7, 2021

Huzzah!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.4.0-alpha.25 containing PR #15775 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

@dcwarwick
Copy link
Author

dcwarwick commented Aug 7, 2021

Bzing! Thanks @lkuechler @shilman Alpha 25 works fine for our storybook now :-) and the FAST_REFRESH=true that started all this looks good too :-)

niklasmh pushed a commit to niklasmh/storybook that referenced this issue Aug 17, 2021
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

3 participants