From 05e51a1adee0567ca4114338e01239b84af80156 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 1 Sep 2022 16:56:28 +1000 Subject: [PATCH] Ensure if a docs render is torndown during preparation, it throws. Implementing https://github.com/storybookjs/storybook/pull/17599 for DocsRenderrs --- code/lib/preview-web/src/PreviewWeb.test.ts | 188 ++++++++++++++---- code/lib/preview-web/src/PreviewWeb.tsx | 3 +- code/lib/preview-web/src/render/Render.ts | 2 + .../src/render/StandaloneDocsRender.ts | 14 +- .../lib/preview-web/src/render/StoryRender.ts | 6 +- .../src/render/TemplateDocsRender.ts | 14 +- 6 files changed, 171 insertions(+), 56 deletions(-) diff --git a/code/lib/preview-web/src/PreviewWeb.test.ts b/code/lib/preview-web/src/PreviewWeb.test.ts index 56b69d95d630..be0860f90299 100644 --- a/code/lib/preview-web/src/PreviewWeb.test.ts +++ b/code/lib/preview-web/src/PreviewWeb.test.ts @@ -1,3 +1,4 @@ +import { jest, jest as mockJest, it, describe, beforeEach, afterEach, expect } from '@jest/globals'; import global from 'global'; import merge from 'lodash/merge'; import { @@ -29,8 +30,8 @@ import { logger } from '@storybook/client-logger'; import { addons, mockChannel as createMockChannel } from '@storybook/addons'; import type { AnyFramework } from '@storybook/csf'; import type { ModuleImportFn, WebProjectAnnotations } from '@storybook/store'; -import { expect } from '@jest/globals'; import { mocked } from 'ts-jest/utils'; +import jestMock from 'jest-mock'; import { PreviewWeb } from './PreviewWeb'; import { @@ -58,8 +59,8 @@ const mockStoryIndex = jest.fn(() => storyIndex); let mockFetchResult; jest.mock('global', () => ({ - ...(jest.requireActual('global') as any), - history: { replaceState: jest.fn() }, + ...(mockJest.requireActual('global') as any), + history: { replaceState: mockJest.fn() }, document: { location: { pathname: 'pathname', @@ -68,7 +69,7 @@ jest.mock('global', () => ({ }, window: { location: { - reload: jest.fn(), + reload: mockJest.fn(), }, }, FEATURES: { @@ -132,7 +133,7 @@ beforeEach(() => { projectAnnotations.render.mockClear(); projectAnnotations.decorators[0].mockClear(); docsRenderer.render.mockClear(); - (logger.warn as jest.Mock).mockClear(); + (logger.warn as jestMock.Mock).mockClear(); mockStoryIndex.mockReset().mockReturnValue(storyIndex); addons.setChannel(mockChannel as any); @@ -347,8 +348,12 @@ describe('PreviewWeb', () => { }); describe('after selection changes', () => { - beforeEach(() => jest.useFakeTimers()); - afterEach(() => jest.useRealTimers()); + beforeEach(() => { + jest.useFakeTimers(); + }); + afterEach(() => { + jest.useRealTimers(); + }); it('DOES NOT try again if CSF file changes', async () => { document.location.search = '?id=component-one--missing'; @@ -1561,49 +1566,140 @@ describe('PreviewWeb', () => { expect(teardownRenderToDOM).not.toHaveBeenCalled(); }); - // For https://github.com/storybookjs/storybook/issues/17214 - it('does NOT render a second time if preparing', async () => { - document.location.search = '?id=component-one--a'; + describe('while preparing', () => { + // For https://github.com/storybookjs/storybook/issues/17214 + it('does NOT render a second time in story mode', async () => { + document.location.search = '?id=component-one--a'; - const [gate, openGate] = createGate(); - const [importedGate, openImportedGate] = createGate(); - importFn - .mockImplementationOnce(async (...args) => { - await gate; - return importFn(...args); - }) - .mockImplementationOnce(async (...args) => { - // The second time we `import()` we open the "imported" gate - openImportedGate(); - await gate; - return importFn(...args); + const [gate, openGate] = createGate(); + const [importedGate, openImportedGate] = createGate(); + importFn + .mockImplementationOnce(async (...args) => { + await gate; + return importFn(...args); + }) + .mockImplementationOnce(async (...args) => { + // The second time we `import()` we open the "imported" gate + openImportedGate(); + await gate; + return importFn(...args); + }); + + const preview = new PreviewWeb(); + // We can't wait for the initialize function, as it waits for `renderSelection()` + // which prepares, but it does emit `CURRENT_STORY_WAS_SET` right before that + preview.initialize({ importFn, getProjectAnnotations }); + await waitForEvents([CURRENT_STORY_WAS_SET]); + + mockChannel.emit.mockClear(); + projectAnnotations.renderToDOM.mockClear(); + emitter.emit(SET_CURRENT_STORY, { + storyId: 'component-one--a', + viewMode: 'story', }); + await importedGate; + // We are blocking import so this won't render yet + expect(projectAnnotations.renderToDOM).not.toHaveBeenCalled(); - const preview = new PreviewWeb(); - // We can't wait for the initialize function, as it waits for `renderSelection()` - // which prepares, but it does emit `CURRENT_STORY_WAS_SET` right before that - preview.initialize({ importFn, getProjectAnnotations }); - await waitForEvents([CURRENT_STORY_WAS_SET]); + mockChannel.emit.mockClear(); + openGate(); + await waitForRender(); - mockChannel.emit.mockClear(); - projectAnnotations.renderToDOM.mockClear(); - emitter.emit(SET_CURRENT_STORY, { - storyId: 'component-one--a', - viewMode: 'story', + // We should only render *once* + expect(projectAnnotations.renderToDOM).toHaveBeenCalledTimes(1); + + // We should not show an error either + expect(preview.view.showErrorDisplay).not.toHaveBeenCalled(); }); - await importedGate; - // We are blocking import so this won't render yet - expect(projectAnnotations.renderToDOM).not.toHaveBeenCalled(); - mockChannel.emit.mockClear(); - openGate(); - await waitForRender(); + // For https://github.com/storybookjs/storybook/issues/19015 + it('does NOT render a second time in template docs mode', async () => { + document.location.search = '?id=component-one--docs&viewMode=docs'; - // We should only render *once* - expect(projectAnnotations.renderToDOM).toHaveBeenCalledTimes(1); + const [gate, openGate] = createGate(); + const [importedGate, openImportedGate] = createGate(); + importFn + .mockImplementationOnce(async (...args) => { + await gate; + return importFn(...args); + }) + .mockImplementationOnce(async (...args) => { + // The second time we `import()` we open the "imported" gate + openImportedGate(); + await gate; + return importFn(...args); + }); + + const preview = new PreviewWeb(); + // We can't wait for the initialize function, as it waits for `renderSelection()` + // which prepares, but it does emit `CURRENT_STORY_WAS_SET` right before that + preview.initialize({ importFn, getProjectAnnotations }); + await waitForEvents([CURRENT_STORY_WAS_SET]); - // We should not show an error either - expect(preview.view.showErrorDisplay).not.toHaveBeenCalled(); + mockChannel.emit.mockClear(); + projectAnnotations.renderToDOM.mockClear(); + emitter.emit(SET_CURRENT_STORY, { + storyId: 'component-one--docs', + viewMode: 'docs', + }); + await importedGate; + // We are blocking import so this won't render yet + expect(docsRenderer.render).not.toHaveBeenCalled(); + + mockChannel.emit.mockClear(); + openGate(); + await waitForRender(); + + // We should only render *once* + expect(docsRenderer.render).toHaveBeenCalledTimes(1); + + // We should not show an error either + expect(preview.view.showErrorDisplay).not.toHaveBeenCalled(); + }); + + it('does NOT render a second time in standalone docs mode', async () => { + document.location.search = '?id=introduction--docs&viewMode=docs'; + + const [gate, openGate] = createGate(); + const [importedGate, openImportedGate] = createGate(); + importFn + .mockImplementationOnce(async (...args) => { + await gate; + return importFn(...args); + }) + .mockImplementationOnce(async (...args) => { + // The second time we `import()` we open the "imported" gate + openImportedGate(); + await gate; + return importFn(...args); + }); + + const preview = new PreviewWeb(); + // We can't wait for the initialize function, as it waits for `renderSelection()` + // which prepares, but it does emit `CURRENT_STORY_WAS_SET` right before that + preview.initialize({ importFn, getProjectAnnotations }); + await waitForEvents([CURRENT_STORY_WAS_SET]); + + mockChannel.emit.mockClear(); + projectAnnotations.renderToDOM.mockClear(); + emitter.emit(SET_CURRENT_STORY, { + storyId: 'introduction--docs', + viewMode: 'docs', + }); + await importedGate; + // We are blocking import so this won't render yet + expect(docsRenderer.render).not.toHaveBeenCalled(); + + mockChannel.emit.mockClear(); + openGate(); + await waitForRender(); + + // We should only render *once* + expect(docsRenderer.render).toHaveBeenCalledTimes(1); + + // We should not show an error either + expect(preview.view.showErrorDisplay).not.toHaveBeenCalled(); + }); }); }); @@ -2713,8 +2809,12 @@ describe('PreviewWeb', () => { }); describe('if it was previously rendered', () => { - beforeEach(() => jest.useFakeTimers()); - afterEach(() => jest.useRealTimers()); + beforeEach(() => { + jest.useFakeTimers(); + }); + afterEach(() => { + jest.useRealTimers(); + }); it('is reloaded when it is re-selected', async () => { document.location.search = '?id=component-one--a'; const preview = await createAndRenderPreview(); diff --git a/code/lib/preview-web/src/PreviewWeb.tsx b/code/lib/preview-web/src/PreviewWeb.tsx index 3178ae7611fe..444c582c9179 100644 --- a/code/lib/preview-web/src/PreviewWeb.tsx +++ b/code/lib/preview-web/src/PreviewWeb.tsx @@ -34,7 +34,8 @@ import { MaybePromise, Preview } from './Preview'; import { UrlStore } from './UrlStore'; import { WebView } from './WebView'; -import { PREPARE_ABORTED, StoryRender } from './render/StoryRender'; +import { PREPARE_ABORTED } from './render/Render'; +import { StoryRender } from './render/StoryRender'; import { TemplateDocsRender } from './render/TemplateDocsRender'; import { StandaloneDocsRender } from './render/StandaloneDocsRender'; diff --git a/code/lib/preview-web/src/render/Render.ts b/code/lib/preview-web/src/render/Render.ts index 64b1458cb885..95d15245336d 100644 --- a/code/lib/preview-web/src/render/Render.ts +++ b/code/lib/preview-web/src/render/Render.ts @@ -19,3 +19,5 @@ export interface Render { torndown: boolean; renderToElement: (canvasElement: HTMLElement, renderStoryToElement?: any) => Promise; } + +export const PREPARE_ABORTED = new Error('prepareAborted'); diff --git a/code/lib/preview-web/src/render/StandaloneDocsRender.ts b/code/lib/preview-web/src/render/StandaloneDocsRender.ts index c6d99edef02a..b84e22c84f01 100644 --- a/code/lib/preview-web/src/render/StandaloneDocsRender.ts +++ b/code/lib/preview-web/src/render/StandaloneDocsRender.ts @@ -3,7 +3,7 @@ import { CSFFile, ModuleExports, StoryStore } from '@storybook/store'; import { Channel, IndexEntry } from '@storybook/addons'; import { DOCS_RENDERED } from '@storybook/core-events'; -import { Render, RenderType } from './Render'; +import { Render, RenderType, PREPARE_ABORTED } from './Render'; import type { DocsContextProps } from '../docs-context/DocsContextProps'; import type { DocsRenderFunction } from '../docs-context/DocsRenderFunction'; import { DocsContext } from '../docs-context/DocsContext'; @@ -26,7 +26,7 @@ export class StandaloneDocsRender implements Re public rerender?: () => Promise; - public teardown?: (options: { viewModeChanged?: boolean }) => Promise; + public teardownRender?: (options: { viewModeChanged?: boolean }) => Promise; public torndown = false; @@ -51,6 +51,9 @@ export class StandaloneDocsRender implements Re async prepare() { this.preparing = true; const { entryExports, csfFiles = [] } = await this.store.loadEntry(this.id); + + if (this.torndown) throw PREPARE_ABORTED; + this.csfFiles = csfFiles; this.exports = entryExports; @@ -96,7 +99,7 @@ export class StandaloneDocsRender implements Re }; this.rerender = async () => renderDocs(); - this.teardown = async ({ viewModeChanged }: { viewModeChanged?: boolean } = {}) => { + this.teardownRender = async ({ viewModeChanged }: { viewModeChanged?: boolean } = {}) => { if (!viewModeChanged || !canvasElement) return; renderer.unmount(canvasElement); this.torndown = true; @@ -104,4 +107,9 @@ export class StandaloneDocsRender implements Re return renderDocs(); } + + async teardown({ viewModeChanged }: { viewModeChanged?: boolean } = {}) { + this.teardownRender?.({ viewModeChanged }); + this.torndown = true; + } } diff --git a/code/lib/preview-web/src/render/StoryRender.ts b/code/lib/preview-web/src/render/StoryRender.ts index 70b6b8fd86a0..5e9e3ac02f74 100644 --- a/code/lib/preview-web/src/render/StoryRender.ts +++ b/code/lib/preview-web/src/render/StoryRender.ts @@ -20,7 +20,7 @@ import { STORY_RENDERED, PLAY_FUNCTION_THREW_EXCEPTION, } from '@storybook/core-events'; -import { Render, RenderType } from './Render'; +import { Render, RenderType, PREPARE_ABORTED } from './Render'; const { AbortController } = global; @@ -60,8 +60,6 @@ export type RenderContextCallbacks = Pick< 'showMain' | 'showError' | 'showException' >; -export const PREPARE_ABORTED = new Error('prepareAborted'); - export class StoryRender implements Render { public type: RenderType = 'story'; @@ -275,7 +273,7 @@ export class StoryRender implements Render implements Rend public rerender?: () => Promise; - public teardown?: (options: { viewModeChanged?: boolean }) => Promise; + public teardownRender?: (options: { viewModeChanged?: boolean }) => Promise; public torndown = false; @@ -70,6 +70,8 @@ export class TemplateDocsRender implements Rend const primaryStoryId = Object.keys(primaryCsfFile.stories)[0]; this.story = this.store.storyFromCSFFile({ storyId: primaryStoryId, csfFile: primaryCsfFile }); + if (this.torndown) throw PREPARE_ABORTED; + this.csfFiles = [primaryCsfFile, ...csfFiles]; this.preparing = false; @@ -112,12 +114,16 @@ export class TemplateDocsRender implements Rend }; this.rerender = async () => renderDocs(); - this.teardown = async ({ viewModeChanged }: { viewModeChanged?: boolean } = {}) => { + this.teardownRender = async ({ viewModeChanged }: { viewModeChanged?: boolean }) => { if (!viewModeChanged || !canvasElement) return; renderer.unmount(canvasElement); - this.torndown = true; }; return renderDocs(); } + + async teardown({ viewModeChanged }: { viewModeChanged?: boolean } = {}) { + this.teardownRender?.({ viewModeChanged }); + this.torndown = true; + } }