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

Manager: Add tags property to ComponentEntry objects #29343

Merged

Conversation

Sidnioulz
Copy link
Contributor

@Sidnioulz Sidnioulz commented Oct 12, 2024

What I did

I added a piece of logic to the code converting the story index to story hashes for use in the manager (transformStoryIndexToStoriesHash).

This extra code computes a tags property for Component entries, which is the intersection of all their stories' hashes. This enables me to display hashes in the sidebar, and this gist from a third party shows that others have sought to achieve the same thing in the past.

Having a definition of tags for component entries allows us to separate story-specific tags from component-specific tags and imo gives a clearer meaning to the tags added to a CSF meta.

Below are screenshots of what can be achieved with tags with and without this PR.

Without this PR

image

With this PR

image

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

  1. Add custom tags to any story you like in the storybook stories, e.g. change the meta in code/core/src/manager/components/panel/Panel.stories.tsx to have tags: ['myCustomTag'],
  2. Add a sidebar renderLabel function of your choice to be able to inspect the entry, or console log in code/core/src/manager/components/sidebar/Tree.tsx's Node ctor
  3. Notice the presence of the tags in the component entry

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Caution

I am not sure what the most appropriate documentation format is for this. Guidance is welcome.

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 78.7 MB 78.7 MB 0 B 0.42 0%
initSize 147 MB 147 MB 748 B -1.06 0%
diffSize 68.3 MB 68.3 MB 748 B -1.06 0%
buildSize 6.79 MB 6.79 MB 245 B 1.11 0%
buildSbAddonsSize 1.5 MB 1.5 MB 0 B -1.22 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.83 MB 1.83 MB 245 B Infinity 0%
buildSbPreviewSize 270 kB 270 kB 0 B - 0%
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.8 MB 3.8 MB 245 B 0.55 0%
buildPreviewSize 2.99 MB 2.99 MB 0 B 1.09 0%
testBuildSize 0 B 0 B 0 B - -
testBuildSbAddonsSize 0 B 0 B 0 B - -
testBuildSbCommonSize 0 B 0 B 0 B - -
testBuildSbManagerSize 0 B 0 B 0 B - -
testBuildSbPreviewSize 0 B 0 B 0 B - -
testBuildStaticSize 0 B 0 B 0 B - -
testBuildPrebuildSize 0 B 0 B 0 B - -
testBuildPreviewSize 0 B 0 B 0 B - -
name before after diff z %
createTime 16.4s 19.5s 3.1s 1.17 16%
generateTime 21.6s 18.3s -3s -353ms -2.25 🔰-18.3%
initTime 12.9s 12.5s -395ms -1.44 -3.1%
buildTime 8.4s 7.2s -1s -117ms -0.96 -15.3%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 5.8s 5.5s -230ms -1.17 -4.1%
devManagerResponsive 3.8s 3.9s 52ms -1.02 1.3%
devManagerHeaderVisible 533ms 521ms -12ms -1.1 -2.3%
devManagerIndexVisible 562ms 544ms -18ms -1.15 -3.3%
devStoryVisibleUncached 931ms 903ms -28ms -0.54 -3.1%
devStoryVisible 572ms 551ms -21ms -1.09 -3.8%
devAutodocsVisible 463ms 444ms -19ms -1.27 -4.3%
devMDXVisible 501ms 461ms -40ms -0.98 -8.7%
buildManagerHeaderVisible 480ms 445ms -35ms -1.18 -7.9%
buildManagerIndexVisible 494ms 448ms -46ms -1.21 -10.3%
buildStoryVisible 516ms 708ms 192ms 0.62 27.1%
buildAutodocsVisible 453ms 406ms -47ms -1.34 🔰-11.6%
buildMDXVisible 669ms 394ms -275ms -1.27 🔰-69.8%

Greptile Summary

This PR adds a new tags property to ComponentEntry objects, computed as the intersection of all their stories' tags, enhancing tag organization and display in the Storybook sidebar.

  • Introduced intersect function in code/core/src/manager-api/lib/intersect.ts for efficient array intersection
  • Modified transformStoryIndexToStoriesHash in code/core/src/manager-api/lib/stories.ts to compute component tags
  • Added comprehensive unit tests for intersect function in code/core/src/manager-api/tests/intersect.test.ts
  • Updated API_ComponentEntry interface in code/core/src/types/modules/api-stories.ts to include tags property
  • Added test case in code/core/src/manager-api/tests/stories.test.ts to verify component tag computation

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

5 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile

@@ -0,0 +1,14 @@
export default <T>(a: T[], b: T[]): T[] => {
// no point in intersecting if one of the input is ill-defined
if (!a || !b) {
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider using Array.isArray() for more robust type checking

}

return a.reduce((acc: T[], aValue) => {
if (b.includes(aValue)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

style: For large arrays, consider using a Set for better performance

Copy link

nx-cloud bot commented Oct 12, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit d60a9d7. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Great job!!! 🚀

@shilman shilman merged commit 7a27337 into storybookjs:next Oct 12, 2024
53 checks passed
@github-actions github-actions bot mentioned this pull request Oct 12, 2024
5 tasks
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.

2 participants