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

v5: Fix tags being ignored #206

Merged
merged 2 commits into from
Aug 27, 2024
Merged

v5: Fix tags being ignored #206

merged 2 commits into from
Aug 27, 2024

Conversation

JReinhold
Copy link
Collaborator

@JReinhold JReinhold commented Aug 27, 2024

Fixes #200

We can't and shouldn't "precombine" tags in the indexer, we should just pass them in, making sure they are in the right order. Storybook will internally combine the tags correctly, with the project-level tags.

Consider the following example:

  • project-level: ['dev', 'test'] (this is the default)
  • meta-level: ['!dev', '!test', '!autodocs'] (disable everything)
  • story-level: ['test', 'autodocs'] (enable tests and docs for this specific story, still keeping sidebar disabled)

If the indexer combines the meta and story tags with combineTags(), they'll end up as ['test', 'autodocs'] ('!dev' will be omitted completely, because it's not enabled). Then this gets passed to Storybook, which combines them with project-level tags, resulting in ['dev', 'test', 'autodocs']. This is wrong, 'dev' should not be there as it was disabled at the meta-level, but Storybook doesn't know that because the indexer already omitted '!dev'.

If we instead don't combine anything and let Storybook do it with the project level tags, it will get the following series:

['dev', 'test', '!dev', '!test', '!autodocs', 'test', 'autodocs']. Parsing these sequentially will yield the correct result: ['test', 'autodocs']

📦 Published PR as canary version: 4.1.7--canary.206.e4feeae.0

✨ Test out this PR locally via:

npm install @storybook/addon-svelte-csf@4.1.7--canary.206.e4feeae.0
# or 
yarn add @storybook/addon-svelte-csf@4.1.7--canary.206.e4feeae.0

Version

Published prerelease version: v5.0.0-next.3

Changelog

💥 Breaking Change

🐛 Bug Fix

Authors: 4

@JReinhold JReinhold marked this pull request as ready for review August 27, 2024 07:09
@JReinhold JReinhold requested a review from xeho91 August 27, 2024 07:09
@JReinhold JReinhold self-assigned this Aug 27, 2024
Copy link
Collaborator

@xeho91 xeho91 left a comment

Choose a reason for hiding this comment

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

Ahhh, I see. My misuse of combineTags was the culprit!
Thanks for explaining it so thoroughly 🙏

@JReinhold JReinhold added bug Something isn't working patch Increment the patch version when merged prerelease This change is available in a prerelease. labels Aug 27, 2024
@JReinhold JReinhold merged commit e0ebaf4 into next Aug 27, 2024
6 of 7 checks passed
@JReinhold JReinhold deleted the fix-ignored-tags branch August 27, 2024 08:58
@shilman
Copy link
Member

shilman commented Aug 27, 2024

🚀 PR was released in v5.0.0-next.3 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working patch Increment the patch version when merged prerelease This change is available in a prerelease.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants