Skip to content

Commit

Permalink
Merge pull request #19071 from storybookjs/tom/sb-699-sb19015-70-vite…
Browse files Browse the repository at this point in the history
…-storybook-crashes-when

Ensure if a docs render is torndown during preparation, it throws
  • Loading branch information
tmeasday authored Sep 2, 2022
2 parents 8645a18 + f84161b commit 5407c0c
Show file tree
Hide file tree
Showing 9 changed files with 326 additions and 57 deletions.
186 changes: 142 additions & 44 deletions code/lib/preview-web/src/PreviewWeb.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/// <reference types="@types/jest" />;

import { jest, jest as mockJest, it, describe, beforeEach, afterEach, expect } from '@jest/globals';
import global from 'global';
import merge from 'lodash/merge';
import {
Expand Down Expand Up @@ -31,7 +31,6 @@ 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 { PreviewWeb } from './PreviewWeb';
Expand Down Expand Up @@ -60,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',
Expand All @@ -70,7 +69,7 @@ jest.mock('global', () => ({
},
window: {
location: {
reload: jest.fn(),
reload: mockJest.fn(),
},
},
FEATURES: {
Expand Down Expand Up @@ -349,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';
Expand Down Expand Up @@ -1563,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();
});
});
});

Expand Down Expand Up @@ -2715,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();
Expand Down
3 changes: 2 additions & 1 deletion code/lib/preview-web/src/PreviewWeb.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
2 changes: 2 additions & 0 deletions code/lib/preview-web/src/render/Render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,5 @@ export interface Render<TFramework extends AnyFramework> {
torndown: boolean;
renderToElement: (canvasElement: HTMLElement, renderStoryToElement?: any) => Promise<void>;
}

export const PREPARE_ABORTED = new Error('prepareAborted');
52 changes: 52 additions & 0 deletions code/lib/preview-web/src/render/StandaloneDocsRender.test.ts
Original file line number Diff line number Diff line change
@@ -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 | undefined>, (_?: any) => void] => {
let openGate = (_?: any) => {};
const gate = new Promise<any | undefined>((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<AnyFramework>,
entry
);

const preparePromise = render.prepare();

render.teardown();

openImportGate();

await expect(preparePromise).rejects.toThrowError(PREPARE_ABORTED);
});
});
13 changes: 10 additions & 3 deletions code/lib/preview-web/src/render/StandaloneDocsRender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -26,7 +26,7 @@ export class StandaloneDocsRender<TFramework extends AnyFramework> implements Re

public rerender?: () => Promise<void>;

public teardown?: (options: { viewModeChanged?: boolean }) => Promise<void>;
public teardownRender?: (options: { viewModeChanged?: boolean }) => Promise<void>;

public torndown = false;

Expand All @@ -51,6 +51,8 @@ export class StandaloneDocsRender<TFramework extends AnyFramework> 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;

Expand Down Expand Up @@ -96,12 +98,17 @@ export class StandaloneDocsRender<TFramework extends AnyFramework> 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;
};

return renderDocs();
}

async teardown({ viewModeChanged }: { viewModeChanged?: boolean } = {}) {
this.teardownRender?.({ viewModeChanged });
this.torndown = true;
}
}
Loading

0 comments on commit 5407c0c

Please sign in to comment.