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

Support Storybook 7.0 play function exceptions #190

Merged
merged 3 commits into from Sep 16, 2022

Conversation

tmeasday
Copy link
Member

@tmeasday tmeasday commented Sep 8, 2022

See storybookjs/storybook#19143

We need to ensure that stories that do not emit an STORY_THREW_EXCEPTION on play function error are still handled. (SBs with hidePlayFunctionExceptions set)

To test:

  1. Run a SB7.0 sandbox with interactions installed and a failing play function
  2. ./bin/test-storybook.js --url http://localhost:6006 -i example-button.test.js
  3. Do the above with interactions removed from the SB.

@yannbf I'm not how to automate the above testing, perhaps you can give some guidance, if it's possible?

📦 Published PR as canary version: 0.7.2--canary.190.e69606f.0

✨ Test out this PR locally via:

npm install @storybook/test-runner@0.7.2--canary.190.e69606f.0
# or 
yarn add @storybook/test-runner@0.7.2--canary.190.e69606f.0

Version

Published prerelease version: v0.7.2-next.0

Changelog

🐛 Bug Fix

Authors: 3

@shilman shilman requested a review from yannbf September 15, 2022 15:23
@yannbf
Copy link
Member

yannbf commented Sep 15, 2022

I'll take a look at this soon, but first, are there more changes required here? Given the new parameter that could/should be set in case it's not? @tmeasday @shilman

@tmeasday
Copy link
Member Author

tmeasday commented Sep 16, 2022

@yannbf, nope this should do it. There are two things that can happen from the SB side:

  1. They have throwPlayFunctionExceptions set (the default): the SB will emit a PLAY_FUNCTION_THREW_EXCEPTION and and a STORY_THREW_EXCEPTION.

  2. They have it set to false (using the interactions addon): the SB will emit a PLAY_FUNCTION_EXCEPTION and that's it.

Either way as long as we listen to both events we should be fine. The breaking change here is in case 2. where the STORY_THREW_EXCEPTION is no longer emitted.

@yannbf
Copy link
Member

yannbf commented Sep 16, 2022

@yannbf, nope this should do it. There are two things that can happen from the SB side:

  1. They have throwPlayFunctionExceptions set (the default): the SB will emit a PLAY_FUNCTION_THREW_EXCEPTION and and a STORY_THREW_EXCEPTION.
  2. They have it set to false (using the interactions addon): the SB will emit a PLAY_FUNCTION_EXCEPTION and that's it.

Either way as long as we listen to both events we should be fine.

Got it! I'll take a look at it tomorrow and check regarding the tests

@yannbf yannbf added the patch Increment the patch version when merged label Sep 16, 2022
Copy link
Member

@yannbf yannbf left a comment

Choose a reason for hiding this comment

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

Thanks @tmeasday !!

@yannbf yannbf changed the title Ensure that we handle SBs with hidePlayFunctionExceptions set Support Storybook 7.0 play function exceptions Sep 16, 2022
@yannbf
Copy link
Member

yannbf commented Sep 16, 2022

@yannbf I'm not how to automate the above testing, perhaps you can give some guidance, if it's possible?

This test is already automated via the nightly check!

@yannbf yannbf merged commit a61bb36 into next Sep 16, 2022
@yannbf yannbf mentioned this pull request Sep 16, 2022
@github-actions
Copy link

🚀 PR was released in v0.7.2 🚀

@yannbf yannbf deleted the tom/sb-722-render-play-function-exceptions-in-the branch November 18, 2022 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Increment the patch version when merged released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants