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

Fix Storybook published messaging #920

Merged
merged 4 commits into from Feb 14, 2024
Merged

Conversation

tevanoff
Copy link
Contributor

@tevanoff tevanoff commented Feb 12, 2024

This fixes a couple of issues with the Storybook published messaging.

  1. Logging this from the publishBuild step meant that sometimes the ctx.build used to construct the message was in an early stage (like PASSED) without component and story information, resulting in a message like:
✔ Storybook published
We found undefined components with undefined stories.
  1. This is also logged from the verifyBuild step under certain conditions (like with exit-once-uploaded), resulting in duplicate messaging like:
✔ Storybook published
We found undefined components with undefined stories.

✔ Storybook published
We found 19 components with 221 stories.

How to Test

  • With the canary version, commit some changes and run builds with various flags including with and without exit-once-uploaded and only-changed.
📦 Published PR as canary version: 10.9.6--canary.920.7904727338.0

✨ Test out this PR locally via:

npm install chromatic@10.9.6--canary.920.7904727338.0
# or 
yarn add chromatic@10.9.6--canary.920.7904727338.0

@tevanoff tevanoff force-pushed the todd/sb-published-message-fix branch from 903f977 to 7890caa Compare February 12, 2024 22:41
@ethriel3695
Copy link
Contributor

@tevanoff
How can I test this?

Can I reference the Canary build and then what?
How can I trigger this use case?

@tevanoff
Copy link
Contributor Author

@tevanoff How can I test this?

Can I reference the Canary build and then what? How can I trigger this use case?

@ethriel3695 I added some info about testing to the PR message. Those two flags seemed to affect the behavior of the output in my testing.

Copy link
Contributor

@ethriel3695 ethriel3695 left a comment

Choose a reason for hiding this comment

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

@tevanoff I think this did it!
I ran the Action with the latest and saw the undefined stories output.

Now on the canary:

Collecting Storybook metadata
Collected Storybook metadata
    → ; no supported addons found
Initializing build
Initialized build
    → Build 17 initialized
Publish your built Storybook
    → Validating Storybook files
Publishing your built Storybook
    → Calculating file hashes
Publishing your built Storybook
    → Starting publish
    → [                    ] 0%
Publish complete in 9 seconds
    → Uploaded 168 files (28.73 MB)
Verifying your Storybook
    → This may take a few minutes
    → [*                   ]
⚠ TurboSnap not available for your account
To ensure your project is fully setup and baselines are properly established,
TurboSnap is not available until at least 10 builds are created from CI and one
of those builds is accepted.
ℹ Review your TurboSnap availability on the Manage screen:
https://www.chromatic.com/manage?appId=65bdd147c425b429f5968c04
✔ Storybook published
We found 14 components with [23](https://github.com/enhanced-dev/blog-tutorial-remix/actions/runs/7880523801/job/21502694657#step:6:24) stories.
ℹ View your Storybook at https://65bdd147c4[25](https://github.com/enhanced-dev/blog-tutorial-remix/actions/runs/7880523801/job/21502694657#step:6:26)b4[29](https://github.com/enhanced-dev/blog-tutorial-remix/actions/runs/7880523801/job/21502694657#step:6:30)f5968c04-bqslfbpzzu.chromatic.com/
Screenshot 2024-02-12 at 6 45 21 PM

It looks like this is what we want.

if (build) {
// `ctx.build` is initialized and overwritten in many ways, which means that
// this can be any kind of build without component and stories information,
// like PASSED builds, for example
Copy link
Member

Choose a reason for hiding this comment

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

I don't see why PASSED builds would not have component and story information. They were tested after all.

@tevanoff tevanoff added release Auto: Create a `latest` release when merged patch Auto: Increment the patch version when merged labels Feb 14, 2024
@tevanoff tevanoff added this pull request to the merge queue Feb 14, 2024
Merged via the queue into main with commit 53414f4 Feb 14, 2024
20 of 24 checks passed
@tevanoff tevanoff deleted the todd/sb-published-message-fix branch February 14, 2024 17:00
@ghengeveld
Copy link
Member

🚀 PR was released in v10.9.5 🚀

@ghengeveld ghengeveld added the released Verdict: This issue/pull request has been released label Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Auto: Increment the patch version when merged release Auto: Create a `latest` release when merged released Verdict: This issue/pull request has been released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants