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

Core: Fix WebProjectAnnotations export in preview-web for back-compat #19048

Merged
merged 3 commits into from Aug 31, 2022

Conversation

clintandrewhall
Copy link
Contributor

@clintandrewhall clintandrewhall commented Aug 29, 2022

Issue: storybookjs/testing-react#106
Discussion: #18531

What I did

I added a type export to resolve. It was unclear if this, too, would affect builder-vite, (I can't imagine it would), but I added the export immediately following for visibility and completeness.

Hope this helps!

How to test

  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

@IanVS
Copy link
Member

IanVS commented Aug 30, 2022

@shilman this is what we talked about in discord. I'm not sure what's needed to get CI running/passing. I attempted to start a discussion in https://discord.com/channels/486522875931656193/839297503446695956/1013924170918269008.

@shilman shilman added bug patch:yes Bugfix & documentation PR that need to be picked to main branch core typescript labels Aug 30, 2022
@shilman
Copy link
Member

shilman commented Aug 30, 2022

@tmeasday the background here is that a type got moved as a breaking change in 6.5. This re-exports the type to fix the break, which affects the test runner specifically.

Questions:

  • Are you good with the change?
  • How about for 7.0?

@shilman shilman changed the title Add type export for storybookjs/testing-react#106 Core: Fix WebProjectAnnotations export in preview-web for back-compat Aug 30, 2022
@clintandrewhall
Copy link
Contributor Author

I'm going to test this fix this afternoon in our PR, and post back here, (in case there's something else blocking).

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.

Thanks so much @clintandrewhall !! 🙏

@clintandrewhall
Copy link
Contributor Author

@shilman @IanVS I don't see any further type issues locally. I can't test in our CI, but I have fairly high confidence in this fix. Should be good to go!

@tmeasday
Copy link
Member

Fine by me, to re-export it. I'm not totally sure I recall why it moved in the first place, but either way re-exporting makes sense to avoid breaking in any case.

@BHA8273
Copy link

BHA8273 commented Aug 31, 2022 via email

@shilman shilman merged commit e9e10ee into storybookjs:next Aug 31, 2022
@sipkoepp
Copy link

sipkoepp commented Sep 1, 2022

@shilman you fixed this in the latest alpha release. Is it possible to fix this for a stable version?

@IanVS
Copy link
Member

IanVS commented Sep 1, 2022

It has the patch label, which means that when it comes time to cut another 6.5 patch release, it will get picked into it.

@shilman shilman added the patch:done Patch/release PRs already cherry-picked to main/release branch label Sep 7, 2022
shilman added a commit that referenced this pull request Sep 7, 2022
Core: Fix WebProjectAnnotations export in preview-web for back-compat
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug core patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants