From 05e51a1adee0567ca4114338e01239b84af80156 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 1 Sep 2022 16:56:28 +1000 Subject: [PATCH 1/3] 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; + } } From 8c9458c5307d768ab535082c872919674c1d2e69 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 1 Sep 2022 17:17:21 +1000 Subject: [PATCH 2/3] Add some basic unit tests for renders --- .../src/render/StandaloneDocsRender.test.ts | 52 ++++++++++++++++++ .../src/render/StandaloneDocsRender.ts | 1 - .../src/render/StoryRender.test.ts | 54 +++++++++++++++++++ .../src/render/TemplateDocsRender.test.ts | 52 ++++++++++++++++++ .../src/render/TemplateDocsRender.ts | 5 +- 5 files changed, 160 insertions(+), 4 deletions(-) create mode 100644 code/lib/preview-web/src/render/StandaloneDocsRender.test.ts create mode 100644 code/lib/preview-web/src/render/StoryRender.test.ts create mode 100644 code/lib/preview-web/src/render/TemplateDocsRender.test.ts diff --git a/code/lib/preview-web/src/render/StandaloneDocsRender.test.ts b/code/lib/preview-web/src/render/StandaloneDocsRender.test.ts new file mode 100644 index 000000000000..492f33dc0bd2 --- /dev/null +++ b/code/lib/preview-web/src/render/StandaloneDocsRender.test.ts @@ -0,0 +1,52 @@ +import { jest, describe, it, expect } from '@jest/globals'; +import { Channel } from '@storybook/channels'; +import { AnyFramework } from '@storybook/csf'; +import { StoryStore } from '@storybook/store'; +import type { StandaloneDocsIndexEntry } from '@storybook/store'; +import { PREPARE_ABORTED } from './Render'; + +import { StandaloneDocsRender } from './StandaloneDocsRender'; + +const entry = { + type: 'docs', + id: 'introduction--docs', + name: 'Docs', + title: 'Introduction', + importPath: './Introduction.mdx', + storiesImports: [], + standalone: true, +} as StandaloneDocsIndexEntry; + +const createGate = (): [Promise, (_?: any) => void] => { + let openGate = (_?: any) => {}; + const gate = new Promise((resolve) => { + openGate = resolve; + }); + return [gate, openGate]; +}; + +describe('StandaloneDocsRender', () => { + it('throws PREPARE_ABORTED if torndown during prepare', async () => { + const [importGate, openImportGate] = createGate(); + const mockStore = { + loadEntry: jest.fn(async () => { + await importGate; + return {}; + }), + }; + + const render = new StandaloneDocsRender( + new Channel(), + mockStore as unknown as StoryStore, + entry + ); + + const preparePromise = render.prepare(); + + render.teardown(); + + openImportGate(); + + await expect(preparePromise).rejects.toThrowError(PREPARE_ABORTED); + }); +}); diff --git a/code/lib/preview-web/src/render/StandaloneDocsRender.ts b/code/lib/preview-web/src/render/StandaloneDocsRender.ts index b84e22c84f01..27581c2ad9b3 100644 --- a/code/lib/preview-web/src/render/StandaloneDocsRender.ts +++ b/code/lib/preview-web/src/render/StandaloneDocsRender.ts @@ -51,7 +51,6 @@ 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; diff --git a/code/lib/preview-web/src/render/StoryRender.test.ts b/code/lib/preview-web/src/render/StoryRender.test.ts new file mode 100644 index 000000000000..7ec87a7d1506 --- /dev/null +++ b/code/lib/preview-web/src/render/StoryRender.test.ts @@ -0,0 +1,54 @@ +import { jest, describe, it, expect } from '@jest/globals'; +import { Channel } from '@storybook/channels'; +import { AnyFramework } from '@storybook/csf'; +import { StoryStore } from '@storybook/store'; +import type { StoryIndexEntry } from '@storybook/store'; +import { PREPARE_ABORTED } from './Render'; + +import { StoryRender } from './StoryRender'; + +const entry = { + type: 'story', + id: 'component--a', + name: 'A', + title: 'component', + importPath: './component.stories.ts', +} as StoryIndexEntry; + +const createGate = (): [Promise, (_?: any) => void] => { + let openGate = (_?: any) => {}; + const gate = new Promise((resolve) => { + openGate = resolve; + }); + return [gate, openGate]; +}; + +describe('StoryRender', () => { + it('throws PREPARE_ABORTED if torndown during prepare', async () => { + const [importGate, openImportGate] = createGate(); + const mockStore = { + loadStory: jest.fn(async () => { + await importGate; + return {}; + }), + cleanupStory: jest.fn(), + }; + + const render = new StoryRender( + new Channel(), + mockStore as unknown as StoryStore, + jest.fn(), + {} as any, + entry.id, + 'story' + ); + + const preparePromise = render.prepare(); + + render.teardown(); + + openImportGate(); + + await expect(preparePromise).rejects.toThrowError(PREPARE_ABORTED); + }); +}); diff --git a/code/lib/preview-web/src/render/TemplateDocsRender.test.ts b/code/lib/preview-web/src/render/TemplateDocsRender.test.ts new file mode 100644 index 000000000000..572fb3541b9a --- /dev/null +++ b/code/lib/preview-web/src/render/TemplateDocsRender.test.ts @@ -0,0 +1,52 @@ +import { jest, describe, it, expect } from '@jest/globals'; +import { Channel } from '@storybook/channels'; +import { AnyFramework } from '@storybook/csf'; +import { StoryStore } from '@storybook/store'; +import type { TemplateDocsIndexEntry } from '@storybook/store'; +import { PREPARE_ABORTED } from './Render'; + +import { TemplateDocsRender } from './TemplateDocsRender'; + +const entry = { + type: 'docs', + id: 'component--docs', + name: 'Docs', + title: 'Component', + importPath: './Component.stories.ts', + storiesImports: [], + standalone: false, +} as TemplateDocsIndexEntry; + +const createGate = (): [Promise, (_?: any) => void] => { + let openGate = (_?: any) => {}; + const gate = new Promise((resolve) => { + openGate = resolve; + }); + return [gate, openGate]; +}; + +describe('TemplateDocsRender', () => { + it('throws PREPARE_ABORTED if torndown during prepare', async () => { + const [importGate, openImportGate] = createGate(); + const mockStore = { + loadEntry: jest.fn(async () => { + await importGate; + return {}; + }), + }; + + const render = new TemplateDocsRender( + new Channel(), + mockStore as unknown as StoryStore, + entry + ); + + const preparePromise = render.prepare(); + + render.teardown(); + + openImportGate(); + + await expect(preparePromise).rejects.toThrowError(PREPARE_ABORTED); + }); +}); diff --git a/code/lib/preview-web/src/render/TemplateDocsRender.ts b/code/lib/preview-web/src/render/TemplateDocsRender.ts index 8f9b696b32ea..1d3be018ae83 100644 --- a/code/lib/preview-web/src/render/TemplateDocsRender.ts +++ b/code/lib/preview-web/src/render/TemplateDocsRender.ts @@ -54,9 +54,10 @@ export class TemplateDocsRender implements Rend async prepare() { this.preparing = true; const { entryExports, csfFiles = [] } = await this.store.loadEntry(this.id); + if (this.torndown) throw PREPARE_ABORTED; const { importPath, title } = this.entry; - const primaryCsfFile = await this.store.processCSFFileWithCache( + const primaryCsfFile = this.store.processCSFFileWithCache( entryExports, importPath, title @@ -70,8 +71,6 @@ 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; From f84161b14b0a2b63ad20849734b4de632a6f7e6c Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Fri, 2 Sep 2022 13:08:10 +1000 Subject: [PATCH 3/3] Fix jest issues --- code/lib/preview-web/src/PreviewWeb.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/code/lib/preview-web/src/PreviewWeb.test.ts b/code/lib/preview-web/src/PreviewWeb.test.ts index 5f9a3a7c6fcf..2efa7ab9a64d 100644 --- a/code/lib/preview-web/src/PreviewWeb.test.ts +++ b/code/lib/preview-web/src/PreviewWeb.test.ts @@ -32,7 +32,6 @@ import { addons, mockChannel as createMockChannel } from '@storybook/addons'; import type { AnyFramework } from '@storybook/csf'; import type { ModuleImportFn, WebProjectAnnotations } from '@storybook/store'; import { mocked } from 'ts-jest/utils'; -import jestMock from 'jest-mock'; import { PreviewWeb } from './PreviewWeb'; import { @@ -134,7 +133,7 @@ beforeEach(() => { projectAnnotations.render.mockClear(); projectAnnotations.decorators[0].mockClear(); docsRenderer.render.mockClear(); - (logger.warn as jestMock.Mock).mockClear(); + (logger.warn as jest.Mock).mockClear(); mockStoryIndex.mockReset().mockReturnValue(storyIndex); addons.setChannel(mockChannel as any);