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

[Bug] Story-level tags are ignored in Vitest integration #261

Open
JReinhold opened this issue Jan 15, 2025 · 3 comments · May be fixed by #266
Open

[Bug] Story-level tags are ignored in Vitest integration #261

JReinhold opened this issue Jan 15, 2025 · 3 comments · May be fixed by #266
Assignees
Labels
bug Something isn't working next Related to the next version

Comments

@JReinhold
Copy link
Collaborator

Describe the bug

When using the new Vitest integration via @storybook/experimental-addon-test, the way to skip specific stories from being tested is by removing the 'test' tag. This is done by setting '!test' on a story:

<Story name="My Story" tags={['!test']} />

Tags work fine in Storybook UI because the Svelte CSF indexer parses them via static analysis, but the indexer doesn't run in Vitest mode (it doesn't need to).

Instead, the Vitest plugin from @storybook/experimental-addon-test uses the CsfFile construct to statically analyse the (compiled) CSF file. But because tags aren't part of the compiled output of Svelte CSF, they aren't detected during static analysis.

This is only a problem for story-level tags, meta or preview level tags are parsed correctly.

Steps to reproduce the behavior

  1. Have a basic Svelte CSF setup and add the addon with npx storybook@next add @storybook/experimental-addon-test@next
  2. Run Vitest with something like npm run test --project storybook
  3. See all stories testing
  4. Add a '!test' tag to a story
  5. See that it is not skipped, but it should be

Additional context

CsfFile parses tags here: https://github.com/storybookjs/storybook/blob/next/code/core/src/csf-tools/CsfFile.ts/#L626-L631
And here are the tests that showcases what it supports: https://github.com/storybookjs/storybook/blob/next/code/core/src/csf-tools/CsfFile.test.ts/#L1278-L1375

The solution is to make sure tags are part of the compiled Svelte CSF.

Today we have:

const __stories = createRuntimeStories(RequiredSnippet_stories, meta);

export default meta;

export const SomeStory = __stories["SomeStory"];
export const OtherStory = __stories["OtherStory"];

We could change that to one of the three:

const __stories = createRuntimeStories(RequiredSnippet_stories, meta);

export const SomeStory = __stories["SomeStory"];
SomeStory.tags = ["some-tag", "other-tag"];

OR:

const __stories = createRuntimeStories(RequiredSnippet_stories, meta);

export const SomeStory = {
  ...__stories["SomeStory"],
  tags: ["some-tag", "other-tag"]
};

OR:

const __stories = createRuntimeStories(RequiredSnippet_stories, meta);

const SomeStoryTags = ["some-tag", "other-tag"];

export const SomeStory = {
  ...__stories["SomeStory"],
  tags: SomeStoryTags
};

All would work, we should just do what is simplest to implement.

@JReinhold JReinhold added bug Something isn't working next Related to the next version labels Jan 15, 2025
@xeho91 xeho91 self-assigned this Jan 17, 2025
@xeho91
Copy link
Collaborator

xeho91 commented Jan 17, 2025

Is it only tags that needs to be static, or do you predict that there might be other properties?

@JReinhold
Copy link
Collaborator Author

Is it only tags that needs to be static, or do you predict that there might be other properties?

Good question. Initially I wonder if extracting the play()-function would make sense too, because we do that in regular CSF and add a 'play-fn' tag to all stories with such a function. However I'm not sure it's doable, the play-function could be referencing instance-level variables like in #257, which wouldn't be extractable to the story export.

@xeho91
Copy link
Collaborator

xeho91 commented Jan 22, 2025

Initially I wonder if extracting the play()-function would make sense too, because we do that in regular CSF and add a 'play-fn' tag to all stories with such a function.

Would manually appending the 'play-fn' tag based on the existence of play attribute in defined <Story /> help the case?

However I'm not sure it's doable, the play-function could be referencing instance-level variables like in #257, which wouldn't be extractable to the story export.

Ah yes, that's a problem. I can think of two solutions for now:

  1. A simple, but also radical one - build a mindset to never define the instance tag (throw an error) inside the *.stories.svelte. I can't think of what could be a good use case to keep it. Stuff like setTemplate(template) is about to be replaced with render in the future.
    Other than this case, if the user wants to use $state() rune, it can be defined in the module tag.
    $effect() as well, I think? Regardless, I can't think of a good reason to use this one in *.stories.svelte file either.

  2. A bit complex (not sure to what depth):

    1. traverse AST of the play function body to collect Identifiers nodes
    2. throw error if any of the identifiers are declared within instance script tag

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working next Related to the next version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants