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

CSF3: Generate default titles based on file path #15376

Merged
merged 6 commits into from
Jun 29, 2021
Merged

Conversation

shilman
Copy link
Member

@shilman shilman commented Jun 26, 2021

Issue: #11186

What I did

Generate default titles based on file path. With this PR the stories field in .storybook/main.js can take on three possible values:

module.exports = {
  stories: = [
    '../packages/foo/**/*.stories.mdx', // glob
    { directory: '../packages/baz', titlePrefix: 'Baz Components', files: '*.stories.*' }, // specifier
    '../packages/bar', // directory
  ]
}
  • A file glob that matches story files => same as CSF2
  • A specifier object containing:
    • A directory that contains story files somewhere beneath to
    • The "titlePrefix" that the directory maps to (optional, defaults to '')
    • A file selector glob (optional, defaults to '*.stories.@(mdx|tsx|ts|jsx|js)')
  • A path to a directory which is equivalent to the specifier object

If you use either of the second two objects and have the features.previewCsfV3 flag set, you no longer need to fill in the default.title annotation in CSF.

What I need

This currently fails in static builds of Storybook because the fileName is rewritten as a number rather than the actual file path.

self-merging @ndelangen @tmeasday with the rename of root to titlePrefix per Norbert's feedback

How to test

  • See examples/react-ts/main.ts / AccountForm.stories.tsx
  • See attached tests

@nx-cloud
Copy link

nx-cloud bot commented Jun 26, 2021

Nx Cloud Report

CI ran the following commands for commit a1091a4. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status Command
#000000 nx run-many --target=prepare --all --parallel --max-parallel=15

Sent with 💌 from NxCloud.

@shilman shilman marked this pull request as draft June 26, 2021 12:30
Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this look good, but I guess the static build thing is a bit of a deal breaker.

Similarly, non webpack builders might have problems with a feature that relies on parameters.fileName.

To me, it feels like this is something that will work well when we are generating stories.json.

lib/core-client/src/preview/autoTitle.ts Outdated Show resolved Hide resolved
lib/core-client/src/preview/autoTitle.test.ts Outdated Show resolved Hide resolved
@bebraw
Copy link
Contributor

bebraw commented Jun 28, 2021

This currently fails in static builds of Storybook because the fileName is rewritten as a number rather than the actual file path.

What's the easiest way to reproduce this behavior?

It sounds like there's some mismatch between dev/prod configurations assuming it's a webpack problem. Looking at the code, it's around line const title = autoTitle(defaultExport, fileName) where I would drop a debugger; statement to see what's going on.

@shilman
Copy link
Member Author

shilman commented Jun 29, 2021

@bebraw sorry I missed your comment. To reproduce the problem:

cd examples/react-ts
yarn storybook # <-- works
yarn build-storybook # <-- fails

@bebraw
Copy link
Contributor

bebraw commented Jun 29, 2021

I can reproduce the issue now and I have a rough idea how you might be able to fix it. Here are a couple of screenshots to illustrate the problem.

yarn storybook

Screen Shot 2021-06-29 at 12 18 48

yarn build-storybook

Screen Shot 2021-06-29 at 12 18 42

fileName expected by autoTitleFromEntry isn't what you expect and during a build it seems to refer to a numeric module id.

I think one way to fix this would be to change webpack configuration in the latter case to include optimization.moduleIds = 'named'. When webpack runs in production mode, it uses numbers for module ids and going with named would change the behavior here as well so you get what you expect.

The side effect of doing this is that it will increase the bundle size slightly since names are longer than numbers but it's possible this doesn't make a big difference.

@shilman
Copy link
Member Author

shilman commented Jun 29, 2021

Thanks for the tip @bebraw -- works great!!! 🙏

@shilman shilman marked this pull request as ready for review June 29, 2021 13:43
@shilman shilman merged commit 928d2b1 into next Jun 29, 2021
@shilman shilman deleted the tech/csf3-default-titles branch June 29, 2021 15:43
@shilman shilman added this to the 6.4 PRs milestone Jul 28, 2021
@marcus13371337
Copy link

Hello @shilman ,

Great with this new feature, will make a lot of things cleaner in our monorepo setup. ❤️

However, I can't see the titlePrefix I'm providing anywhere in the UI. What am I doing wrong? Tried with --no-manager-cache

@marcus13371337
Copy link

Found the issue, apparently, you can't specify a manual title if you use the titlePrefix? Just reading the parameter name, I would expected that what I'm passing to the titlePrefix would be the prefix to any title no matter if I specified manually or if it was auto generated from the path

@shilman
Copy link
Member Author

shilman commented Sep 9, 2021

@marcus13371337 If you specify the title, it uses that title verbatim. We'll document all this as part of of the 6.4 release (cc @jonniebigodes)

@spacecakes
Copy link

spacecakes commented Apr 22, 2022

Love this feature as it has allowed me to get rid of hundreds of lines of code. Just wondering if there's a way to get Storybook to flatten the sidebar folder structure when the directory name is the same as the story file name, so we can avoid having this redundant nesting that happens when the component and story are in a folder with the same name:

Screenshot 2022-04-22 at 14 39 11

Is there any way for me to configure that?

@shilman
Copy link
Member Author

shilman commented Apr 22, 2022

@spacecakes fixed in 6.5

Try upgrading to the latest prerelease:

npx sb@next upgrade --prerelease

@spacecakes
Copy link

@spacecakes fixed in 6.5

Wow, that is convenient. Thank you very much!

The only issue I noticed with this is that story hoisting does not work for multi-word component names, because, say, CheckboxGroup.stories.js is gets processed into Checkbox Group for the story name, while the path remains as-is so they no longer match. So we get this:

Screenshot 2022-04-22 at 18 58 16

Not a dealbreaker by any means and I realise it's prelease software, but it would be nice if this worked to avoid nesting single-story components with this setup.

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

Successfully merging this pull request may close these issues.

None yet

5 participants