Skip to content
This repository has been archived by the owner on Jan 23, 2024. It is now read-only.

Make waitFor interceptable and don't override interval/timeout #23

Merged
merged 2 commits into from
Dec 12, 2022

Conversation

ghengeveld
Copy link
Member

@ghengeveld ghengeveld commented Jun 15, 2022

Works in tandem with storybookjs/storybook#18460

Change Type

Indicate the type of change your pull request is:

  • maintenance
  • documentation
  • patch
  • minor
  • major
📦 Published PR as canary version: 0.0.14--canary.23.51ebb1b.0

✨ Test out this PR locally via:

npm install @storybook/testing-library@0.0.14--canary.23.51ebb1b.0
# or 
yarn add @storybook/testing-library@0.0.14--canary.23.51ebb1b.0

Version

Published prerelease version: v0.0.14-next.1

Changelog

🐛 Bug Fix

Authors: 2

@ghengeveld ghengeveld requested a review from yannbf June 15, 2022 13:07
@ghengeveld ghengeveld changed the title Make waitFor interceptable and don't override interval and timeout while debugging Make waitFor interceptable and don't override interval/timeout while debugging Jun 15, 2022
@ghengeveld ghengeveld changed the title Make waitFor interceptable and don't override interval/timeout while debugging Make waitFor interceptable and don't override interval/timeout Jun 15, 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.

Great stuff @ghengeveld ! What does this mean for users that might be in a scenario with Storybook 6.4 and the new version of this package?

@yannbf
Copy link
Member

yannbf commented Jun 16, 2022

Nevermind I got the answer. For Storybook 6.4 users using this fix:

For situations where steps pass, it will surface waitFor, which is nice:

image


But in a failure scenario, there is a big difference:

Before:
image

After:
image

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

IanVS commented Jun 16, 2022

Is it possible that this will fix #9? And maybe even #8, or is that too optimistic of me?

@ghengeveld
Copy link
Member Author

ghengeveld commented Jun 16, 2022

Is it possible that this will fix #9? And maybe even #8, or is that too optimistic of me?

There's a good chance this will address #9 because we've gotten rid of "forwardedExceptions" which sometimes led to weird behavior. Give it a try!

For #8 at the very least this will log any exception before throwing the ignoredException, so you'll be able to see the original error in the browser console. I'll also revisit storybookjs/storybook#16592 which is related.

@IanVS
Copy link
Member

IanVS commented Jul 27, 2022

Hi, I'm excited for this change, is it waiting for something, or maybe it can be merged?

@yannbf
Copy link
Member

yannbf commented Jul 27, 2022

Hi, I'm excited for this change, is it waiting for something, or maybe it can be merged?

It's complicated. This fix is tied to a fix that currently only lives in storybook 7. If this change was merged, it'd bring a regression for 6.5 users (as I mentioned above). What I'm hoping for is to patch the interaction fix back to Storybook 6.5, and then everything would become much simpler (though could still impact 6.4 users). Cc @shilman

We're currently using a release of this PR in 0.0.14-next.0 in the storybook monorepo :s

@IanVS
Copy link
Member

IanVS commented Dec 7, 2022

As we approach 7.0 beta, whats the plan for this PR?

@yannbf yannbf changed the base branch from main to next December 12, 2022 11:18
@yannbf yannbf merged commit b071c0a into next Dec 12, 2022
@yannbf yannbf deleted the ghengeveld/ap-1950-fix-waitfor-behavior-while-debugging branch December 12, 2022 11:41
@github-actions github-actions bot added the prerelease This change is available in a prerelease. label Dec 13, 2022
@github-actions github-actions bot mentioned this pull request Mar 31, 2023
@github-actions
Copy link

github-actions bot commented Apr 3, 2023

🚀 PR was released in v0.1.0 🚀

@github-actions github-actions bot added released This issue/pull request has been released. and removed prerelease This change is available in a prerelease. labels Apr 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
patch Increment the patch version when merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants