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

Ensure if a docs render is torndown during preparation, it throws #19071

Merged
merged 4 commits into from
Sep 2, 2022

Conversation

tmeasday
Copy link
Member

@tmeasday tmeasday commented Sep 1, 2022

Issue: #19015

Implementing #17599 for DocsRenders

What I did

Basically mirrored the behaviour of a story render in a docs render. The contract is, if you call render.prepare() and then render.teardown() before it's done, the prepare should reject with a PREPARE_ABORTED.

How to test

  • See integration tests in XRender.test.ts
  • See integration tests in PreviewWeb.test.ts
  • Run the react-vite/default-js sandbox and browser to http://localhost:6006. Previously it would error about 50% of the time on my machine.

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.

Mostly LGTM. I'm not sure what's going on with those jest changes tho

code/lib/preview-web/src/PreviewWeb.test.ts Outdated Show resolved Hide resolved
@shilman shilman changed the title Ensure if a docs render is torndown during preparation, it throws. Ensure if a docs render is torndown during preparation, it throws Sep 2, 2022
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.

LGTM!

NOTE: CI is failing because this is based on an old version of next and we removed one of the templates due to test flake. Might revisit the implementation of that, but I think this run is OK for now.

@@ -1,5 +1,5 @@
/// <reference types="@types/jest" />;

import { jest, jest as mockJest, it, describe, beforeEach, afterEach, expect } from '@jest/globals';
Copy link
Member

Choose a reason for hiding this comment

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

do we need this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Strictly no, but I prefer it, rather than having global symbols available.

@tmeasday tmeasday merged commit 5407c0c into next Sep 2, 2022
@tmeasday tmeasday deleted the tom/sb-699-sb19015-70-vite-storybook-crashes-when branch September 2, 2022 04:26
@shilman shilman mentioned this pull request Oct 9, 2022
1 task
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