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: Ensure we don't reset WebPreview if calling start() in v7 mode #16475

Merged
merged 5 commits into from
Oct 26, 2021

Conversation

tmeasday
Copy link
Member

Issue: #16455

The problem was that when you import from @storybook/angular it calls start() (as do all other frameworks), which created an old, non-v7 StoryStore and assigned it to __STORYBOOK_STORY_STORE__, then awaited a configure() call (that never came....)

What I did

  • Ensure start() is a no-op in v7 mode
  • Return an API that throws errors

Questions

Should we make some of that API work (i.e. api.raw() etc?)

How to test

See example in issue.

@tmeasday tmeasday requested a review from shilman October 26, 2021 05:20
@nx-cloud
Copy link

nx-cloud bot commented Oct 26, 2021

Nx Cloud Report

CI ran the following commands for commit 894fd20. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status Command
#000000 nx run-many --target=prepare --all --parallel --max-parallel=15

Sent with 💌 from NxCloud.

@tmeasday tmeasday added the bug label Oct 26, 2021
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.

Sorry, I'm missing how this is connected to the bug.

  1. I don't see any imports from @storybook/angular in his code
  2. Is this the line that's ultimately failing?

https:/storybookjs/storybook/blob/next/addons/a11y/src/a11yRunner.ts#L72

@tmeasday
Copy link
Member Author

tmeasday commented Oct 26, 2021

I don't see any imports from @storybook/angular in his code

https:/literalpie/sb-ng-performance/blob/f6539c5fbe7338a71a437c4514e89b6cef314b2b/src/stories/Header.stories.ts#L1

Is this the line that's ultimately failing?

Yeah, correct. The second story store that is created is never initialized (as configure() is never called) and so loadStory() hangs forever waiting on the initializationPromise.

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 for the clarification @tmeasday. Fixed a couple bugs with the API signature 😉

@shilman shilman added this to the 6.4 PRs milestone Oct 26, 2021
@shilman shilman changed the title Ensure we don't reset WebPreview if calling start() in v7 mode Core: Ensure we don't reset WebPreview if calling start() in v7 mode Oct 26, 2021
@shilman shilman merged commit 7e6bf86 into next Oct 26, 2021
@shilman shilman deleted the tom/ch-661-addon-a11y-initializing-forever branch October 26, 2021 16:34
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