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
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
189 changes: 144 additions & 45 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';
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.

import global from 'global';
import merge from 'lodash/merge';
import {
Expand Down Expand Up @@ -31,8 +31,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';
tmeasday marked this conversation as resolved.
Show resolved Hide resolved

import { PreviewWeb } from './PreviewWeb';
import {
Expand Down Expand Up @@ -60,8 +60,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 +70,7 @@ jest.mock('global', () => ({
},
window: {
location: {
reload: jest.fn(),
reload: mockJest.fn(),
},
},
FEATURES: {
Expand Down Expand Up @@ -134,7 +134,7 @@ beforeEach(() => {
projectAnnotations.render.mockClear();
projectAnnotations.decorators[0].mockClear();
docsRenderer.render.mockClear();
(logger.warn as jest.Mock<typeof logger.warn>).mockClear();
(logger.warn as jestMock.Mock<typeof logger.warn>).mockClear();
mockStoryIndex.mockReset().mockReturnValue(storyIndex);

addons.setChannel(mockChannel as any);
Expand Down Expand Up @@ -349,8 +349,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 +1567,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 +2810,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;
tmeasday marked this conversation as resolved.
Show resolved Hide resolved
renderer.unmount(canvasElement);
this.torndown = true;
};

return renderDocs();
}

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