storybookjs / storybook

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

6.4 Breaking Chromatic excludeStories check with storyStoreV7 #16951

Open AndrewLeedham opened 2 years ago

AndrewLeedham commented 2 years ago

Describe the bug In Storybook 6.3 we are using the following excludeStories expression:

import isChromatic from 'chromatic';

export default {
  component: Component,
  excludeStories: isChromatic() ? undefined : /^Chromatic/;
};

This no longer works in 6.4 when using the experimental storyStoreV7 feature. I can only assume it is related to https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#string-literal-titles, is metadata being pulled with static analysis now?

To Reproduce I created a repro here: https://github.com/AndrewLeedham/storybook-exclude-stories-repro/tree/master I can't publish it to Chromatic because the build fails because of this issue.

System

System:
    OS: macOS 11.2.3
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
  Binaries:
    Node: 14.18.1 - ~/.nvm/versions/node/v14.18.1/bin/node
    Yarn: 3.1.1 - /usr/local/bin/yarn
    npm: 6.14.15 - ~/.nvm/versions/node/v14.18.1/bin/npm
  Browsers:
    Chrome: 96.0.4664.93
    Edge: 96.0.1054.34
    Firefox: 94.0.1
    Safari: 14.0.3
  npmPackages:
    @storybook/addon-actions: ^6.5.0-alpha.1 => 6.5.0-alpha.1 
    @storybook/addon-docs: ^6.5.0-alpha.1 => 6.5.0-alpha.1 
    @storybook/addon-essentials: ^6.5.0-alpha.1 => 6.5.0-alpha.1 
    @storybook/addon-links: ^6.5.0-alpha.1 => 6.5.0-alpha.1 
    @storybook/react: ^6.5.0-alpha.1 => 6.5.0-alpha.1
shilman commented 2 years ago

@AndrewLeedham Yes, we're doing static analysis on the story files in storyStoreV7 mode. It's a breaking change, which is why it's behind an opt-in feature flag.

This is an interesting pattern -- basically a workaround for https://github.com/storybookjs/storybook/issues/9209

If we added that feature (API etc still TBD) would that be a suitable solution for you? cc @tmeasday

AndrewLeedham commented 2 years ago

@shilman Presumably the static analysis is to speed up discovery for the sidebar, so you don't have to process all the stories on load? Curious what fields it looks at, assuming include/exclude and title?

Being able to hide things from the sidebar seems like it would resolve my issue yes, as Chromatic will still be able to find the stories but it won't clog things up on the actual storybook :)

shilman commented 2 years ago

@AndrewLeedham Yes, you can see the test cases here: https://github.com/storybookjs/storybook/blob/next/lib/csf-tools/src/CsfFile.test.ts

For the moment I think the workaround is to not use storyStoreV7. Longer term, we'll need to come up with a better solution for this, possibly creating a tag system for stories so they can be used selectively in canvas / docs / tests.

AndrewLeedham commented 2 years ago

@shilman Yeah my plan was to not use storyStoreV7 for now. Thanks :)

stale[bot] commented 2 years ago

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

AndrewLeedham commented 2 years ago

@shilman could another option be a global excludeStories option, meaning it could be run as part of main.js once, rather than statically analysed per story.

xgebi commented 11 months ago

Is there any progress on this issue? I was able to find a workaround but it would be better if there was a documented way.