From 91c58820886ce5998c5cee83a353172f4a166b13 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Tue, 21 Nov 2023 15:28:18 +1100 Subject: [PATCH 01/26] Move preview init parameters to constructor --- .../src/codegen-modern-iframe-script.ts | 4 +- .../virtualModuleModernEntry.js.handlebars | 4 +- .../src/modules/preview-web/Preview.tsx | 40 ++--- .../PreviewWeb.integration.test.ts | 15 +- .../modules/preview-web/PreviewWeb.test.ts | 162 ++++++++---------- .../src/modules/preview-web/PreviewWeb.tsx | 13 +- .../preview-web/PreviewWithSelection.tsx | 57 +++--- 7 files changed, 132 insertions(+), 163 deletions(-) diff --git a/code/builders/builder-vite/src/codegen-modern-iframe-script.ts b/code/builders/builder-vite/src/codegen-modern-iframe-script.ts index c30c533dbd8d..eb9e36b08cc5 100644 --- a/code/builders/builder-vite/src/codegen-modern-iframe-script.ts +++ b/code/builders/builder-vite/src/codegen-modern-iframe-script.ts @@ -66,10 +66,10 @@ export async function generateModernIframeScriptCode(options: Options, projectRo ${getPreviewAnnotationsFunction} - window.__STORYBOOK_PREVIEW__ = window.__STORYBOOK_PREVIEW__ || new PreviewWeb(); + window.__STORYBOOK_PREVIEW__ = window.__STORYBOOK_PREVIEW__ || new PreviewWeb(importFn, getProjectAnnotations); window.__STORYBOOK_STORY_STORE__ = window.__STORYBOOK_STORY_STORE__ || window.__STORYBOOK_PREVIEW__.storyStore; - window.__STORYBOOK_PREVIEW__.initialize({ importFn, getProjectAnnotations }); + window.__STORYBOOK_PREVIEW__.initialize(); ${generateHMRHandler(frameworkName)}; `.trim(); diff --git a/code/builders/builder-webpack5/templates/virtualModuleModernEntry.js.handlebars b/code/builders/builder-webpack5/templates/virtualModuleModernEntry.js.handlebars index 20d420c825f6..6ef2d9f9647c 100644 --- a/code/builders/builder-webpack5/templates/virtualModuleModernEntry.js.handlebars +++ b/code/builders/builder-webpack5/templates/virtualModuleModernEntry.js.handlebars @@ -15,13 +15,13 @@ if (global.CONFIG_TYPE === 'DEVELOPMENT'){ window.__STORYBOOK_SERVER_CHANNEL__ = channel; } -const preview = new PreviewWeb(); +const preview = new PreviewWeb(importFn, getProjectAnnotations); window.__STORYBOOK_PREVIEW__ = preview; window.__STORYBOOK_STORY_STORE__ = preview.storyStore; window.__STORYBOOK_ADDONS_CHANNEL__ = channel; -preview.initialize({ importFn, getProjectAnnotations }); +preview.initialize(); if (import.meta.webpackHot) { import.meta.webpackHot.accept('./{{storiesFilename}}', () => { diff --git a/code/lib/preview-api/src/modules/preview-web/Preview.tsx b/code/lib/preview-api/src/modules/preview-web/Preview.tsx index 593b14c7c3f7..0d6868c63fb8 100644 --- a/code/lib/preview-api/src/modules/preview-web/Preview.tsx +++ b/code/lib/preview-api/src/modules/preview-web/Preview.tsx @@ -49,40 +49,27 @@ export class Preview { storyStore: StoryStore; - getStoryIndex?: () => StoryIndex; - - importFn?: ModuleImportFn; - renderToCanvas?: RenderToCanvas; storyRenders: StoryRender[] = []; previewEntryError?: Error; - constructor(protected channel: Channel = addons.getChannel()) { + constructor( + public importFn: ModuleImportFn, + + public getProjectAnnotations: () => MaybePromise>, + + protected channel: Channel = addons.getChannel() + ) { this.storyStore = new StoryStore(); } // INITIALIZATION - async initialize({ - getStoryIndex, - importFn, - getProjectAnnotations, - }: { - // In the case of the v6 store, we can only get the index from the facade *after* - // getProjectAnnotations has been run, thus this slightly awkward approach - getStoryIndex?: () => StoryIndex; - importFn: ModuleImportFn; - getProjectAnnotations: () => MaybePromise>; - }) { - // We save these two on initialization in case `getProjectAnnotations` errors, - // in which case we may need them later when we recover. - this.getStoryIndex = getStoryIndex; - this.importFn = importFn; - + async initialize() { this.setupListeners(); - const projectAnnotations = await this.getProjectAnnotationsOrRenderError(getProjectAnnotations); + const projectAnnotations = await this.getProjectAnnotationsOrRenderError(); return this.initializeWithProjectAnnotations(projectAnnotations); } @@ -95,11 +82,9 @@ export class Preview { this.channel.on(FORCE_REMOUNT, this.onForceRemount.bind(this)); } - async getProjectAnnotationsOrRenderError( - getProjectAnnotations: () => MaybePromise> - ): Promise> { + async getProjectAnnotationsOrRenderError(): Promise> { try { - const projectAnnotations = await getProjectAnnotations(); + const projectAnnotations = await this.getProjectAnnotations(); this.renderToCanvas = projectAnnotations.renderToCanvas; if (!this.renderToCanvas) { @@ -176,8 +161,9 @@ export class Preview { getProjectAnnotations: () => MaybePromise>; }) { delete this.previewEntryError; + this.getProjectAnnotations = getProjectAnnotations; - const projectAnnotations = await this.getProjectAnnotationsOrRenderError(getProjectAnnotations); + const projectAnnotations = await this.getProjectAnnotationsOrRenderError(); if (!this.storyStore.projectAnnotations) { await this.initializeWithProjectAnnotations(projectAnnotations); return; diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.integration.test.ts b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.integration.test.ts index 61e0f239df15..963689b30611 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.integration.test.ts +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.integration.test.ts @@ -82,7 +82,7 @@ describe.skip('PreviewWeb', () => { storyFn() ); document.location.search = '?id=component-one--a'; - await new PreviewWeb().initialize({ importFn, getProjectAnnotations }); + await new PreviewWeb(importFn, getProjectAnnotations).initialize(); await waitForRender(); @@ -95,7 +95,7 @@ describe.skip('PreviewWeb', () => { projectAnnotations.parameters.docs.renderer = () => new DocsRenderer() as any; document.location.search = '?id=component-one--docs&viewMode=docs'; - const preview = new PreviewWeb(); + const preview = new PreviewWeb(importFn, getProjectAnnotations); const docsRoot = document.createElement('div'); vi.mocked(preview.view.prepareForDocs).mockReturnValue(docsRoot as any); @@ -103,7 +103,7 @@ describe.skip('PreviewWeb', () => { React.createElement('div', {}, 'INSIDE') ); - await preview.initialize({ importFn, getProjectAnnotations }); + await preview.initialize(); await waitForRender(); expect(docsRoot.outerHTML).toMatchInlineSnapshot('"
INSIDE
"'); @@ -117,7 +117,7 @@ describe.skip('PreviewWeb', () => { projectAnnotations.parameters.docs.renderer = () => new DocsRenderer() as any; document.location.search = '?id=component-one--docs&viewMode=docs'; - const preview = new PreviewWeb(); + const preview = new PreviewWeb(importFn, getProjectAnnotations); const docsRoot = document.createElement('div'); vi.mocked(preview.view.prepareForDocs).mockReturnValue(docsRoot as any); @@ -126,7 +126,8 @@ describe.skip('PreviewWeb', () => { }); vi.mocked(preview.view.showErrorDisplay).mockClear(); - await preview.initialize({ importFn, getProjectAnnotations }); + + await preview.initialize(); await waitForRender(); expect(preview.view.showErrorDisplay).toHaveBeenCalled(); @@ -149,8 +150,8 @@ describe.skip('PreviewWeb', () => { projectAnnotations.parameters.docs.renderer = () => new DocsRenderer() as any; document.location.search = '?id=component-one--a'; - const preview = new PreviewWeb(); - await preview.initialize({ importFn, getProjectAnnotations }); + const preview = new PreviewWeb(importFn, getProjectAnnotations); + await preview.initialize(); await waitForRender(); projectAnnotations.renderToCanvas.mockImplementationOnce(({ storyFn }: RenderContext) => diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts index f6d27e21c031..88bb5fd97940 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts @@ -103,11 +103,8 @@ async function createAndRenderPreview({ importFn?: ModuleImportFn; getProjectAnnotations?: () => ProjectAnnotations; } = {}) { - const preview = new PreviewWeb(); - await preview.initialize({ - importFn: inputImportFn, - getProjectAnnotations: inputGetProjectAnnotations, - }); + const preview = new PreviewWeb(inputImportFn, inputGetProjectAnnotations); + await preview.initialize(); await waitForRender(); return preview; @@ -141,15 +138,10 @@ describe.skip('PreviewWeb', () => { describe('initialize', () => { it('shows an error if getProjectAnnotations throws', async () => { const err = new Error('meta error'); - const preview = new PreviewWeb(); - await expect( - preview.initialize({ - importFn, - getProjectAnnotations: () => { - throw err; - }, - }) - ).rejects.toThrow(err); + const preview = new PreviewWeb(importFn, () => { + throw err; + }); + await expect(preview.initialize()).rejects.toThrow(err); expect(preview.view.showErrorDisplay).toHaveBeenCalled(); expect(mockChannel.emit).toHaveBeenCalledWith(CONFIG_ERROR, err); @@ -159,10 +151,8 @@ describe.skip('PreviewWeb', () => { const err = new Error('sort error'); mockFetchResult = { status: 500, text: async () => err.toString() }; - const preview = new PreviewWeb(); - await expect(preview.initialize({ importFn, getProjectAnnotations })).rejects.toThrow( - 'sort error' - ); + const preview = new PreviewWeb(importFn, getProjectAnnotations); + await expect(preview.initialize()).rejects.toThrow('sort error'); expect(preview.view.showErrorDisplay).toHaveBeenCalled(); expect(mockChannel.emit).toHaveBeenCalledWith(CONFIG_ERROR, expect.any(Error)); @@ -229,13 +219,10 @@ describe.skip('PreviewWeb', () => { }); it('allows async getProjectAnnotations', async () => { - const preview = new PreviewWeb(); - await preview.initialize({ - importFn, - getProjectAnnotations: async () => { - return getProjectAnnotations(); - }, + const preview = new PreviewWeb(importFn, async () => { + return getProjectAnnotations(); }); + await preview.initialize(); expect(preview.storyStore.globals!.get()).toEqual({ a: 'b' }); }); @@ -521,8 +508,8 @@ describe.skip('PreviewWeb', () => { ...projectAnnotations, renderToCanvas: undefined, }); - const preview = new PreviewWeb(); - await expect(preview.initialize({ importFn, getProjectAnnotations })).rejects.toThrow(); + const preview = new PreviewWeb(importFn, getProjectAnnotations); + await expect(preview.initialize()).rejects.toThrow(); expect(preview.view.showErrorDisplay).toHaveBeenCalled(); expect(vi.mocked(preview.view.showErrorDisplay).mock.calls[0][0]).toMatchInlineSnapshot(` @@ -626,6 +613,24 @@ describe.skip('PreviewWeb', () => { expect(mockChannel.emit).toHaveBeenCalledWith(STORY_RENDERED, 'component-one--a'); }); + + it('does not show error display if the render function throws IGNORED_EXCEPTION', async () => { + document.location.search = '?id=component-one--a'; + projectAnnotations.renderToCanvas.mockImplementation(() => { + throw IGNORED_EXCEPTION; + }); + + const preview = new PreviewWeb(importFn, getProjectAnnotations); + await preview.initialize(); + + await waitForRender(); + + expect(mockChannel.emit).toHaveBeenCalledWith( + STORY_THREW_EXCEPTION, + serializeError(IGNORED_EXCEPTION) + ); + expect(preview.view.showErrorDisplay).not.toHaveBeenCalled(); + }); }); describe('CSF docs entries', () => { @@ -920,7 +925,7 @@ describe.skip('PreviewWeb', () => { document.location.search = '?id=component-one--a'; componentOneExports.default.loaders[0].mockImplementationOnce(async () => gate); - await new PreviewWeb().initialize({ importFn, getProjectAnnotations }); + await new PreviewWeb(importFn, getProjectAnnotations).initialize(); await waitForRenderPhase('loading'); expect(componentOneExports.default.loaders[0]).toHaveBeenCalledWith( @@ -981,7 +986,7 @@ describe.skip('PreviewWeb', () => { document.location.search = '?id=component-one--a'; projectAnnotations.renderToCanvas.mockImplementation(async () => gate); - await new PreviewWeb().initialize({ importFn, getProjectAnnotations }); + await new PreviewWeb(importFn, getProjectAnnotations).initialize(); await waitForRenderPhase('rendering'); emitter.emit(UPDATE_STORY_ARGS, { @@ -1060,7 +1065,7 @@ describe.skip('PreviewWeb', () => { }); document.location.search = '?id=component-one--a'; - await new PreviewWeb().initialize({ importFn, getProjectAnnotations }); + await new PreviewWeb(importFn, getProjectAnnotations).initialize(); await waitForRenderPhase('playing'); await renderToCanvasCalled; @@ -1480,7 +1485,7 @@ describe.skip('PreviewWeb', () => { document.location.search = '?id=component-one--a'; projectAnnotations.renderToCanvas.mockImplementation(async () => gate); - await new PreviewWeb().initialize({ importFn, getProjectAnnotations }); + await new PreviewWeb(importFn, getProjectAnnotations).initialize(); await waitForRenderPhase('rendering'); expect(projectAnnotations.renderToCanvas).toHaveBeenCalledWith( @@ -1579,7 +1584,7 @@ describe.skip('PreviewWeb', () => { it('works when there was no selection specifier', async () => { document.location.search = ''; // We intentionally are *not* awaiting here - new PreviewWeb().initialize({ importFn, getProjectAnnotations }); + new PreviewWeb(importFn, getProjectAnnotations).initialize(); emitter.emit(SET_CURRENT_STORY, { storyId: 'component-one--b', viewMode: 'story' }); @@ -1604,10 +1609,7 @@ describe.skip('PreviewWeb', () => { it('works when there was a selection specifier', async () => { document.location.search = '?id=component-one--a'; - const initialized = new PreviewWeb().initialize({ - importFn, - getProjectAnnotations, - }); + const initialized = new PreviewWeb(importFn, getProjectAnnotations).initialize(); emitter.emit(SET_CURRENT_STORY, { storyId: 'component-one--b', viewMode: 'story' }); @@ -1719,10 +1721,10 @@ describe.skip('PreviewWeb', () => { return importFn(m); }); - const preview = new PreviewWeb(); + const preview = new PreviewWeb(importFn, getProjectAnnotations); // 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 }); + preview.initialize(); await waitForEvents([CURRENT_STORY_WAS_SET]); mockChannel.emit.mockClear(); @@ -1766,10 +1768,10 @@ describe.skip('PreviewWeb', () => { return importFn(m); }); - const preview = new PreviewWeb(); + const preview = new PreviewWeb(importFn, getProjectAnnotations); // 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 }); + preview.initialize(); await waitForEvents([CURRENT_STORY_WAS_SET]); mockChannel.emit.mockClear(); @@ -1812,10 +1814,10 @@ describe.skip('PreviewWeb', () => { return importFn(m); }); - const preview = new PreviewWeb(); + const preview = new PreviewWeb(importFn, getProjectAnnotations); // 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 }); + preview.initialize(); await waitForEvents([CURRENT_STORY_WAS_SET]); mockChannel.emit.mockClear(); @@ -2148,7 +2150,7 @@ describe.skip('PreviewWeb', () => { componentOneExports.default.loaders[0].mockImplementationOnce(async () => gate); document.location.search = '?id=component-one--a'; - await new PreviewWeb().initialize({ importFn, getProjectAnnotations }); + await new PreviewWeb(importFn, getProjectAnnotations).initialize(); await waitForRenderPhase('loading'); emitter.emit(SET_CURRENT_STORY, { @@ -2181,7 +2183,7 @@ describe.skip('PreviewWeb', () => { document.location.search = '?id=component-one--a'; projectAnnotations.renderToCanvas.mockImplementation(async () => gate); - await new PreviewWeb().initialize({ importFn, getProjectAnnotations }); + await new PreviewWeb(importFn, getProjectAnnotations).initialize(); await waitForRenderPhase('rendering'); mockChannel.emit.mockClear(); @@ -2215,7 +2217,7 @@ describe.skip('PreviewWeb', () => { componentOneExports.a.play.mockImplementationOnce(async () => gate); document.location.search = '?id=component-one--a'; - await new PreviewWeb().initialize({ importFn, getProjectAnnotations }); + await new PreviewWeb(importFn, getProjectAnnotations).initialize(); await waitForRenderPhase('playing'); expect(projectAnnotations.renderToCanvas).toHaveBeenCalledWith( @@ -2268,7 +2270,7 @@ describe.skip('PreviewWeb', () => { componentOneExports.a.play.mockImplementationOnce(async () => gate); document.location.search = '?id=component-one--a'; - await new PreviewWeb().initialize({ importFn, getProjectAnnotations }); + await new PreviewWeb(importFn, getProjectAnnotations).initialize(); await waitForRenderPhase('playing'); expect(projectAnnotations.renderToCanvas).toHaveBeenCalledWith( @@ -2697,10 +2699,8 @@ describe.skip('PreviewWeb', () => { const err = new Error('sort error'); mockFetchResult = { status: 500, text: async () => err.toString() }; - const preview = new PreviewWeb(); - await expect(preview.initialize({ importFn, getProjectAnnotations })).rejects.toThrow( - 'sort error' - ); + const preview = new PreviewWeb(importFn, getProjectAnnotations); + await expect(preview.initialize()).rejects.toThrow('sort error'); expect(preview.view.showErrorDisplay).toHaveBeenCalled(); expect(mockChannel.emit).toHaveBeenCalledWith(CONFIG_ERROR, expect.any(Error)); @@ -2717,10 +2717,8 @@ describe.skip('PreviewWeb', () => { const err = new Error('sort error'); mockFetchResult = { status: 500, text: async () => err.toString() }; - const preview = new PreviewWeb(); - await expect(preview.initialize({ importFn, getProjectAnnotations })).rejects.toThrow( - 'sort error' - ); + const preview = new PreviewWeb(importFn, getProjectAnnotations); + await expect(preview.initialize()).rejects.toThrow('sort error'); expect(preview.view.showErrorDisplay).toHaveBeenCalled(); expect(mockChannel.emit).toHaveBeenCalledWith(CONFIG_ERROR, expect.any(Error)); @@ -3302,15 +3300,10 @@ describe.skip('PreviewWeb', () => { document.location.search = '?id=component-one--a'; const err = new Error('meta error'); - const preview = new PreviewWeb(); - await expect( - preview.initialize({ - importFn, - getProjectAnnotations: () => { - throw err; - }, - }) - ).rejects.toThrow(err); + const preview = new PreviewWeb(importFn, () => { + throw err; + }); + await expect(preview.initialize()).rejects.toThrow(err); preview.onGetProjectAnnotationsChanged({ getProjectAnnotations }); await waitForRender(); @@ -3322,15 +3315,10 @@ describe.skip('PreviewWeb', () => { document.location.search = '?id=*&globals=a:c'; const err = new Error('meta error'); - const preview = new PreviewWeb(); - await expect( - preview.initialize({ - importFn, - getProjectAnnotations: () => { - throw err; - }, - }) - ).rejects.toThrow(err); + const preview = new PreviewWeb(importFn, () => { + throw err; + }); + await expect(preview.initialize()).rejects.toThrow(err); preview.onGetProjectAnnotationsChanged({ getProjectAnnotations }); await waitForRender(); @@ -3511,8 +3499,8 @@ describe.skip('PreviewWeb', () => { const [gate, openGate] = createGate(); componentOneExports.a.play.mockImplementationOnce(async () => gate); - const preview = new PreviewWeb(); - await preview.initialize({ importFn, getProjectAnnotations }); + const preview = new PreviewWeb(importFn, getProjectAnnotations); + await preview.initialize(); await waitForRenderPhase('playing'); await preview.onKeydown({ @@ -3554,26 +3542,12 @@ describe.skip('PreviewWeb', () => { }); describe('extract', () => { - // NOTE: if you are using storyStoreV6, and your `preview.js` throws, we do not currently - // detect it (as we do not wrap the import of `preview.js` in a `try/catch`). The net effect - // of that is that the `PreviewWeb`/`StoryStore` end up in an uninitalized state. - it('throws an error if the preview is uninitialized', async () => { - const preview = new PreviewWeb(); - await expect(preview.extract()).rejects.toThrow(/Failed to initialize/); - }); - it('throws an error if preview.js throws', async () => { const err = new Error('meta error'); - const preview = new PreviewWeb(); - await expect( - preview.initialize({ - importFn, - getProjectAnnotations: () => { - throw err; - }, - }) - ).rejects.toThrow(err); - + const preview = new PreviewWeb(importFn, () => { + throw err; + }); + await expect(preview.initialize()).rejects.toThrow(err); await expect(preview.extract()).rejects.toThrow(err); }); @@ -3581,10 +3555,8 @@ describe.skip('PreviewWeb', () => { const err = new Error('sort error'); mockFetchResult = { status: 500, text: async () => err.toString() }; - const preview = new PreviewWeb(); - await expect(preview.initialize({ importFn, getProjectAnnotations })).rejects.toThrow( - 'sort error' - ); + const preview = new PreviewWeb(importFn, getProjectAnnotations); + await expect(preview.initialize()).rejects.toThrow('sort error'); await expect(preview.extract()).rejects.toThrow('sort error'); }); diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.tsx b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.tsx index e945320a0f0c..eea7635b9e30 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.tsx +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.tsx @@ -1,14 +1,19 @@ /* eslint-disable no-underscore-dangle */ import { global } from '@storybook/global'; -import type { Renderer } from '@storybook/types'; +import type { Renderer, ProjectAnnotations, StoryIndex, ModuleImportFn } from '@storybook/types'; import { PreviewWithSelection } from './PreviewWithSelection'; import { UrlStore } from './UrlStore'; import { WebView } from './WebView'; +import type { MaybePromise } from './Preview'; -export class PreviewWeb extends PreviewWithSelection { - constructor() { - super(new UrlStore(), new WebView()); +export class PreviewWeb extends PreviewWithSelection { + constructor( + public importFn: ModuleImportFn, + + public getProjectAnnotations: () => MaybePromise> + ) { + super(importFn, getProjectAnnotations, new UrlStore(), new WebView()); global.__STORYBOOK_PREVIEW__ = this; } diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx b/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx index 27f653cd60f1..56753b17daea 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx @@ -56,39 +56,44 @@ export function isMdxEntry({ tags }: DocsIndexEntry) { return !tags?.includes(AUTODOCS_TAG) && !tags?.includes(STORIES_MDX_TAG); } -type PossibleRender = - | StoryRender - | CsfDocsRender - | MdxDocsRender; - -function isStoryRender( - r: PossibleRender -): r is StoryRender { +type PossibleRender = + | StoryRender + | CsfDocsRender + | MdxDocsRender; + +function isStoryRender( + r: PossibleRender +): r is StoryRender { return r.type === 'story'; } -function isDocsRender( - r: PossibleRender -): r is CsfDocsRender | MdxDocsRender { +function isDocsRender( + r: PossibleRender +): r is CsfDocsRender | MdxDocsRender { return r.type === 'docs'; } -function isCsfDocsRender( - r: PossibleRender -): r is CsfDocsRender { +function isCsfDocsRender( + r: PossibleRender +): r is CsfDocsRender { return isDocsRender(r) && r.subtype === 'csf'; } -export class PreviewWithSelection extends Preview { +export class PreviewWithSelection extends Preview { currentSelection?: Selection; - currentRender?: PossibleRender; + currentRender?: PossibleRender; constructor( + public importFn: ModuleImportFn, + + public getProjectAnnotations: () => MaybePromise>, + public selectionStore: SelectionStore, - public view: View + + public view: View ) { - super(); + super(importFn, getProjectAnnotations); } setupListeners() { @@ -179,7 +184,7 @@ export class PreviewWithSelection extends Preview MaybePromise>; + getProjectAnnotations: () => MaybePromise>; }) { await super.onGetProjectAnnotationsChanged({ getProjectAnnotations }); @@ -300,9 +305,9 @@ export class PreviewWithSelection extends Preview; + let render: PossibleRender; if (entry.type === 'story') { - render = new StoryRender( + render = new StoryRender( this.channel, this.storyStore, (...args: Parameters) => { @@ -315,14 +320,14 @@ export class PreviewWithSelection extends Preview( + render = new MdxDocsRender( this.channel, this.storyStore, entry, this.mainStoryCallbacks(storyId) ); } else { - render = new CsfDocsRender( + render = new CsfDocsRender( this.channel, this.storyStore, entry, @@ -422,8 +427,8 @@ export class PreviewWithSelection extends Preview); - (this.currentRender as StoryRender).renderToElement( + this.storyRenders.push(render as StoryRender); + (this.currentRender as StoryRender).renderToElement( this.view.prepareForStory(render.story) ); } else { @@ -436,7 +441,7 @@ export class PreviewWithSelection extends Preview, + render: PossibleRender, { viewModeChanged = false }: { viewModeChanged?: boolean } = {} ) { this.storyRenders = this.storyRenders.filter((r) => r !== render); From 0cf9b3808fb3555ba8a8782e3cba1c9405b709dd Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Tue, 21 Nov 2023 16:28:40 +1100 Subject: [PATCH 02/26] Re-enable tests --- .../src/modules/preview-web/PreviewWeb.integration.test.ts | 7 +++---- .../preview-api/src/modules/preview-web/PreviewWeb.test.ts | 3 ++- code/yarn.lock | 1 + 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.integration.test.ts b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.integration.test.ts index 963689b30611..4f2118d56b68 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.integration.test.ts +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.integration.test.ts @@ -72,7 +72,7 @@ beforeEach(() => { vi.mocked(WebView.prototype).prepareForStory.mockReturnValue('story-element' as any); }); -describe.skip('PreviewWeb', () => { +describe('PreviewWeb', () => { describe('initial render', () => { it('renders story mode through the stack', async () => { const { DocsRenderer } = await import('@storybook/addon-docs'); @@ -111,8 +111,7 @@ describe.skip('PreviewWeb', () => { // Error: Event was not emitted in time: storyRendered,docsRendered,storyThrewException,storyErrored,storyMissing }, 10_000); - // TODO @tmeasday please help fixing this test - it.skip('sends docs rendering exceptions to showException', async () => { + it('sends docs rendering exceptions to showException', async () => { const { DocsRenderer } = await import('@storybook/addon-docs'); projectAnnotations.parameters.docs.renderer = () => new DocsRenderer() as any; @@ -121,7 +120,7 @@ describe.skip('PreviewWeb', () => { const docsRoot = document.createElement('div'); vi.mocked(preview.view.prepareForDocs).mockReturnValue(docsRoot as any); - componentOneExports.default.parameters.docs.container.mockImplementationOnce(() => { + componentOneExports.default.parameters.docs.container.mockImplementation(() => { throw new Error('Docs rendering error'); }); diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts index 88bb5fd97940..c769679ff2b3 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts @@ -1,3 +1,4 @@ +/* eslint-disable local-rules/no-uncategorized-errors */ /** * @vitest-environment jsdom */ @@ -134,7 +135,7 @@ beforeEach(() => { vi.mocked(WebView.prototype).prepareForStory.mockReturnValue('story-element' as any); }); -describe.skip('PreviewWeb', () => { +describe('PreviewWeb', () => { describe('initialize', () => { it('shows an error if getProjectAnnotations throws', async () => { const err = new Error('meta error'); diff --git a/code/yarn.lock b/code/yarn.lock index 2590e466330b..484fd969621b 100644 --- a/code/yarn.lock +++ b/code/yarn.lock @@ -6199,6 +6199,7 @@ __metadata: resolution: "@storybook/preview-api@workspace:lib/preview-api" dependencies: "@jest/globals": "npm:^29.5.0" + "@storybook/addon-docs": "workspace:*" "@storybook/channels": "workspace:*" "@storybook/client-logger": "workspace:*" "@storybook/core-common": "workspace:*" From bea10728f5bcce0792a0992176e196c7fa5d50c3 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Tue, 21 Nov 2023 17:16:09 +1100 Subject: [PATCH 03/26] Move initialization promise to Preview and simplify store --- .../src/modules/preview-web/Preview.tsx | 90 +++++++---- .../modules/preview-web/PreviewWeb.test.ts | 10 +- .../preview-web/PreviewWithSelection.tsx | 16 +- .../src/modules/store/StoryStore.test.ts | 146 ++++-------------- .../src/modules/store/StoryStore.ts | 90 +++-------- 5 files changed, 116 insertions(+), 236 deletions(-) diff --git a/code/lib/preview-api/src/modules/preview-web/Preview.tsx b/code/lib/preview-api/src/modules/preview-web/Preview.tsx index 0d6868c63fb8..ce84a3b10421 100644 --- a/code/lib/preview-api/src/modules/preview-web/Preview.tsx +++ b/code/lib/preview-api/src/modules/preview-web/Preview.tsx @@ -47,7 +47,7 @@ export class Preview { */ serverChannel?: Channel; - storyStore: StoryStore; + storyStore?: StoryStore; renderToCanvas?: RenderToCanvas; @@ -55,6 +55,14 @@ export class Preview { previewEntryError?: Error; + // While we wait for the index to load (note it may error for a while), we need to store the + // project annotations. Once the index loads, it is stored on the store and this will get unset. + private projectAnnotationsBeforeInitialization?: ProjectAnnotations; + + protected storeInitializationPromise: Promise; + + protected resolveStoreInitializationPromise!: () => void; + constructor( public importFn: ModuleImportFn, @@ -62,7 +70,9 @@ export class Preview { protected channel: Channel = addons.getChannel() ) { - this.storyStore = new StoryStore(); + this.storeInitializationPromise = new Promise((resolve) => { + this.resolveStoreInitializationPromise = resolve; + }); } // INITIALIZATION @@ -70,7 +80,7 @@ export class Preview { this.setupListeners(); const projectAnnotations = await this.getProjectAnnotationsOrRenderError(); - return this.initializeWithProjectAnnotations(projectAnnotations); + await this.initializeWithProjectAnnotations(projectAnnotations); } setupListeners() { @@ -107,10 +117,7 @@ export class Preview { // If initialization gets as far as project annotations, this function runs. async initializeWithProjectAnnotations(projectAnnotations: ProjectAnnotations) { - this.storyStore.setProjectAnnotations(projectAnnotations); - - this.setInitialGlobals(); - + this.projectAnnotationsBeforeInitialization = projectAnnotations; try { const storyIndex = await this.getStoryIndexFromServer(); return this.initializeWithStoryIndex(storyIndex); @@ -120,21 +127,6 @@ export class Preview { } } - async setInitialGlobals() { - this.emitGlobals(); - } - - emitGlobals() { - if (!this.storyStore.globals || !this.storyStore.projectAnnotations) - throw new Error(`Cannot emit before initialization`); - - const payload: SetGlobalsPayload = { - globals: this.storyStore.globals.get() || {}, - globalTypes: this.storyStore.projectAnnotations.globalTypes || {}, - }; - this.channel.emit(SET_GLOBALS, payload); - } - async getStoryIndexFromServer() { const result = await fetch(STORY_INDEX_PATH); if (result.status === 200) { @@ -146,10 +138,33 @@ export class Preview { // If initialization gets as far as the story index, this function runs. initializeWithStoryIndex(storyIndex: StoryIndex): void { - if (!this.importFn) - throw new Error(`Cannot call initializeWithStoryIndex before initialization`); + if (!this.projectAnnotationsBeforeInitialization) + throw new Error('Cannot call initializeWithStoryIndex until project annotations resolve'); - this.storyStore.initialize({ storyIndex, importFn: this.importFn }); + this.storyStore = new StoryStore( + storyIndex, + this.importFn, + this.projectAnnotationsBeforeInitialization + ); + delete this.projectAnnotationsBeforeInitialization; // to avoid confusion + + this.setInitialGlobals(); + + this.resolveStoreInitializationPromise(); + } + + async setInitialGlobals() { + this.emitGlobals(); + } + + emitGlobals() { + if (!this.storyStore) throw new Error(`Cannot emit before initialization`); + + const payload: SetGlobalsPayload = { + globals: this.storyStore.globals.get() || {}, + globalTypes: this.storyStore.projectAnnotations.globalTypes || {}, + }; + this.channel.emit(SET_GLOBALS, payload); } // EVENT HANDLERS @@ -164,7 +179,7 @@ export class Preview { this.getProjectAnnotations = getProjectAnnotations; const projectAnnotations = await this.getProjectAnnotationsOrRenderError(); - if (!this.storyStore.projectAnnotations) { + if (!this.storyStore) { await this.initializeWithProjectAnnotations(projectAnnotations); return; } @@ -176,18 +191,19 @@ export class Preview { async onStoryIndexChanged() { delete this.previewEntryError; - if (!this.storyStore.projectAnnotations) { - // We haven't successfully set project annotations yet, - // we need to do that before we can do anything else. + // We haven't successfully set project annotations yet, + // we need to do that before we can do anything else. + if (!this.storyStore && !this.projectAnnotationsBeforeInitialization) { return; } try { const storyIndex = await this.getStoryIndexFromServer(); - // This is the first time the story index worked, let's load it into the store - if (!this.storyStore.storyIndex) { + // We've been waiting for the index to resolve, now it has, so we can continue + if (this.projectAnnotationsBeforeInitialization) { this.initializeWithStoryIndex(storyIndex); + return; } // Update the store with the new stories. @@ -206,12 +222,13 @@ export class Preview { importFn?: ModuleImportFn; storyIndex?: StoryIndex; }) { + if (!this.storyStore) throw new Error(`Cannot call onStoriesChanged before initialization`); + await this.storyStore.onStoriesChanged({ importFn, storyIndex }); } async onUpdateGlobals({ globals }: { globals: Globals }) { - if (!this.storyStore.globals) - throw new Error(`Cannot call onUpdateGlobals before initialization`); + if (!this.storyStore) throw new Error(`Cannot call onUpdateGlobals before initialization`); this.storyStore.globals.update(globals); await Promise.all(this.storyRenders.map((r) => r.rerender())); @@ -223,6 +240,7 @@ export class Preview { } async onUpdateArgs({ storyId, updatedArgs }: { storyId: StoryId; updatedArgs: Args }) { + if (!this.storyStore) throw new Error(`Cannot call onUpdateArgs before initialization`); this.storyStore.args.update(storyId, updatedArgs); await Promise.all( @@ -238,6 +256,8 @@ export class Preview { } async onResetArgs({ storyId, argNames }: { storyId: string; argNames?: string[] }) { + if (!this.storyStore) throw new Error(`Cannot call onResetArgs before initialization`); + // NOTE: we have to be careful here and avoid await-ing when updating a rendered's args. // That's because below in `renderStoryToElement` we have also bound to this event and will // render the story in the same tick. @@ -281,7 +301,7 @@ export class Preview { callbacks: RenderContextCallbacks, options: StoryRenderOptions ) { - if (!this.renderToCanvas) + if (!this.renderToCanvas || !this.storyStore) throw new Error(`Cannot call renderStoryToElement before initialization`); const render = new StoryRender( @@ -313,6 +333,8 @@ export class Preview { // API async extract(options?: { includeDocsOnly: boolean }) { + if (!this.storyStore) throw new Error(`Cannot call extract before initialization`); + if (this.previewEntryError) { throw this.previewEntryError; } diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts index c769679ff2b3..416a73db003a 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts @@ -164,7 +164,7 @@ describe('PreviewWeb', () => { const preview = await createAndRenderPreview(); - expect(preview.storyStore.globals!.get()).toEqual({ a: 'c' }); + expect(preview.storyStore!.globals.get()).toEqual({ a: 'c' }); }); it('emits the SET_GLOBALS event', async () => { @@ -225,7 +225,7 @@ describe('PreviewWeb', () => { }); await preview.initialize(); - expect(preview.storyStore.globals!.get()).toEqual({ a: 'b' }); + expect(preview.storyStore!.globals.get()).toEqual({ a: 'b' }); }); }); @@ -798,7 +798,7 @@ describe('PreviewWeb', () => { emitter.emit(UPDATE_GLOBALS, { globals: { foo: 'bar' } }); - expect(preview.storyStore.globals!.get()).toEqual({ a: 'b' }); + expect(preview.storyStore!.globals.get()).toEqual({ a: 'b' }); }); it('passes globals in context to renderToCanvas', async () => { @@ -3324,7 +3324,7 @@ describe('PreviewWeb', () => { preview.onGetProjectAnnotationsChanged({ getProjectAnnotations }); await waitForRender(); - expect(preview.storyStore.globals!.get()).toEqual({ a: 'c' }); + expect(preview.storyStore!.globals.get()).toEqual({ a: 'c' }); }); }); @@ -3374,7 +3374,7 @@ describe('PreviewWeb', () => { preview.onGetProjectAnnotationsChanged({ getProjectAnnotations: newGetProjectAnnotations }); await waitForRender(); - expect(preview.storyStore.globals!.get()).toEqual({ a: 'edited' }); + expect(preview.storyStore!.globals.get()).toEqual({ a: 'edited' }); }); it('emits SET_GLOBALS with new values', async () => { diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx b/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx index 56753b17daea..eb8a72d0c06e 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx @@ -107,8 +107,7 @@ export class PreviewWithSelection extends Preview extends Preview extends Preview extends Preview this.storyStore.loadEntry(id))); + await Promise.allSettled(ids.map((id) => storyStore.loadEntry(id))); } // RENDERING @@ -270,6 +270,8 @@ export class PreviewWithSelection extends Preview extends Preview { describe('projectAnnotations', () => { it('normalizes on initialization', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); expect(store.projectAnnotations!.globalTypes).toEqual({ a: { name: 'a', type: { name: 'string' } }, @@ -97,9 +95,7 @@ describe('StoryStore', () => { }); it('normalizes on updateGlobalAnnotations', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); store.setProjectAnnotations(projectAnnotations); expect(store.projectAnnotations!.globalTypes).toEqual({ @@ -113,9 +109,7 @@ describe('StoryStore', () => { describe('loadStory', () => { it('pulls the story via the importFn', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); importFn.mockClear(); expect(await store.loadStory({ storyId: 'component-one--a' })).toMatchObject({ @@ -128,9 +122,7 @@ describe('StoryStore', () => { }); it('uses a cache', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); const story = await store.loadStory({ storyId: 'component-one--a' }); expect(processCSFFile).toHaveBeenCalledTimes(1); @@ -149,33 +141,11 @@ describe('StoryStore', () => { expect(processCSFFile).toHaveBeenCalledTimes(2); expect(prepareStory).toHaveBeenCalledTimes(3); }); - - describe('if the store is not yet initialized', () => { - it('waits for initialization', async () => { - const store = new StoryStore(); - - importFn.mockClear(); - const loadPromise = store.loadStory({ storyId: 'component-one--a' }); - - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); - - expect(await loadPromise).toMatchObject({ - id: 'component-one--a', - name: 'A', - title: 'Component One', - initialArgs: { foo: 'a' }, - }); - expect(importFn).toHaveBeenCalledWith('./src/ComponentOne.stories.js'); - }); - }); }); describe('setProjectAnnotations', () => { it('busts the loadStory cache', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); const story = await store.loadStory({ storyId: 'component-one--a' }); expect(processCSFFile).toHaveBeenCalledTimes(1); @@ -192,9 +162,7 @@ describe('StoryStore', () => { describe('onStoriesChanged', () => { it('busts the loadStory cache if the importFn returns a new module', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); const story = await store.loadStory({ storyId: 'component-one--a' }); expect(processCSFFile).toHaveBeenCalledTimes(1); @@ -214,9 +182,7 @@ describe('StoryStore', () => { }); it('busts the loadStory cache if the csf file no longer appears in the index', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); await store.loadStory({ storyId: 'component-one--a' }); expect(processCSFFile).toHaveBeenCalledTimes(1); @@ -233,9 +199,7 @@ describe('StoryStore', () => { }); it('reuses the cache if a story importPath has not changed', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); const story = await store.loadStory({ storyId: 'component-one--a' }); expect(processCSFFile).toHaveBeenCalledTimes(1); @@ -265,9 +229,7 @@ describe('StoryStore', () => { }); it('imports with a new path for a story id if provided', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); await store.loadStory({ storyId: 'component-one--a' }); expect(importFn).toHaveBeenCalledWith(storyIndex.entries['component-one--a'].importPath); @@ -295,9 +257,7 @@ describe('StoryStore', () => { }); it('re-caches stories if the were cached already', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); await store.cacheAllCSFFiles(); await store.loadStory({ storyId: 'component-one--a' }); @@ -368,9 +328,7 @@ describe('StoryStore', () => { describe('componentStoriesFromCSFFile', () => { it('returns all the stories in the file', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); const csfFile = await store.loadCSFFileByStoryId('component-one--a'); const stories = store.componentStoriesFromCSFFile({ csfFile }); @@ -380,8 +338,6 @@ describe('StoryStore', () => { }); it('returns them in the order they are in the index, not the file', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); const reversedIndex = { v: 4, entries: { @@ -389,7 +345,7 @@ describe('StoryStore', () => { 'component-one--a': storyIndex.entries['component-one--a'], }, }; - store.initialize({ storyIndex: reversedIndex, importFn }); + const store = new StoryStore(reversedIndex, importFn, projectAnnotations); const csfFile = await store.loadCSFFileByStoryId('component-one--a'); const stories = store.componentStoriesFromCSFFile({ csfFile }); @@ -401,9 +357,7 @@ describe('StoryStore', () => { describe('getStoryContext', () => { it('returns the args and globals correctly', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); const story = await store.loadStory({ storyId: 'component-one--a' }); @@ -414,9 +368,7 @@ describe('StoryStore', () => { }); it('returns the args and globals correctly when they change', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); const story = await store.loadStory({ storyId: 'component-one--a' }); @@ -430,9 +382,7 @@ describe('StoryStore', () => { }); it('can force initial args', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); const story = await store.loadStory({ storyId: 'component-one--a' }); @@ -444,9 +394,7 @@ describe('StoryStore', () => { }); it('returns the same hooks each time', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); const story = await store.loadStory({ storyId: 'component-one--a' }); @@ -461,9 +409,7 @@ describe('StoryStore', () => { describe('cleanupStory', () => { it('cleans the hooks from the context', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); const story = await store.loadStory({ storyId: 'component-one--a' }); @@ -476,9 +422,7 @@ describe('StoryStore', () => { describe('loadAllCSFFiles', () => { it('imports *all* csf files', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); importFn.mockClear(); const csfFiles = await store.loadAllCSFFiles(); @@ -496,9 +440,7 @@ describe('StoryStore', () => { await gate; return importFn(file); }); - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn: blockedImportFn }); + const store = new StoryStore(storyIndex, blockedImportFn, projectAnnotations); const promise = store.loadAllCSFFiles({ batchSize: 1 }); expect(blockedImportFn).toHaveBeenCalledTimes(1); @@ -511,17 +453,13 @@ describe('StoryStore', () => { describe('extract', () => { it('throws if you have not called cacheAllCSFFiles', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); expect(() => store.extract()).toThrow(/Cannot call extract/); }); it('produces objects with functions and hooks stripped', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); await store.cacheAllCSFFiles(); expect(store.extract()).toMatchInlineSnapshot(` @@ -653,12 +591,7 @@ describe('StoryStore', () => { } : componentTwoExports; }); - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ - storyIndex, - importFn: docsOnlyImportFn, - }); + const store = new StoryStore(storyIndex, docsOnlyImportFn, projectAnnotations); await store.cacheAllCSFFiles(); expect(Object.keys(store.extract())).toEqual(['component-one--b', 'component-two--c']); @@ -685,12 +618,7 @@ describe('StoryStore', () => { }, }, }; - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ - storyIndex: unnattachedStoryIndex, - importFn, - }); + const store = new StoryStore(unnattachedStoryIndex, importFn, projectAnnotations); await store.cacheAllCSFFiles(); expect(Object.keys(store.extract())).toEqual([ @@ -709,9 +637,7 @@ describe('StoryStore', () => { describe('raw', () => { it('produces an array of stories', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); await store.cacheAllCSFFiles(); expect(store.raw()).toMatchInlineSnapshot(` @@ -858,9 +784,7 @@ describe('StoryStore', () => { describe('getSetStoriesPayload', () => { it('maps stories list to payload correctly', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); await store.cacheAllCSFFiles(); expect(store.getSetStoriesPayload()).toMatchInlineSnapshot(` @@ -998,9 +922,7 @@ describe('StoryStore', () => { describe('getStoriesJsonData', () => { describe('in back-compat mode', () => { it('maps stories list to payload correctly', async () => { - const store = new StoryStore(); - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); + const store = new StoryStore(storyIndex, importFn, projectAnnotations); await store.cacheAllCSFFiles(); expect(store.getStoriesJsonData()).toMatchInlineSnapshot(` @@ -1049,20 +971,4 @@ describe('StoryStore', () => { }); }); }); - - describe('cacheAllCsfFiles', () => { - describe('if the store is not yet initialized', () => { - it('waits for initialization', async () => { - const store = new StoryStore(); - - importFn.mockClear(); - const cachePromise = store.cacheAllCSFFiles(); - - store.setProjectAnnotations(projectAnnotations); - store.initialize({ storyIndex, importFn }); - - await expect(cachePromise).resolves.toEqual(undefined); - }); - }); - }); }); diff --git a/code/lib/preview-api/src/modules/store/StoryStore.ts b/code/lib/preview-api/src/modules/store/StoryStore.ts index e12732b871fc..2e646c788b3e 100644 --- a/code/lib/preview-api/src/modules/store/StoryStore.ts +++ b/code/lib/preview-api/src/modules/store/StoryStore.ts @@ -41,13 +41,11 @@ const STORY_CACHE_SIZE = 10000; const EXTRACT_BATCH_SIZE = 20; export class StoryStore { - storyIndex?: StoryIndexStore; + public storyIndex: StoryIndexStore; - importFn?: ModuleImportFn; + projectAnnotations: NormalizedProjectAnnotations; - projectAnnotations?: NormalizedProjectAnnotations; - - globals?: GlobalsStore; + globals: GlobalsStore; args: ArgsStore; @@ -61,13 +59,20 @@ export class StoryStore { prepareStoryWithCache: typeof prepareStory; - initializationPromise: Promise; + constructor( + storyIndex: StoryIndex, + + public importFn: ModuleImportFn, + + projectAnnotations: ProjectAnnotations + ) { + this.storyIndex = new StoryIndexStore(storyIndex); - // This *does* get set in the constructor but the semantics of `new Promise` trip up TS - resolveInitializationPromise!: () => void; + this.projectAnnotations = normalizeProjectAnnotations(projectAnnotations); + const { globals, globalTypes } = projectAnnotations; - constructor() { this.args = new ArgsStore(); + this.globals = new GlobalsStore({ globals, globalTypes }); this.hooks = {}; // We use a cache for these two functions for two reasons: @@ -76,37 +81,13 @@ export class StoryStore { this.processCSFFileWithCache = memoize(CSF_CACHE_SIZE)(processCSFFile) as typeof processCSFFile; this.prepareMetaWithCache = memoize(CSF_CACHE_SIZE)(prepareMeta) as typeof prepareMeta; this.prepareStoryWithCache = memoize(STORY_CACHE_SIZE)(prepareStory) as typeof prepareStory; - - // We cannot call `loadStory()` until we've been initialized properly. But we can wait for it. - this.initializationPromise = new Promise((resolve) => { - this.resolveInitializationPromise = resolve; - }); } setProjectAnnotations(projectAnnotations: ProjectAnnotations) { // By changing `this.projectAnnotations, we implicitly invalidate the `prepareStoryWithCache` this.projectAnnotations = normalizeProjectAnnotations(projectAnnotations); const { globals, globalTypes } = projectAnnotations; - - if (this.globals) { - this.globals.set({ globals, globalTypes }); - } else { - this.globals = new GlobalsStore({ globals, globalTypes }); - } - } - - initialize({ - storyIndex, - importFn, - }: { - storyIndex?: StoryIndex; - importFn: ModuleImportFn; - }): void { - this.storyIndex = new StoryIndexStore(storyIndex); - this.importFn = importFn; - - // We don't need the cache to be loaded to call `loadStory`, we just need the index ready - this.resolveInitializationPromise(); + this.globals.set({ globals, globalTypes }); } // This means that one of the CSF files has changed. @@ -120,26 +101,20 @@ export class StoryStore { importFn?: ModuleImportFn; storyIndex?: StoryIndex; }) { - await this.initializationPromise; - if (importFn) this.importFn = importFn; // The index will always be set before the initialization promise returns - if (storyIndex) this.storyIndex!.entries = storyIndex.entries; + if (storyIndex) this.storyIndex.entries = storyIndex.entries; if (this.cachedCSFFiles) await this.cacheAllCSFFiles(); } // Get an entry from the index, waiting on initialization if necessary async storyIdToEntry(storyId: StoryId): Promise { - await this.initializationPromise; // The index will always be set before the initialization promise returns - return this.storyIndex!.storyIdToEntry(storyId); + return this.storyIndex.storyIdToEntry(storyId); } // To load a single CSF file to service a story we need to look up the importPath in the index async loadCSFFileByStoryId(storyId: StoryId): Promise> { - if (!this.storyIndex || !this.importFn) - throw new Error(`loadCSFFileByStoryId called before initialization`); - const { importPath, title } = this.storyIndex.storyIdToEntry(storyId); const moduleExports = await this.importFn(importPath); @@ -150,8 +125,6 @@ export class StoryStore { async loadAllCSFFiles({ batchSize = EXTRACT_BATCH_SIZE } = {}): Promise< StoryStore['cachedCSFFiles'] > { - if (!this.storyIndex) throw new Error(`loadAllCSFFiles called before initialization`); - const importPaths = Object.entries(this.storyIndex.entries).map(([storyId, { importPath }]) => [ importPath, storyId, @@ -185,13 +158,10 @@ export class StoryStore { } async cacheAllCSFFiles(): Promise { - await this.initializationPromise; this.cachedCSFFiles = await this.loadAllCSFFiles(); } preparedMetaFromCSFFile({ csfFile }: { csfFile: CSFFile }): PreparedMeta { - if (!this.projectAnnotations) throw new Error(`storyFromCSFFile called before initialization`); - const componentAnnotations = csfFile.meta; return this.prepareMetaWithCache( @@ -203,7 +173,6 @@ export class StoryStore { // Load the CSF file for a story and prepare the story from it and the project annotations. async loadStory({ storyId }: { storyId: StoryId }): Promise> { - await this.initializationPromise; const csfFile = await this.loadCSFFileByStoryId(storyId); return this.storyFromCSFFile({ storyId, csfFile }); } @@ -217,8 +186,6 @@ export class StoryStore { storyId: StoryId; csfFile: CSFFile; }): PreparedStory { - if (!this.projectAnnotations) throw new Error(`storyFromCSFFile called before initialization`); - const storyAnnotations = csfFile.stories[storyId]; if (!storyAnnotations) { throw new Error(`Didn't find '${storyId}' in CSF file, this is unexpected`); @@ -241,9 +208,6 @@ export class StoryStore { }: { csfFile: CSFFile; }): PreparedStory[] { - if (!this.storyIndex) - throw new Error(`componentStoriesFromCSFFile called before initialization`); - return Object.keys(this.storyIndex.entries) .filter((storyId: StoryId) => !!csfFile.stories[storyId]) .map((storyId: StoryId) => this.storyFromCSFFile({ storyId, csfFile })); @@ -252,15 +216,12 @@ export class StoryStore { async loadEntry(id: StoryId) { const entry = await this.storyIdToEntry(id); - const { importFn, storyIndex } = this; - if (!storyIndex || !importFn) throw new Error(`loadEntry called before initialization`); - const storyImports = entry.type === 'docs' ? entry.storiesImports : []; const [entryExports, ...csfFiles] = (await Promise.all([ - importFn(entry.importPath), + this.importFn(entry.importPath), ...storyImports.map((storyImportPath) => { - const firstStoryEntry = storyIndex.importPathToEntry(storyImportPath); + const firstStoryEntry = this.storyIndex.importPathToEntry(storyImportPath); return this.loadCSFFileByStoryId(firstStoryEntry.id); }), ])) as [ModuleExports, ...CSFFile[]]; @@ -274,8 +235,6 @@ export class StoryStore { story: PreparedStory, { forceInitialArgs = false } = {} ): Omit { - if (!this.globals) throw new Error(`getStoryContext called before initialization`); - return prepareContext({ ...story, args: forceInitialArgs ? story.initialArgs : this.args.get(story.id), @@ -291,8 +250,6 @@ export class StoryStore { extract( options: { includeDocsOnly?: boolean } = { includeDocsOnly: false } ): Record> { - if (!this.storyIndex) throw new Error(`extract called before initialization`); - const { cachedCSFFiles } = this; if (!cachedCSFFiles) throw new Error('Cannot call extract() unless you call cacheAllCSFFiles() first.'); @@ -328,8 +285,6 @@ export class StoryStore { } getSetStoriesPayload() { - if (!this.globals) throw new Error(`getSetStoriesPayload called before initialization`); - const stories = this.extract({ includeDocsOnly: true }); const kindParameters: Parameters = Object.values(stories).reduce( @@ -353,14 +308,11 @@ export class StoryStore { // It is used to allow v7 Storybooks to be composed in v6 Storybooks, which expect a // `stories.json` file with legacy fields (`kind` etc). getStoriesJsonData = (): StoryIndexV3 => { - const { storyIndex } = this; - if (!storyIndex) throw new Error(`getStoriesJsonData called before initialization`); - const value = this.getSetStoriesPayload(); const allowedParameters = ['fileName', 'docsOnly', 'framework', '__id', '__isArgsStory']; const stories: Record = mapValues(value.stories, (story) => { - const { importPath } = storyIndex.entries[story.id]; + const { importPath } = this.storyIndex.entries[story.id]; return { ...pick(story, ['id', 'name', 'title']), importPath, @@ -389,8 +341,6 @@ export class StoryStore { } fromId(storyId: StoryId): BoundStory | null { - if (!this.storyIndex) throw new Error(`fromId called before initialization`); - if (!this.cachedCSFFiles) throw new Error('Cannot call fromId/raw() unless you call cacheAllCSFFiles() first.'); From 166599ae474bfed08d7f08c4a393c950bddd524d Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Wed, 22 Nov 2023 13:32:31 +1100 Subject: [PATCH 04/26] Deprecate `fromId`/`raw` --- MIGRATION.md | 6 ++++++ code/addons/links/src/react/components/link.test.tsx | 4 ---- code/lib/preview-api/src/modules/store/StoryStore.ts | 9 +++++++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 0a13cc59da81..6f31fbe477e9 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -63,6 +63,7 @@ - [Canvas Doc block properties](#canvas-doc-block-properties) - [`Primary` Doc block properties](#primary-doc-block-properties) - [`createChannel` from `@storybook/postmessage` and `@storybook/channel-websocket`](#createchannel-from-storybookpostmessage-and--storybookchannel-websocket) + - [StoryStore methods deprecated](#storystore-methods-deprecated) - [From version 7.5.0 to 7.6.0](#from-version-750-to-760) - [CommonJS with Vite is deprecated](#commonjs-with-vite-is-deprecated) - [Using implicit actions during rendering is deprecated](#using-implicit-actions-during-rendering-is-deprecated) @@ -977,6 +978,11 @@ The `name` prop is now removed in favor of the `of` property. [More info](#doc-b The `createChannel` APIs from both `@storybook/channel-websocket` and `@storybook/postmessage` are now removed. Please use `createBrowserChannel` instead, from the `@storybook/channels` package. Additionally, the `PostmsgTransport` type is now removed in favor of `PostMessageTransport`. +#### StoryStore methods deprecated + +The following methods on the `StoryStore` are deprecated and will be removed in 9.0: + - `store.fromId()` - please use `store.loadStory()` instead. + - `store.raw()` - please use `store.extract()` instead. ## From version 7.5.0 to 7.6.0 diff --git a/code/addons/links/src/react/components/link.test.tsx b/code/addons/links/src/react/components/link.test.tsx index e49ad02ede9f..872c29d5b898 100644 --- a/code/addons/links/src/react/components/link.test.tsx +++ b/code/addons/links/src/react/components/link.test.tsx @@ -17,10 +17,6 @@ vi.mock('@storybook/global', () => ({ search: 'search', }, }, - window: global, - __STORYBOOK_STORY_STORE__: { - fromId: vi.fn(() => ({})), - }, }, })); diff --git a/code/lib/preview-api/src/modules/store/StoryStore.ts b/code/lib/preview-api/src/modules/store/StoryStore.ts index 2e646c788b3e..04bac3de3102 100644 --- a/code/lib/preview-api/src/modules/store/StoryStore.ts +++ b/code/lib/preview-api/src/modules/store/StoryStore.ts @@ -335,13 +335,22 @@ export class StoryStore { }; raw(): BoundStory[] { + deprecate( + 'StoryStore.raw() is deprecated and will be removed in 9.0, please use extract() instead' + ); return Object.values(this.extract()) .map(({ id }: { id: StoryId }) => this.fromId(id)) .filter(Boolean) as BoundStory[]; } fromId(storyId: StoryId): BoundStory | null { + deprecate( + 'StoryStore.fromId() is deprecated and will be removed in 9.0, please use loadStory() instead' + ); + + // Deprecated so won't make a proper error for this if (!this.cachedCSFFiles) + // eslint-disable-next-line local-rules/no-uncategorized-errors throw new Error('Cannot call fromId/raw() unless you call cacheAllCSFFiles() first.'); let importPath; From 9d813a0f30bc140a737a3c5bb8c41bccea3bfc85 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Wed, 22 Nov 2023 13:58:48 +1100 Subject: [PATCH 05/26] Refactored Preview + Store errors to new format --- .../core-events/src/errors/preview-errors.ts | 158 ++++++++++++++++++ code/lib/preview-api/package.json | 1 + .../src/modules/preview-web/Preview.tsx | 56 +++---- .../preview-web/PreviewWithSelection.tsx | 81 ++++----- .../src/modules/store/StoryStore.ts | 13 +- code/yarn.lock | 2 +- 6 files changed, 225 insertions(+), 86 deletions(-) diff --git a/code/lib/core-events/src/errors/preview-errors.ts b/code/lib/core-events/src/errors/preview-errors.ts index 26544d7efd8e..5d1485c4ab68 100644 --- a/code/lib/core-events/src/errors/preview-errors.ts +++ b/code/lib/core-events/src/errors/preview-errors.ts @@ -74,3 +74,161 @@ export class ImplicitActionsDuringRendering extends StorybookError { `; } } + +export class CalledExtractOnStoreError extends StorybookError { + readonly category = Category.PREVIEW_API; + + readonly code = 3; + + constructor(public data: object) { + super(); + } + + template() { + return dedent` + Cannot call \`storyStore.extract()\` without calling \`storyStore.cacheAllCsfFiles()\` first. + + You probably meant to call \`await preview.extract()\` which does the above for you.`; + } +} + +export class MissingRenderToCanvasError extends StorybookError { + readonly category = Category.PREVIEW_API; + + readonly code = 4; + + constructor(public data: object) { + super(); + } + + template() { + return dedent` + Expected your framework's preset to export a \`renderToCanvas\` field. + + Perhaps it needs to be upgraded for Storybook 6.4? + + More info: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#mainjs-framework-field`; + } +} + +export class CalledPreviewMethodBeforeInitializationError extends StorybookError { + readonly category = Category.PREVIEW_API; + + readonly code = 5; + + constructor(public data: { methodName: string }) { + super(); + } + + template() { + return dedent` + Called \`Preview.${this.data.methodName}()\` before initialization. + + The preview needs to load the story index before most methods can be called. If you want + to call \`${this.data.methodName}\`, try \`await preview.initializationPromise;\` first. + + If you didn't call the above code, then likely it was called by an addon that needs to + do the above.`; + } +} + +export class StoryIndexFetchError extends StorybookError { + readonly category = Category.PREVIEW_API; + + readonly code = 6; + + constructor(public data: { text: string }) { + super(); + } + + template() { + return dedent` + Error fetching \`/index.json\`: + + ${this.data.text} + + If you are in development, this likely indicates a problem with your Storybook process, + check the terminal for errors. + + If you are in a deployed Storybook, there may have been an issue deploying the full Storybook + build.`; + } +} + +export class MdxFileWithNoCsfReferencesError extends StorybookError { + readonly category = Category.PREVIEW_API; + + readonly code = 7; + + constructor(public data: { storyId: string }) { + super(); + } + + template() { + return dedent` + Tried to render docs entry ${this.data.storyId} but it is a MDX file that has no CSF + references, or autodocs for a CSF file that some doesn't refer to itself. + + This likely is an internal error in Storybook's indexing, or you've attached the + \`attached-mdx\` tag to an MDX file that is not attached.`; + } +} + +export class EmptyIndexError extends StorybookError { + readonly category = Category.PREVIEW_API; + + readonly code = 8; + + constructor(public data: object) { + super(); + } + + template() { + return dedent` + Couldn't find any stories in your Storybook. + + - Please check your stories field of your main.js config: does it match correctly? + - Also check the browser console and terminal for error messages.`; + } +} + +export class NoStoryMatchError extends StorybookError { + readonly category = Category.PREVIEW_API; + + readonly code = 9; + + constructor(public data: { storySpecifier: string }) { + super(); + } + + template() { + return dedent` + Couldn't find story matching '${this.data.storySpecifier}'. + + - Are you sure a story with that id exists? + - Please check your stories field of your main.js config. + - Also check the browser console and terminal for error messages.`; + } +} + +export class MissingStoryFromCsfFileError extends StorybookError { + readonly category = Category.PREVIEW_API; + + readonly code = 10; + + constructor(public data: { storyId: string }) { + super(); + } + + template() { + return dedent` + Couldn't find story matching id '${this.data.storyId}' after importing a CSF file. + + The file was indexed as if the story was there, but then after importing the file in the browser + we didn't find the story. Possible reasons: + - You are using a custom story indexer that is misbehaving. + - You have a custom file loader that is removing or renaming exports. + + Please check your browser console and terminal for errors that may explain the issue.`; + } +} diff --git a/code/lib/preview-api/package.json b/code/lib/preview-api/package.json index 483a068b53f7..b56fd96b454e 100644 --- a/code/lib/preview-api/package.json +++ b/code/lib/preview-api/package.json @@ -54,6 +54,7 @@ "lodash": "^4.17.21", "memoizerific": "^1.11.3", "qs": "^6.10.0", + "tiny-invariant": "^1.3.1", "ts-dedent": "^2.0.0", "util-deprecate": "^1.0.2" }, diff --git a/code/lib/preview-api/src/modules/preview-web/Preview.tsx b/code/lib/preview-api/src/modules/preview-web/Preview.tsx index ce84a3b10421..c164d412d26e 100644 --- a/code/lib/preview-api/src/modules/preview-web/Preview.tsx +++ b/code/lib/preview-api/src/modules/preview-web/Preview.tsx @@ -1,4 +1,3 @@ -import { dedent } from 'ts-dedent'; import { global } from '@storybook/global'; import { CONFIG_ERROR, @@ -28,6 +27,11 @@ import type { StoryRenderOptions, SetGlobalsPayload, } from '@storybook/types'; +import { + CalledPreviewMethodBeforeInitializationError, + MissingRenderToCanvasError, + StoryIndexFetchError, +} from '@storybook/core-events/preview-errors'; import { addons } from '../addons'; import { StoryStore } from '../../store'; @@ -97,15 +101,8 @@ export class Preview { const projectAnnotations = await this.getProjectAnnotations(); this.renderToCanvas = projectAnnotations.renderToCanvas; - if (!this.renderToCanvas) { - throw new Error(dedent` - Expected your framework's preset to export a \`renderToCanvas\` field. - - Perhaps it needs to be upgraded for Storybook 6.4? + if (!this.renderToCanvas) throw new MissingRenderToCanvasError({}); - More info: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#mainjs-framework-field - `); - } return projectAnnotations; } catch (err) { // This is an error extracting the projectAnnotations (i.e. evaluating the previewEntries) and @@ -133,12 +130,14 @@ export class Preview { return result.json() as any as StoryIndex; } - throw new Error(await result.text()); + throw new StoryIndexFetchError({ text: await result.text() }); } // If initialization gets as far as the story index, this function runs. - initializeWithStoryIndex(storyIndex: StoryIndex): void { + protected initializeWithStoryIndex(storyIndex: StoryIndex): void { if (!this.projectAnnotationsBeforeInitialization) + // This is a protected method and so shouldn't be called out of order by users + // eslint-disable-next-line local-rules/no-uncategorized-errors throw new Error('Cannot call initializeWithStoryIndex until project annotations resolve'); this.storyStore = new StoryStore( @@ -158,7 +157,8 @@ export class Preview { } emitGlobals() { - if (!this.storyStore) throw new Error(`Cannot emit before initialization`); + if (!this.storyStore) + throw new CalledPreviewMethodBeforeInitializationError({ methodName: 'emitGlobals' }); const payload: SetGlobalsPayload = { globals: this.storyStore.globals.get() || {}, @@ -222,13 +222,14 @@ export class Preview { importFn?: ModuleImportFn; storyIndex?: StoryIndex; }) { - if (!this.storyStore) throw new Error(`Cannot call onStoriesChanged before initialization`); - + if (!this.storyStore) + throw new CalledPreviewMethodBeforeInitializationError({ methodName: 'onStoriesChanged' }); await this.storyStore.onStoriesChanged({ importFn, storyIndex }); } async onUpdateGlobals({ globals }: { globals: Globals }) { - if (!this.storyStore) throw new Error(`Cannot call onUpdateGlobals before initialization`); + if (!this.storyStore) + throw new CalledPreviewMethodBeforeInitializationError({ methodName: 'onUpdateGlobals' }); this.storyStore.globals.update(globals); await Promise.all(this.storyRenders.map((r) => r.rerender())); @@ -240,7 +241,8 @@ export class Preview { } async onUpdateArgs({ storyId, updatedArgs }: { storyId: StoryId; updatedArgs: Args }) { - if (!this.storyStore) throw new Error(`Cannot call onUpdateArgs before initialization`); + if (!this.storyStore) + throw new CalledPreviewMethodBeforeInitializationError({ methodName: 'onUpdateArgs' }); this.storyStore.args.update(storyId, updatedArgs); await Promise.all( @@ -256,7 +258,8 @@ export class Preview { } async onResetArgs({ storyId, argNames }: { storyId: string; argNames?: string[] }) { - if (!this.storyStore) throw new Error(`Cannot call onResetArgs before initialization`); + if (!this.storyStore) + throw new CalledPreviewMethodBeforeInitializationError({ methodName: 'onResetArgs' }); // NOTE: we have to be careful here and avoid await-ing when updating a rendered's args. // That's because below in `renderStoryToElement` we have also bound to this event and will @@ -302,7 +305,9 @@ export class Preview { options: StoryRenderOptions ) { if (!this.renderToCanvas || !this.storyStore) - throw new Error(`Cannot call renderStoryToElement before initialization`); + throw new CalledPreviewMethodBeforeInitializationError({ + methodName: 'renderStoryToElement', + }); const render = new StoryRender( this.channel, @@ -333,19 +338,10 @@ export class Preview { // API async extract(options?: { includeDocsOnly: boolean }) { - if (!this.storyStore) throw new Error(`Cannot call extract before initialization`); + if (!this.storyStore) + throw new CalledPreviewMethodBeforeInitializationError({ methodName: 'extract' }); - if (this.previewEntryError) { - throw this.previewEntryError; - } - - if (!this.storyStore.projectAnnotations) { - // In v6 mode, if your preview.js throws, we never get a chance to initialize the preview - // or store, and the error is simply logged to the browser console. This is the best we can do - throw new Error(dedent`Failed to initialize Storybook. - - Do you have an error in your \`preview.js\`? Check your Storybook's browser console for errors.`); - } + if (this.previewEntryError) throw this.previewEntryError; await this.storyStore.cacheAllCSFFiles(); diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx b/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx index eb8a72d0c06e..a1ba086d161b 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx @@ -1,3 +1,4 @@ +import invariant from 'tiny-invariant'; import { dedent } from 'ts-dedent'; import { CURRENT_STORY_WAS_SET, @@ -29,6 +30,12 @@ import type { DocsIndexEntry, } from '@storybook/types'; +import { + CalledPreviewMethodBeforeInitializationError, + EmptyIndexError, + MdxFileWithNoCsfReferencesError, + NoStoryMatchError, +} from '@storybook/core-events/preview-errors'; import type { MaybePromise } from './Preview'; import { Preview } from './Preview'; @@ -107,7 +114,8 @@ export class PreviewWithSelection extends Preview extends Preview extends Preview extends Preview extends Preview extends Preview extends Preview extends Preview extends Preview); (this.currentRender as StoryRender).renderToElement( this.view.prepareForStory(render.story) @@ -450,25 +450,6 @@ export class PreviewWithSelection extends Preview { csfFile: CSFFile; }): PreparedStory { const storyAnnotations = csfFile.stories[storyId]; - if (!storyAnnotations) { - throw new Error(`Didn't find '${storyId}' in CSF file, this is unexpected`); - } + if (!storyAnnotations) throw new MissingStoryFromCsfFileError({ storyId }); + const componentAnnotations = csfFile.meta; const story = this.prepareStoryWithCache( @@ -251,8 +255,7 @@ export class StoryStore { options: { includeDocsOnly?: boolean } = { includeDocsOnly: false } ): Record> { const { cachedCSFFiles } = this; - if (!cachedCSFFiles) - throw new Error('Cannot call extract() unless you call cacheAllCSFFiles() first.'); + if (!cachedCSFFiles) throw new CalledExtractOnStoreError({}); return Object.entries(this.storyIndex.entries).reduce( (acc, [storyId, { type, importPath }]) => { diff --git a/code/yarn.lock b/code/yarn.lock index 484fd969621b..a11c60892a36 100644 --- a/code/yarn.lock +++ b/code/yarn.lock @@ -6199,7 +6199,6 @@ __metadata: resolution: "@storybook/preview-api@workspace:lib/preview-api" dependencies: "@jest/globals": "npm:^29.5.0" - "@storybook/addon-docs": "workspace:*" "@storybook/channels": "workspace:*" "@storybook/client-logger": "workspace:*" "@storybook/core-common": "workspace:*" @@ -6214,6 +6213,7 @@ __metadata: memoizerific: "npm:^1.11.3" qs: "npm:^6.10.0" slash: "npm:^5.0.0" + tiny-invariant: "npm:^1.3.1" ts-dedent: "npm:^2.0.0" util-deprecate: "npm:^1.0.2" languageName: unknown From 1722ea4312f4b1dc92084b82da36995b7b2c76ec Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Wed, 22 Nov 2023 21:54:56 +1100 Subject: [PATCH 06/26] Fix error tests --- .../src/modules/preview-web/PreviewWeb.test.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts index 416a73db003a..dd29a6b9cc46 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts @@ -1,4 +1,3 @@ -/* eslint-disable local-rules/no-uncategorized-errors */ /** * @vitest-environment jsdom */ @@ -514,12 +513,12 @@ describe('PreviewWeb', () => { expect(preview.view.showErrorDisplay).toHaveBeenCalled(); expect(vi.mocked(preview.view.showErrorDisplay).mock.calls[0][0]).toMatchInlineSnapshot(` - [Error: Expected your framework's preset to export a \`renderToCanvas\` field. + [SB_PREVIEW_API_0004 (MissingRenderToCanvasError): Expected your framework's preset to export a \`renderToCanvas\` field. - Perhaps it needs to be upgraded for Storybook 6.4? + Perhaps it needs to be upgraded for Storybook 6.4? - More info: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#mainjs-framework-field] - `); + More info: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#mainjs-framework-field] + `); }); describe('when `throwPlayFunctionExceptions` is set', () => { @@ -3548,8 +3547,8 @@ describe('PreviewWeb', () => { const preview = new PreviewWeb(importFn, () => { throw err; }); - await expect(preview.initialize()).rejects.toThrow(err); - await expect(preview.extract()).rejects.toThrow(err); + await expect(preview.initialize()).rejects.toThrow(/meta error/); + await expect(preview.extract()).rejects.toThrow(); }); it('shows an error if the stories.json endpoint 500s', async () => { @@ -3559,7 +3558,7 @@ describe('PreviewWeb', () => { const preview = new PreviewWeb(importFn, getProjectAnnotations); await expect(preview.initialize()).rejects.toThrow('sort error'); - await expect(preview.extract()).rejects.toThrow('sort error'); + await expect(preview.extract()).rejects.toThrow(); }); it('waits for stories to be cached', async () => { From f7e958699d9e2936bccd3f555f2c28b5c071f853 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Wed, 22 Nov 2023 21:58:04 +1100 Subject: [PATCH 07/26] Update code/lib/core-events/src/errors/preview-errors.ts Co-authored-by: Yann Braga --- code/lib/core-events/src/errors/preview-errors.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/code/lib/core-events/src/errors/preview-errors.ts b/code/lib/core-events/src/errors/preview-errors.ts index 5d1485c4ab68..4068033812c5 100644 --- a/code/lib/core-events/src/errors/preview-errors.ts +++ b/code/lib/core-events/src/errors/preview-errors.ts @@ -96,18 +96,15 @@ export class MissingRenderToCanvasError extends StorybookError { readonly category = Category.PREVIEW_API; readonly code = 4; - - constructor(public data: object) { - super(); - } + + readonly documentation = + 'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#mainjs-framework-field'; template() { return dedent` Expected your framework's preset to export a \`renderToCanvas\` field. - Perhaps it needs to be upgraded for Storybook 6.4? - - More info: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#mainjs-framework-field`; + Perhaps it needs to be upgraded for Storybook 6.4?`; } } From 96f837bce60dc88aa1cafc02fdc984ea605086cb Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 23 Nov 2023 16:37:12 +1100 Subject: [PATCH 08/26] Remove `Preview.initialize`, use `Preview.ready()` instead --- .../src/codegen-modern-iframe-script.ts | 1 - .../virtualModuleModernEntry.js.handlebars | 2 - .../src/modules/preview-web/Preview.tsx | 9 ++- .../PreviewWeb.integration.test.ts | 8 +-- .../modules/preview-web/PreviewWeb.test.ts | 64 +++++++++---------- 5 files changed, 44 insertions(+), 40 deletions(-) diff --git a/code/builders/builder-vite/src/codegen-modern-iframe-script.ts b/code/builders/builder-vite/src/codegen-modern-iframe-script.ts index eb9e36b08cc5..39109c148df4 100644 --- a/code/builders/builder-vite/src/codegen-modern-iframe-script.ts +++ b/code/builders/builder-vite/src/codegen-modern-iframe-script.ts @@ -69,7 +69,6 @@ export async function generateModernIframeScriptCode(options: Options, projectRo window.__STORYBOOK_PREVIEW__ = window.__STORYBOOK_PREVIEW__ || new PreviewWeb(importFn, getProjectAnnotations); window.__STORYBOOK_STORY_STORE__ = window.__STORYBOOK_STORY_STORE__ || window.__STORYBOOK_PREVIEW__.storyStore; - window.__STORYBOOK_PREVIEW__.initialize(); ${generateHMRHandler(frameworkName)}; `.trim(); diff --git a/code/builders/builder-webpack5/templates/virtualModuleModernEntry.js.handlebars b/code/builders/builder-webpack5/templates/virtualModuleModernEntry.js.handlebars index 6ef2d9f9647c..1224d3d015df 100644 --- a/code/builders/builder-webpack5/templates/virtualModuleModernEntry.js.handlebars +++ b/code/builders/builder-webpack5/templates/virtualModuleModernEntry.js.handlebars @@ -21,8 +21,6 @@ window.__STORYBOOK_PREVIEW__ = preview; window.__STORYBOOK_STORY_STORE__ = preview.storyStore; window.__STORYBOOK_ADDONS_CHANNEL__ = channel; -preview.initialize(); - if (import.meta.webpackHot) { import.meta.webpackHot.accept('./{{storiesFilename}}', () => { // importFn has changed so we need to patch the new one in diff --git a/code/lib/preview-api/src/modules/preview-web/Preview.tsx b/code/lib/preview-api/src/modules/preview-web/Preview.tsx index c164d412d26e..4894616bbb55 100644 --- a/code/lib/preview-api/src/modules/preview-web/Preview.tsx +++ b/code/lib/preview-api/src/modules/preview-web/Preview.tsx @@ -77,16 +77,23 @@ export class Preview { this.storeInitializationPromise = new Promise((resolve) => { this.resolveStoreInitializationPromise = resolve; }); + + // Cannot await this in constructor, but if you want to await it, use `ready()` + this.initialize(); } // INITIALIZATION - async initialize() { + private async initialize() { this.setupListeners(); const projectAnnotations = await this.getProjectAnnotationsOrRenderError(); await this.initializeWithProjectAnnotations(projectAnnotations); } + ready() { + return this.storeInitializationPromise; + } + setupListeners() { this.channel.on(STORY_INDEX_INVALIDATED, this.onStoryIndexChanged.bind(this)); this.channel.on(UPDATE_GLOBALS, this.onUpdateGlobals.bind(this)); diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.integration.test.ts b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.integration.test.ts index 4f2118d56b68..ccb2bcbfeaf3 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.integration.test.ts +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.integration.test.ts @@ -82,7 +82,7 @@ describe('PreviewWeb', () => { storyFn() ); document.location.search = '?id=component-one--a'; - await new PreviewWeb(importFn, getProjectAnnotations).initialize(); + await new PreviewWeb(importFn, getProjectAnnotations).ready(); await waitForRender(); @@ -103,7 +103,7 @@ describe('PreviewWeb', () => { React.createElement('div', {}, 'INSIDE') ); - await preview.initialize(); + await preview.ready(); await waitForRender(); expect(docsRoot.outerHTML).toMatchInlineSnapshot('"
INSIDE
"'); @@ -126,7 +126,7 @@ describe('PreviewWeb', () => { vi.mocked(preview.view.showErrorDisplay).mockClear(); - await preview.initialize(); + await preview.ready(); await waitForRender(); expect(preview.view.showErrorDisplay).toHaveBeenCalled(); @@ -150,7 +150,7 @@ describe('PreviewWeb', () => { document.location.search = '?id=component-one--a'; const preview = new PreviewWeb(importFn, getProjectAnnotations); - await preview.initialize(); + await preview.ready(); await waitForRender(); projectAnnotations.renderToCanvas.mockImplementationOnce(({ storyFn }: RenderContext) => diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts index dd29a6b9cc46..7f37ad737d22 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts @@ -104,7 +104,7 @@ async function createAndRenderPreview({ getProjectAnnotations?: () => ProjectAnnotations; } = {}) { const preview = new PreviewWeb(inputImportFn, inputGetProjectAnnotations); - await preview.initialize(); + await preview.ready(); await waitForRender(); return preview; @@ -135,13 +135,13 @@ beforeEach(() => { }); describe('PreviewWeb', () => { - describe('initialize', () => { + describe('ready', () => { it('shows an error if getProjectAnnotations throws', async () => { const err = new Error('meta error'); const preview = new PreviewWeb(importFn, () => { throw err; }); - await expect(preview.initialize()).rejects.toThrow(err); + await expect(preview.ready()).rejects.toThrow(err); expect(preview.view.showErrorDisplay).toHaveBeenCalled(); expect(mockChannel.emit).toHaveBeenCalledWith(CONFIG_ERROR, err); @@ -152,7 +152,7 @@ describe('PreviewWeb', () => { mockFetchResult = { status: 500, text: async () => err.toString() }; const preview = new PreviewWeb(importFn, getProjectAnnotations); - await expect(preview.initialize()).rejects.toThrow('sort error'); + await expect(preview.ready()).rejects.toThrow('sort error'); expect(preview.view.showErrorDisplay).toHaveBeenCalled(); expect(mockChannel.emit).toHaveBeenCalledWith(CONFIG_ERROR, expect.any(Error)); @@ -222,7 +222,7 @@ describe('PreviewWeb', () => { const preview = new PreviewWeb(importFn, async () => { return getProjectAnnotations(); }); - await preview.initialize(); + await preview.ready(); expect(preview.storyStore!.globals.get()).toEqual({ a: 'b' }); }); @@ -509,7 +509,7 @@ describe('PreviewWeb', () => { renderToCanvas: undefined, }); const preview = new PreviewWeb(importFn, getProjectAnnotations); - await expect(preview.initialize()).rejects.toThrow(); + await expect(preview.ready()).rejects.toThrow(); expect(preview.view.showErrorDisplay).toHaveBeenCalled(); expect(vi.mocked(preview.view.showErrorDisplay).mock.calls[0][0]).toMatchInlineSnapshot(` @@ -621,7 +621,7 @@ describe('PreviewWeb', () => { }); const preview = new PreviewWeb(importFn, getProjectAnnotations); - await preview.initialize(); + await preview.ready(); await waitForRender(); @@ -925,7 +925,7 @@ describe('PreviewWeb', () => { document.location.search = '?id=component-one--a'; componentOneExports.default.loaders[0].mockImplementationOnce(async () => gate); - await new PreviewWeb(importFn, getProjectAnnotations).initialize(); + await new PreviewWeb(importFn, getProjectAnnotations).ready(); await waitForRenderPhase('loading'); expect(componentOneExports.default.loaders[0]).toHaveBeenCalledWith( @@ -986,7 +986,7 @@ describe('PreviewWeb', () => { document.location.search = '?id=component-one--a'; projectAnnotations.renderToCanvas.mockImplementation(async () => gate); - await new PreviewWeb(importFn, getProjectAnnotations).initialize(); + await new PreviewWeb(importFn, getProjectAnnotations).ready(); await waitForRenderPhase('rendering'); emitter.emit(UPDATE_STORY_ARGS, { @@ -1065,7 +1065,7 @@ describe('PreviewWeb', () => { }); document.location.search = '?id=component-one--a'; - await new PreviewWeb(importFn, getProjectAnnotations).initialize(); + await new PreviewWeb(importFn, getProjectAnnotations).ready(); await waitForRenderPhase('playing'); await renderToCanvasCalled; @@ -1485,7 +1485,7 @@ describe('PreviewWeb', () => { document.location.search = '?id=component-one--a'; projectAnnotations.renderToCanvas.mockImplementation(async () => gate); - await new PreviewWeb(importFn, getProjectAnnotations).initialize(); + await new PreviewWeb(importFn, getProjectAnnotations).ready(); await waitForRenderPhase('rendering'); expect(projectAnnotations.renderToCanvas).toHaveBeenCalledWith( @@ -1580,11 +1580,11 @@ describe('PreviewWeb', () => { expect(mockChannel.emit).toHaveBeenCalledWith(STORY_MISSING, 'random'); }); - describe('if called before the preview is initialized', () => { + describe('if called before the preview is readyd', () => { it('works when there was no selection specifier', async () => { document.location.search = ''; // We intentionally are *not* awaiting here - new PreviewWeb(importFn, getProjectAnnotations).initialize(); + new PreviewWeb(importFn, getProjectAnnotations).ready(); emitter.emit(SET_CURRENT_STORY, { storyId: 'component-one--b', viewMode: 'story' }); @@ -1609,11 +1609,11 @@ describe('PreviewWeb', () => { it('works when there was a selection specifier', async () => { document.location.search = '?id=component-one--a'; - const initialized = new PreviewWeb(importFn, getProjectAnnotations).initialize(); + const readyd = new PreviewWeb(importFn, getProjectAnnotations).ready(); emitter.emit(SET_CURRENT_STORY, { storyId: 'component-one--b', viewMode: 'story' }); - await initialized; + await readyd; await waitForEvents([STORY_RENDERED]); // If we emitted CURRENT_STORY_WAS_SET for the original selection, the manager might @@ -1722,9 +1722,9 @@ describe('PreviewWeb', () => { }); const preview = new PreviewWeb(importFn, getProjectAnnotations); - // We can't wait for the initialize function, as it waits for `renderSelection()` + // We can't wait for the ready function, as it waits for `renderSelection()` // which prepares, but it does emit `CURRENT_STORY_WAS_SET` right before that - preview.initialize(); + preview.ready(); await waitForEvents([CURRENT_STORY_WAS_SET]); mockChannel.emit.mockClear(); @@ -1769,9 +1769,9 @@ describe('PreviewWeb', () => { }); const preview = new PreviewWeb(importFn, getProjectAnnotations); - // We can't wait for the initialize function, as it waits for `renderSelection()` + // We can't wait for the ready function, as it waits for `renderSelection()` // which prepares, but it does emit `CURRENT_STORY_WAS_SET` right before that - preview.initialize(); + preview.ready(); await waitForEvents([CURRENT_STORY_WAS_SET]); mockChannel.emit.mockClear(); @@ -1815,9 +1815,9 @@ describe('PreviewWeb', () => { }); const preview = new PreviewWeb(importFn, getProjectAnnotations); - // We can't wait for the initialize function, as it waits for `renderSelection()` + // We can't wait for the ready function, as it waits for `renderSelection()` // which prepares, but it does emit `CURRENT_STORY_WAS_SET` right before that - preview.initialize(); + preview.ready(); await waitForEvents([CURRENT_STORY_WAS_SET]); mockChannel.emit.mockClear(); @@ -2150,7 +2150,7 @@ describe('PreviewWeb', () => { componentOneExports.default.loaders[0].mockImplementationOnce(async () => gate); document.location.search = '?id=component-one--a'; - await new PreviewWeb(importFn, getProjectAnnotations).initialize(); + await new PreviewWeb(importFn, getProjectAnnotations).ready(); await waitForRenderPhase('loading'); emitter.emit(SET_CURRENT_STORY, { @@ -2183,7 +2183,7 @@ describe('PreviewWeb', () => { document.location.search = '?id=component-one--a'; projectAnnotations.renderToCanvas.mockImplementation(async () => gate); - await new PreviewWeb(importFn, getProjectAnnotations).initialize(); + await new PreviewWeb(importFn, getProjectAnnotations).ready(); await waitForRenderPhase('rendering'); mockChannel.emit.mockClear(); @@ -2217,7 +2217,7 @@ describe('PreviewWeb', () => { componentOneExports.a.play.mockImplementationOnce(async () => gate); document.location.search = '?id=component-one--a'; - await new PreviewWeb(importFn, getProjectAnnotations).initialize(); + await new PreviewWeb(importFn, getProjectAnnotations).ready(); await waitForRenderPhase('playing'); expect(projectAnnotations.renderToCanvas).toHaveBeenCalledWith( @@ -2270,7 +2270,7 @@ describe('PreviewWeb', () => { componentOneExports.a.play.mockImplementationOnce(async () => gate); document.location.search = '?id=component-one--a'; - await new PreviewWeb(importFn, getProjectAnnotations).initialize(); + await new PreviewWeb(importFn, getProjectAnnotations).ready(); await waitForRenderPhase('playing'); expect(projectAnnotations.renderToCanvas).toHaveBeenCalledWith( @@ -2700,7 +2700,7 @@ describe('PreviewWeb', () => { mockFetchResult = { status: 500, text: async () => err.toString() }; const preview = new PreviewWeb(importFn, getProjectAnnotations); - await expect(preview.initialize()).rejects.toThrow('sort error'); + await expect(preview.ready()).rejects.toThrow('sort error'); expect(preview.view.showErrorDisplay).toHaveBeenCalled(); expect(mockChannel.emit).toHaveBeenCalledWith(CONFIG_ERROR, expect.any(Error)); @@ -2718,7 +2718,7 @@ describe('PreviewWeb', () => { mockFetchResult = { status: 500, text: async () => err.toString() }; const preview = new PreviewWeb(importFn, getProjectAnnotations); - await expect(preview.initialize()).rejects.toThrow('sort error'); + await expect(preview.ready()).rejects.toThrow('sort error'); expect(preview.view.showErrorDisplay).toHaveBeenCalled(); expect(mockChannel.emit).toHaveBeenCalledWith(CONFIG_ERROR, expect.any(Error)); @@ -3303,7 +3303,7 @@ describe('PreviewWeb', () => { const preview = new PreviewWeb(importFn, () => { throw err; }); - await expect(preview.initialize()).rejects.toThrow(err); + await expect(preview.ready()).rejects.toThrow(err); preview.onGetProjectAnnotationsChanged({ getProjectAnnotations }); await waitForRender(); @@ -3318,7 +3318,7 @@ describe('PreviewWeb', () => { const preview = new PreviewWeb(importFn, () => { throw err; }); - await expect(preview.initialize()).rejects.toThrow(err); + await expect(preview.ready()).rejects.toThrow(err); preview.onGetProjectAnnotationsChanged({ getProjectAnnotations }); await waitForRender(); @@ -3500,7 +3500,7 @@ describe('PreviewWeb', () => { const [gate, openGate] = createGate(); componentOneExports.a.play.mockImplementationOnce(async () => gate); const preview = new PreviewWeb(importFn, getProjectAnnotations); - await preview.initialize(); + await preview.ready(); await waitForRenderPhase('playing'); await preview.onKeydown({ @@ -3547,7 +3547,7 @@ describe('PreviewWeb', () => { const preview = new PreviewWeb(importFn, () => { throw err; }); - await expect(preview.initialize()).rejects.toThrow(/meta error/); + await expect(preview.ready()).rejects.toThrow(/meta error/); await expect(preview.extract()).rejects.toThrow(); }); @@ -3556,7 +3556,7 @@ describe('PreviewWeb', () => { mockFetchResult = { status: 500, text: async () => err.toString() }; const preview = new PreviewWeb(importFn, getProjectAnnotations); - await expect(preview.initialize()).rejects.toThrow('sort error'); + await expect(preview.ready()).rejects.toThrow('sort error'); await expect(preview.extract()).rejects.toThrow(); }); From f69f96eea64fbaca1a8c34d5f18a16c16c16ddc6 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Fri, 24 Nov 2023 12:31:17 +0100 Subject: [PATCH 09/26] fix --- .../preview-api/src/modules/preview-web/PreviewWithSelection.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx b/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx index a1ba086d161b..8a599615098f 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx @@ -1,5 +1,4 @@ import invariant from 'tiny-invariant'; -import { dedent } from 'ts-dedent'; import { CURRENT_STORY_WAS_SET, DOCS_PREPARED, From 7386fb708726ab2826570e75f8a2f4e22e89c1cc Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Fri, 24 Nov 2023 12:47:51 +0100 Subject: [PATCH 10/26] fixes --- code/lib/preview-api/src/modules/preview-web/Preview.tsx | 2 +- code/lib/preview-api/src/modules/preview-web/PreviewWeb.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/code/lib/preview-api/src/modules/preview-web/Preview.tsx b/code/lib/preview-api/src/modules/preview-web/Preview.tsx index 4894616bbb55..11c760f46a32 100644 --- a/code/lib/preview-api/src/modules/preview-web/Preview.tsx +++ b/code/lib/preview-api/src/modules/preview-web/Preview.tsx @@ -108,7 +108,7 @@ export class Preview { const projectAnnotations = await this.getProjectAnnotations(); this.renderToCanvas = projectAnnotations.renderToCanvas; - if (!this.renderToCanvas) throw new MissingRenderToCanvasError({}); + if (!this.renderToCanvas) throw new MissingRenderToCanvasError(); return projectAnnotations; } catch (err) { diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.tsx b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.tsx index eea7635b9e30..bb458812eb02 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.tsx +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.tsx @@ -1,6 +1,6 @@ /* eslint-disable no-underscore-dangle */ import { global } from '@storybook/global'; -import type { Renderer, ProjectAnnotations, StoryIndex, ModuleImportFn } from '@storybook/types'; +import type { Renderer, ProjectAnnotations, ModuleImportFn } from '@storybook/types'; import { PreviewWithSelection } from './PreviewWithSelection'; import { UrlStore } from './UrlStore'; From 709222e2cc03664fc011ea1d3b0d49539f3f59d7 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Fri, 24 Nov 2023 13:02:47 +0100 Subject: [PATCH 11/26] fixes --- .../modules/preview-web/PreviewWeb.test.ts | 26 ++++++++++--------- .../src/blocks/external/ExternalPreview.ts | 2 ++ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts index 7f37ad737d22..9cb5d745cff0 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts @@ -202,7 +202,7 @@ describe('PreviewWeb', () => { const preview = await createAndRenderPreview(); - expect(preview.storyStore.args.get('component-one--a')).toEqual({ + expect(preview.storyStore?.args.get('component-one--a')).toEqual({ foo: 'url', one: 1, }); @@ -873,7 +873,7 @@ describe('PreviewWeb', () => { updatedArgs: { new: 'arg' }, }); - expect(preview.storyStore.args.get('component-one--a')).toEqual({ + expect(preview.storyStore?.args.get('component-one--a')).toEqual({ foo: 'a', new: 'arg', one: 1, @@ -1132,7 +1132,8 @@ describe('PreviewWeb', () => { await waitForRender(); mockChannel.emit.mockClear(); - const story = await preview.storyStore.loadStory({ storyId: 'component-one--a' }); + const story = await preview.storyStore?.loadStory({ storyId: 'component-one--a' }); + // @ts-expect-error (tom will fix this) preview.renderStoryToElement(story, 'story-element' as any, callbacks, {}); await waitForRender(); @@ -1170,7 +1171,8 @@ describe('PreviewWeb', () => { await waitForRender(); mockChannel.emit.mockClear(); - const story = await preview.storyStore.loadStory({ storyId: 'component-one--a' }); + const story = await preview.storyStore?.loadStory({ storyId: 'component-one--a' }); + // @ts-expect-error (tom will fix this) preview.renderStoryToElement(story, 'story-element' as any, callbacks, { forceInitialArgs: true, }); @@ -2102,7 +2104,7 @@ describe('PreviewWeb', () => { updatedArgs: { foo: 'updated' }, }); await waitForRender(); - expect(preview.storyStore.args.get('component-one--a')).toEqual({ + expect(preview.storyStore?.args.get('component-one--a')).toEqual({ foo: 'updated', one: 1, }); @@ -2114,7 +2116,7 @@ describe('PreviewWeb', () => { }); await waitForSetCurrentStory(); await waitForRender(); - expect(preview.storyStore.args.get('component-one--a')).toEqual({ + expect(preview.storyStore?.args.get('component-one--a')).toEqual({ foo: 'updated', one: 1, }); @@ -2126,7 +2128,7 @@ describe('PreviewWeb', () => { }); await waitForSetCurrentStory(); await waitForRender(); - expect(preview.storyStore.args.get('component-one--a')).toEqual({ + expect(preview.storyStore?.args.get('component-one--a')).toEqual({ foo: 'updated', one: 1, }); @@ -2727,7 +2729,7 @@ describe('PreviewWeb', () => { mockFetchResult = { status: 200, json: mockStoryIndex, text: () => 'error text' }; preview.onStoryIndexChanged(); await waitForRender(); - expect(preview.storyStore.args.get('component-one--a')).toEqual({ + expect(preview.storyStore?.args.get('component-one--a')).toEqual({ foo: 'url', one: 1, }); @@ -3133,7 +3135,7 @@ describe('PreviewWeb', () => { }); await waitForSetCurrentStory(); await waitForRender(); - expect(preview.storyStore.args.get('component-one--a')).toEqual({ + expect(preview.storyStore?.args.get('component-one--a')).toEqual({ foo: 'updated', one: 1, }); @@ -3152,7 +3154,7 @@ describe('PreviewWeb', () => { }); await waitForSetCurrentStory(); await waitForRender(); - expect(preview.storyStore.args.get('component-one--a')).toEqual({ + expect(preview.storyStore?.args.get('component-one--a')).toEqual({ foo: 'updated', bar: 'edited', one: 1, @@ -3399,7 +3401,7 @@ describe('PreviewWeb', () => { preview.onGetProjectAnnotationsChanged({ getProjectAnnotations: newGetProjectAnnotations }); await waitForRender(); - expect(preview.storyStore.args.get('component-one--a')).toEqual({ + expect(preview.storyStore?.args.get('component-one--a')).toEqual({ foo: 'a', one: 1, global: 'added', @@ -3524,7 +3526,7 @@ describe('PreviewWeb', () => { componentOneExports.b.play.mockImplementationOnce(async () => gate); // @ts-expect-error (not strict) preview.renderStoryToElement( - await preview.storyStore.loadStory({ storyId: 'component-one--b' }), + await preview.storyStore?.loadStory({ storyId: 'component-one--b' }), {} as any ); await waitForRenderPhase('playing'); diff --git a/code/ui/blocks/src/blocks/external/ExternalPreview.ts b/code/ui/blocks/src/blocks/external/ExternalPreview.ts index 52d8e90378d7..e476acb9428c 100644 --- a/code/ui/blocks/src/blocks/external/ExternalPreview.ts +++ b/code/ui/blocks/src/blocks/external/ExternalPreview.ts @@ -36,8 +36,10 @@ export class ExternalPreview extends Prev private moduleExportsByImportPath: Record = {}; constructor(public projectAnnotations: ProjectAnnotations) { + // @ts-expect-error (tom will fix this) super(new Channel({})); + // @ts-expect-error (tom will fix this) this.initialize({ getStoryIndex: () => this.storyIndex, importFn: (path: Path) => { From 24ec1314827a6903e2da1e78387c3108808582b6 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Wed, 20 Dec 2023 22:53:25 +1100 Subject: [PATCH 12/26] Ensure we reject `.ready()` if initialization fails --- .../src/modules/preview-web/Preview.tsx | 21 +++++++++++++------ .../preview-web/PreviewWithSelection.tsx | 5 ++++- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/code/lib/preview-api/src/modules/preview-web/Preview.tsx b/code/lib/preview-api/src/modules/preview-web/Preview.tsx index 11c760f46a32..845a65923a18 100644 --- a/code/lib/preview-api/src/modules/preview-web/Preview.tsx +++ b/code/lib/preview-api/src/modules/preview-web/Preview.tsx @@ -67,27 +67,36 @@ export class Preview { protected resolveStoreInitializationPromise!: () => void; + protected rejectStoreInitializationPromise!: (err: Error) => void; + constructor( public importFn: ModuleImportFn, public getProjectAnnotations: () => MaybePromise>, - protected channel: Channel = addons.getChannel() + protected channel: Channel = addons.getChannel(), + + shouldInitialize = true ) { - this.storeInitializationPromise = new Promise((resolve) => { + this.storeInitializationPromise = new Promise((resolve, reject) => { this.resolveStoreInitializationPromise = resolve; + this.rejectStoreInitializationPromise = reject; }); // Cannot await this in constructor, but if you want to await it, use `ready()` - this.initialize(); + if (shouldInitialize) this.initialize(); } // INITIALIZATION - private async initialize() { + protected async initialize() { this.setupListeners(); - const projectAnnotations = await this.getProjectAnnotationsOrRenderError(); - await this.initializeWithProjectAnnotations(projectAnnotations); + try { + const projectAnnotations = await this.getProjectAnnotationsOrRenderError(); + await this.initializeWithProjectAnnotations(projectAnnotations); + } catch (err) { + this.rejectStoreInitializationPromise(err as Error); + } } ready() { diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx b/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx index 8a599615098f..cebd442cba54 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx @@ -99,7 +99,10 @@ export class PreviewWithSelection extends Preview ) { - super(importFn, getProjectAnnotations); + // We need to call initialize ourself (i.e. stop super() from doing it, with false) + // because otherwise this.view will not get set in time. + super(importFn, getProjectAnnotations, undefined, false); + this.initialize(); } setupListeners() { From cbf2095864074716fd87e899a769c2c33b850c62 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Wed, 20 Dec 2023 22:53:42 +1100 Subject: [PATCH 13/26] Fix error message and loaders behavior in PreviewWeb.test.ts --- .../src/modules/preview-web/PreviewWeb.test.ts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts index 9cb5d745cff0..0bcc3b967e7f 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts @@ -513,11 +513,12 @@ describe('PreviewWeb', () => { expect(preview.view.showErrorDisplay).toHaveBeenCalled(); expect(vi.mocked(preview.view.showErrorDisplay).mock.calls[0][0]).toMatchInlineSnapshot(` - [SB_PREVIEW_API_0004 (MissingRenderToCanvasError): Expected your framework's preset to export a \`renderToCanvas\` field. + [SB_PREVIEW_API_0005 (MissingRenderToCanvasError): Expected your framework's preset to export a \`renderToCanvas\` field. Perhaps it needs to be upgraded for Storybook 6.4? - More info: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#mainjs-framework-field] + More info: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#mainjs-framework-field + ] `); }); @@ -920,13 +921,17 @@ describe('PreviewWeb', () => { describe('while story is still rendering', () => { it('runs loaders again', async () => { - const [gate, openGate] = createGate(); + const [loadersRanGate, openLoadersRanGate] = createGate(); + const [blockLoadersGate, openBlockLoadersGate] = createGate(); document.location.search = '?id=component-one--a'; - componentOneExports.default.loaders[0].mockImplementationOnce(async () => gate); + componentOneExports.default.loaders[0].mockImplementationOnce(async () => { + openLoadersRanGate(); + return blockLoadersGate; + }); await new PreviewWeb(importFn, getProjectAnnotations).ready(); - await waitForRenderPhase('loading'); + await loadersRanGate; expect(componentOneExports.default.loaders[0]).toHaveBeenCalledWith( expect.objectContaining({ @@ -963,7 +968,7 @@ describe('PreviewWeb', () => { // Now let the first loader call resolve mockChannel.emit.mockClear(); projectAnnotations.renderToCanvas.mockClear(); - openGate({ l: 8 }); + openBlockLoadersGate({ l: 8 }); await waitForRender(); // Now the first call comes through, but picks up the new args From 7e3c37625b62be28e3bc34f90d70943f22b4ec12 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Fri, 5 Jan 2024 14:43:49 +0100 Subject: [PATCH 14/26] fixing tests --- .../modules/preview-web/PreviewWeb.test.ts | 39 ++++++------------- .../preview-web/render/CsfDocsRender.ts | 1 + .../src/modules/store/StoryStore.test.ts | 2 + 3 files changed, 14 insertions(+), 28 deletions(-) diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts index 0bcc3b967e7f..99fb5464c54f 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts @@ -614,24 +614,6 @@ describe('PreviewWeb', () => { expect(mockChannel.emit).toHaveBeenCalledWith(STORY_RENDERED, 'component-one--a'); }); - - it('does not show error display if the render function throws IGNORED_EXCEPTION', async () => { - document.location.search = '?id=component-one--a'; - projectAnnotations.renderToCanvas.mockImplementation(() => { - throw IGNORED_EXCEPTION; - }); - - const preview = new PreviewWeb(importFn, getProjectAnnotations); - await preview.ready(); - - await waitForRender(); - - expect(mockChannel.emit).toHaveBeenCalledWith( - STORY_THREW_EXCEPTION, - serializeError(IGNORED_EXCEPTION) - ); - expect(preview.view.showErrorDisplay).not.toHaveBeenCalled(); - }); }); describe('CSF docs entries', () => { @@ -2442,36 +2424,37 @@ describe('PreviewWeb', () => { }); describe('when changing from docs viewMode to story', () => { - it('updates URL', async () => { + it('unmounts docs', async () => { document.location.search = '?id=component-one--docs&viewMode=docs'; await createAndRenderPreview(); + mockChannel.emit.mockClear(); emitter.emit(SET_CURRENT_STORY, { storyId: 'component-one--a', viewMode: 'story', }); await waitForSetCurrentStory(); + await waitForRender(); - expect(history.replaceState).toHaveBeenCalledWith( - {}, - '', - 'pathname?id=component-one--a&viewMode=story' - ); + expect(docsRenderer.unmount).toHaveBeenCalled(); }); - it('unmounts docs', async () => { + // this test seems to somehow affect the test above, I re-ordered them to get it green, but someone should look into this + it('updates URL', async () => { document.location.search = '?id=component-one--docs&viewMode=docs'; await createAndRenderPreview(); - mockChannel.emit.mockClear(); emitter.emit(SET_CURRENT_STORY, { storyId: 'component-one--a', viewMode: 'story', }); await waitForSetCurrentStory(); - await waitForRender(); - expect(docsRenderer.unmount).toHaveBeenCalled(); + expect(history.replaceState).toHaveBeenCalledWith( + {}, + '', + 'pathname?id=component-one--a&viewMode=story' + ); }); // NOTE: I am not sure this entirely makes sense but this is the behaviour from 6.3 diff --git a/code/lib/preview-api/src/modules/preview-web/render/CsfDocsRender.ts b/code/lib/preview-api/src/modules/preview-web/render/CsfDocsRender.ts index 019d43d97893..5ba148e51a67 100644 --- a/code/lib/preview-api/src/modules/preview-web/render/CsfDocsRender.ts +++ b/code/lib/preview-api/src/modules/preview-web/render/CsfDocsRender.ts @@ -138,6 +138,7 @@ export class CsfDocsRender implements Render renderDocs(); + console.log('what'); this.teardownRender = async ({ viewModeChanged }: { viewModeChanged?: boolean }) => { if (!viewModeChanged || !canvasElement) return; renderer.unmount(canvasElement); diff --git a/code/lib/preview-api/src/modules/store/StoryStore.test.ts b/code/lib/preview-api/src/modules/store/StoryStore.test.ts index 9927476d01f1..187f5b49ccf5 100644 --- a/code/lib/preview-api/src/modules/store/StoryStore.test.ts +++ b/code/lib/preview-api/src/modules/store/StoryStore.test.ts @@ -26,6 +26,8 @@ vi.mock('@storybook/global', async (importOriginal) => ({ }, })); +vi.mock('@storybook/client-logger'); + const createGate = (): [Promise, (_?: any) => void] => { let openGate = (_?: any) => {}; const gate = new Promise((resolve) => { From 3e7e6b476510dd6c282d1282c05e6352e26c63fd Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 11 Jan 2024 23:30:06 +1100 Subject: [PATCH 15/26] Create a proxy `storyStore` object on the preview. And warn if accessed --- .../core-events/src/errors/preview-errors.ts | 24 +++--- .../src/modules/preview-web/Preview.tsx | 76 ++++++++++++------- .../preview-web/PreviewWithSelection.tsx | 37 +++++---- .../src/modules/store/StoryStore.ts | 2 +- 4 files changed, 84 insertions(+), 55 deletions(-) diff --git a/code/lib/core-events/src/errors/preview-errors.ts b/code/lib/core-events/src/errors/preview-errors.ts index 4068033812c5..fb33c42688b3 100644 --- a/code/lib/core-events/src/errors/preview-errors.ts +++ b/code/lib/core-events/src/errors/preview-errors.ts @@ -80,10 +80,6 @@ export class CalledExtractOnStoreError extends StorybookError { readonly code = 3; - constructor(public data: object) { - super(); - } - template() { return dedent` Cannot call \`storyStore.extract()\` without calling \`storyStore.cacheAllCsfFiles()\` first. @@ -96,7 +92,7 @@ export class MissingRenderToCanvasError extends StorybookError { readonly category = Category.PREVIEW_API; readonly code = 4; - + readonly documentation = 'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#mainjs-framework-field'; @@ -176,10 +172,6 @@ export class EmptyIndexError extends StorybookError { readonly code = 8; - constructor(public data: object) { - super(); - } - template() { return dedent` Couldn't find any stories in your Storybook. @@ -229,3 +221,17 @@ export class MissingStoryFromCsfFileError extends StorybookError { Please check your browser console and terminal for errors that may explain the issue.`; } } + +export class StoryStoreAccessedBeforeInitializationError extends StorybookError { + readonly category = Category.PREVIEW_API; + + readonly code = 11; + + template() { + return dedent` + Cannot access the Story Store until the index is ready. + + It is not recommended to use methods directly on the Story Store anyway, in Storybook 9 we will + remove access to the store entirely`; + } +} diff --git a/code/lib/preview-api/src/modules/preview-web/Preview.tsx b/code/lib/preview-api/src/modules/preview-web/Preview.tsx index 845a65923a18..30fbf2650205 100644 --- a/code/lib/preview-api/src/modules/preview-web/Preview.tsx +++ b/code/lib/preview-api/src/modules/preview-web/Preview.tsx @@ -1,4 +1,5 @@ import { global } from '@storybook/global'; +import { deprecate, logger } from '@storybook/client-logger'; import { CONFIG_ERROR, FORCE_REMOUNT, @@ -11,7 +12,6 @@ import { UPDATE_GLOBALS, UPDATE_STORY_ARGS, } from '@storybook/core-events'; -import { logger } from '@storybook/client-logger'; import type { Channel } from '@storybook/channels'; import type { Renderer, @@ -31,6 +31,7 @@ import { CalledPreviewMethodBeforeInitializationError, MissingRenderToCanvasError, StoryIndexFetchError, + StoryStoreAccessedBeforeInitializationError, } from '@storybook/core-events/preview-errors'; import { addons } from '../addons'; import { StoryStore } from '../../store'; @@ -51,7 +52,7 @@ export class Preview { */ serverChannel?: Channel; - storyStore?: StoryStore; + protected storyStoreValue?: StoryStore; renderToCanvas?: RenderToCanvas; @@ -87,6 +88,29 @@ export class Preview { if (shouldInitialize) this.initialize(); } + // Create a proxy object for `__STORYBOOK_STORY_STORE__` and `__STORYBOOK_PREVIEW__.storyStore` + // That proxies through to the store once ready, and errors beforehand. This means we can set + // `__STORYBOOK_STORY_STORE__ = __STORYBOOK_PREVIEW__.storyStore` without having to wait, and + // simiarly integrators can access the `storyStore` on the preview at any time, although + // it is considered deprecated and we will no longer allow access in 9.0 + get storyStore() { + return new Proxy( + {}, + { + get: (_, method) => { + if (this.storyStoreValue) { + deprecate('Accessing the Story Store is deprecated and will be removed in 9.0'); + + // @ts-expect-error I'm not sure if there's a way to keep TS happy here + return this.storyStoreValue[method]; + } + + throw new StoryStoreAccessedBeforeInitializationError(); + }, + } + ); + } + // INITIALIZATION protected async initialize() { this.setupListeners(); @@ -156,7 +180,7 @@ export class Preview { // eslint-disable-next-line local-rules/no-uncategorized-errors throw new Error('Cannot call initializeWithStoryIndex until project annotations resolve'); - this.storyStore = new StoryStore( + this.storyStoreValue = new StoryStore( storyIndex, this.importFn, this.projectAnnotationsBeforeInitialization @@ -173,12 +197,12 @@ export class Preview { } emitGlobals() { - if (!this.storyStore) + if (!this.storyStoreValue) throw new CalledPreviewMethodBeforeInitializationError({ methodName: 'emitGlobals' }); const payload: SetGlobalsPayload = { - globals: this.storyStore.globals.get() || {}, - globalTypes: this.storyStore.projectAnnotations.globalTypes || {}, + globals: this.storyStoreValue.globals.get() || {}, + globalTypes: this.storyStoreValue.projectAnnotations.globalTypes || {}, }; this.channel.emit(SET_GLOBALS, payload); } @@ -195,12 +219,12 @@ export class Preview { this.getProjectAnnotations = getProjectAnnotations; const projectAnnotations = await this.getProjectAnnotationsOrRenderError(); - if (!this.storyStore) { + if (!this.storyStoreValue) { await this.initializeWithProjectAnnotations(projectAnnotations); return; } - this.storyStore.setProjectAnnotations(projectAnnotations); + this.storyStoreValue.setProjectAnnotations(projectAnnotations); this.emitGlobals(); } @@ -209,7 +233,7 @@ export class Preview { // We haven't successfully set project annotations yet, // we need to do that before we can do anything else. - if (!this.storyStore && !this.projectAnnotationsBeforeInitialization) { + if (!this.storyStoreValue && !this.projectAnnotationsBeforeInitialization) { return; } @@ -238,28 +262,28 @@ export class Preview { importFn?: ModuleImportFn; storyIndex?: StoryIndex; }) { - if (!this.storyStore) + if (!this.storyStoreValue) throw new CalledPreviewMethodBeforeInitializationError({ methodName: 'onStoriesChanged' }); - await this.storyStore.onStoriesChanged({ importFn, storyIndex }); + await this.storyStoreValue.onStoriesChanged({ importFn, storyIndex }); } async onUpdateGlobals({ globals }: { globals: Globals }) { - if (!this.storyStore) + if (!this.storyStoreValue) throw new CalledPreviewMethodBeforeInitializationError({ methodName: 'onUpdateGlobals' }); - this.storyStore.globals.update(globals); + this.storyStoreValue.globals.update(globals); await Promise.all(this.storyRenders.map((r) => r.rerender())); this.channel.emit(GLOBALS_UPDATED, { - globals: this.storyStore.globals.get(), - initialGlobals: this.storyStore.globals.initialGlobals, + globals: this.storyStoreValue.globals.get(), + initialGlobals: this.storyStoreValue.globals.initialGlobals, }); } async onUpdateArgs({ storyId, updatedArgs }: { storyId: StoryId; updatedArgs: Args }) { - if (!this.storyStore) + if (!this.storyStoreValue) throw new CalledPreviewMethodBeforeInitializationError({ methodName: 'onUpdateArgs' }); - this.storyStore.args.update(storyId, updatedArgs); + this.storyStoreValue.args.update(storyId, updatedArgs); await Promise.all( this.storyRenders @@ -269,12 +293,12 @@ export class Preview { this.channel.emit(STORY_ARGS_UPDATED, { storyId, - args: this.storyStore.args.get(storyId), + args: this.storyStoreValue.args.get(storyId), }); } async onResetArgs({ storyId, argNames }: { storyId: string; argNames?: string[] }) { - if (!this.storyStore) + if (!this.storyStoreValue) throw new CalledPreviewMethodBeforeInitializationError({ methodName: 'onResetArgs' }); // NOTE: we have to be careful here and avoid await-ing when updating a rendered's args. @@ -282,12 +306,12 @@ export class Preview { // render the story in the same tick. // However, we can do that safely as the current story is available in `this.storyRenders` const render = this.storyRenders.find((r) => r.id === storyId); - const story = render?.story || (await this.storyStore.loadStory({ storyId })); + const story = render?.story || (await this.storyStoreValue.loadStory({ storyId })); const argNamesToReset = argNames || [ ...new Set([ ...Object.keys(story.initialArgs), - ...Object.keys(this.storyStore.args.get(storyId)), + ...Object.keys(this.storyStoreValue.args.get(storyId)), ]), ]; @@ -320,14 +344,14 @@ export class Preview { callbacks: RenderContextCallbacks, options: StoryRenderOptions ) { - if (!this.renderToCanvas || !this.storyStore) + if (!this.renderToCanvas || !this.storyStoreValue) throw new CalledPreviewMethodBeforeInitializationError({ methodName: 'renderStoryToElement', }); const render = new StoryRender( this.channel, - this.storyStore, + this.storyStoreValue, this.renderToCanvas, callbacks, story.id, @@ -354,14 +378,14 @@ export class Preview { // API async extract(options?: { includeDocsOnly: boolean }) { - if (!this.storyStore) + if (!this.storyStoreValue) throw new CalledPreviewMethodBeforeInitializationError({ methodName: 'extract' }); if (this.previewEntryError) throw this.previewEntryError; - await this.storyStore.cacheAllCSFFiles(); + await this.storyStoreValue.cacheAllCSFFiles(); - return this.storyStore.extract(options); + return this.storyStoreValue.extract(options); } // UTILITIES diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx b/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx index cebd442cba54..76f17e2419f8 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWithSelection.tsx @@ -116,12 +116,12 @@ export class PreviewWithSelection extends Preview extends Preview extends Preview extends Preview extends Preview storyStore.loadEntry(id))); + await Promise.allSettled(ids.map((id) => storyStoreValue.loadEntry(id))); } // RENDERING @@ -274,7 +274,7 @@ export class PreviewWithSelection extends Preview extends Preview extends Preview( this.channel, - this.storyStore, + this.storyStoreValue, (...args: Parameters) => { // At the start of renderToCanvas we make the story visible (see note in WebView) this.view.showStoryDuringRender(); @@ -329,14 +329,14 @@ export class PreviewWithSelection extends Preview( this.channel, - this.storyStore, + this.storyStoreValue, entry, this.mainStoryCallbacks(storyId) ); } else { render = new CsfDocsRender( this.channel, - this.storyStore, + this.storyStoreValue, entry, this.mainStoryCallbacks(storyId) ); @@ -366,7 +366,7 @@ export class PreviewWithSelection extends Preview extends Preview extends Preview { options: { includeDocsOnly?: boolean } = { includeDocsOnly: false } ): Record> { const { cachedCSFFiles } = this; - if (!cachedCSFFiles) throw new CalledExtractOnStoreError({}); + if (!cachedCSFFiles) throw new CalledExtractOnStoreError(); return Object.entries(this.storyIndex.entries).reduce( (acc, [storyId, { type, importPath }]) => { From bc1e4cdc9655522296ab345b5328fe924cd93624 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 11 Jan 2024 23:46:58 +1100 Subject: [PATCH 16/26] Update a11y addon to not use story store --- code/addons/a11y/src/a11yRunner.ts | 25 +++++-------------- code/addons/a11y/src/components/A11YPanel.tsx | 10 ++++++-- .../a11y/src/components/A11yContext.tsx | 3 ++- 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/code/addons/a11y/src/a11yRunner.ts b/code/addons/a11y/src/a11yRunner.ts index fb32e0f543a0..f37c1b5e5f7c 100644 --- a/code/addons/a11y/src/a11yRunner.ts +++ b/code/addons/a11y/src/a11yRunner.ts @@ -11,22 +11,21 @@ let active = false; // Holds latest story we requested a run let activeStoryId: string | undefined; +const defaultParameters = { config: {}, options: {} }; + /** * Handle A11yContext events. * Because the event are sent without manual check, we split calls */ -const handleRequest = async (storyId: string) => { - const { manual } = await getParams(storyId); - if (!manual) { - await run(storyId); +const handleRequest = async (storyId: string, input: A11yParameters = defaultParameters) => { + if (!input?.manual) { + await run(storyId, input); } }; -const run = async (storyId: string) => { +const run = async (storyId: string, input: A11yParameters = defaultParameters) => { activeStoryId = storyId; try { - const input = await getParams(storyId); - if (!active) { active = true; channel.emit(EVENTS.RUNNING); @@ -69,17 +68,5 @@ const run = async (storyId: string) => { } }; -/** Returns story parameters or default ones. */ -const getParams = async (storyId: string): Promise => { - const { parameters } = - (await globalWindow.__STORYBOOK_STORY_STORE__.loadStory({ storyId })) || {}; - return ( - parameters.a11y || { - config: {}, - options: {}, - } - ); -}; - channel.on(EVENTS.REQUEST, handleRequest); channel.on(EVENTS.MANUAL, run); diff --git a/code/addons/a11y/src/components/A11YPanel.tsx b/code/addons/a11y/src/components/A11YPanel.tsx index 9552b7951e9d..5f1cf4627abe 100644 --- a/code/addons/a11y/src/components/A11YPanel.tsx +++ b/code/addons/a11y/src/components/A11YPanel.tsx @@ -6,7 +6,12 @@ import { ActionBar, ScrollArea } from '@storybook/components'; import { SyncIcon, CheckIcon } from '@storybook/icons'; import type { AxeResults } from 'axe-core'; -import { useChannel, useParameter, useStorybookState } from '@storybook/manager-api'; +import { + useChannel, + useParameter, + useStorybookApi, + useStorybookState, +} from '@storybook/manager-api'; import { Report } from './Report'; @@ -59,6 +64,7 @@ export const A11YPanel: React.FC = () => { const [error, setError] = React.useState(undefined); const { setResults, results } = useA11yContext(); const { storyId } = useStorybookState(); + const api = useStorybookApi(); React.useEffect(() => { setStatus(manual ? 'manual' : 'initial'); @@ -92,7 +98,7 @@ export const A11YPanel: React.FC = () => { const handleManual = useCallback(() => { setStatus('running'); - emit(EVENTS.MANUAL, storyId); + emit(EVENTS.MANUAL, storyId, api.getParameters(storyId, 'a11y')); }, [storyId]); const manualActionItems = useMemo( diff --git a/code/addons/a11y/src/components/A11yContext.tsx b/code/addons/a11y/src/components/A11yContext.tsx index 8410a646ce65..5c7582604213 100644 --- a/code/addons/a11y/src/components/A11yContext.tsx +++ b/code/addons/a11y/src/components/A11yContext.tsx @@ -5,6 +5,7 @@ import { useChannel, useAddonState, useStorybookApi } from '@storybook/manager-a import { STORY_CHANGED, STORY_RENDERED } from '@storybook/core-events'; import { HIGHLIGHT } from '@storybook/addon-highlight'; import { ADDON_ID, EVENTS } from '../constants'; +import type { A11yParameters } from '../params'; export interface Results { passes: Result[]; @@ -70,7 +71,7 @@ export const A11yContextProvider: React.FC { - emit(EVENTS.REQUEST, renderedStoryId); + emit(EVENTS.REQUEST, renderedStoryId, api.getParameters(renderedStoryId, 'a11y')); }; const handleClearHighlights = React.useCallback(() => setHighlighted([]), []); const handleSetTab = React.useCallback((index: number) => { From 67ea6d14489ef6de038e24c2845cf64762d9fff0 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Sat, 13 Jan 2024 22:41:54 +1100 Subject: [PATCH 17/26] Fix some tests --- code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts | 2 +- code/lib/preview-api/src/modules/store/StoryStore.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts index 99fb5464c54f..ec3aa4c3af81 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts @@ -513,7 +513,7 @@ describe('PreviewWeb', () => { expect(preview.view.showErrorDisplay).toHaveBeenCalled(); expect(vi.mocked(preview.view.showErrorDisplay).mock.calls[0][0]).toMatchInlineSnapshot(` - [SB_PREVIEW_API_0005 (MissingRenderToCanvasError): Expected your framework's preset to export a \`renderToCanvas\` field. + [SB_PREVIEW_API_0004 (MissingRenderToCanvasError): Expected your framework's preset to export a \`renderToCanvas\` field. Perhaps it needs to be upgraded for Storybook 6.4? diff --git a/code/lib/preview-api/src/modules/store/StoryStore.test.ts b/code/lib/preview-api/src/modules/store/StoryStore.test.ts index 187f5b49ccf5..7d796970b560 100644 --- a/code/lib/preview-api/src/modules/store/StoryStore.test.ts +++ b/code/lib/preview-api/src/modules/store/StoryStore.test.ts @@ -457,7 +457,7 @@ describe('StoryStore', () => { it('throws if you have not called cacheAllCSFFiles', async () => { const store = new StoryStore(storyIndex, importFn, projectAnnotations); - expect(() => store.extract()).toThrow(/Cannot call extract/); + expect(() => store.extract()).toThrow(/Cannot call/); }); it('produces objects with functions and hooks stripped', async () => { From f73756e46d7660da158e209feae813b6b8192b4e Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Sat, 13 Jan 2024 22:42:19 +1100 Subject: [PATCH 18/26] Remove random log --- .../preview-api/src/modules/preview-web/render/CsfDocsRender.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/code/lib/preview-api/src/modules/preview-web/render/CsfDocsRender.ts b/code/lib/preview-api/src/modules/preview-web/render/CsfDocsRender.ts index 5ba148e51a67..019d43d97893 100644 --- a/code/lib/preview-api/src/modules/preview-web/render/CsfDocsRender.ts +++ b/code/lib/preview-api/src/modules/preview-web/render/CsfDocsRender.ts @@ -138,7 +138,6 @@ export class CsfDocsRender implements Render renderDocs(); - console.log('what'); this.teardownRender = async ({ viewModeChanged }: { viewModeChanged?: boolean }) => { if (!viewModeChanged || !canvasElement) return; renderer.unmount(canvasElement); From bf01aa7218da0378d8d07d65910ce61610164def Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Sat, 13 Jan 2024 22:45:10 +1100 Subject: [PATCH 19/26] Fix a11y tests --- code/addons/a11y/src/components/A11yContext.test.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/code/addons/a11y/src/components/A11yContext.test.tsx b/code/addons/a11y/src/components/A11yContext.test.tsx index 5cfde1886953..2269b8071908 100644 --- a/code/addons/a11y/src/components/A11yContext.test.tsx +++ b/code/addons/a11y/src/components/A11yContext.test.tsx @@ -57,6 +57,7 @@ describe('A11YPanel', () => { }); const getCurrentStoryData = vi.fn(); + const getParameters = vi.fn(); beforeEach(() => { mockedApi.useChannel.mockReset(); mockedApi.useStorybookApi.mockReset(); @@ -65,7 +66,8 @@ describe('A11YPanel', () => { mockedApi.useAddonState.mockImplementation((_, defaultState) => React.useState(defaultState)); mockedApi.useChannel.mockReturnValue(vi.fn()); getCurrentStoryData.mockReset().mockReturnValue({ id: storyId, type: 'story' }); - mockedApi.useStorybookApi.mockReturnValue({ getCurrentStoryData } as any); + getParameters.mockReturnValue({}); + mockedApi.useStorybookApi.mockReturnValue({ getCurrentStoryData, getParameters } as any); }); it('should render children', () => { @@ -94,7 +96,7 @@ describe('A11YPanel', () => { mockedApi.useChannel.mockReturnValue(emit); const { rerender } = render(); rerender(); - expect(emit).toHaveBeenLastCalledWith(EVENTS.REQUEST, storyId); + expect(emit).toHaveBeenLastCalledWith(EVENTS.REQUEST, storyId, {}); }); it('should emit highlight with no values when inactive', () => { From 11108d950725cdcefb82062a785d00bed15914cf Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Sun, 14 Jan 2024 22:40:07 +1100 Subject: [PATCH 20/26] Fix ExternalPreview --- .../src/blocks/external/ExternalPreview.ts | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/code/ui/blocks/src/blocks/external/ExternalPreview.ts b/code/ui/blocks/src/blocks/external/ExternalPreview.ts index e476acb9428c..bb6fc24764f2 100644 --- a/code/ui/blocks/src/blocks/external/ExternalPreview.ts +++ b/code/ui/blocks/src/blocks/external/ExternalPreview.ts @@ -36,21 +36,19 @@ export class ExternalPreview extends Prev private moduleExportsByImportPath: Record = {}; constructor(public projectAnnotations: ProjectAnnotations) { - // @ts-expect-error (tom will fix this) - super(new Channel({})); - - // @ts-expect-error (tom will fix this) - this.initialize({ - getStoryIndex: () => this.storyIndex, - importFn: (path: Path) => { - return Promise.resolve(this.moduleExportsByImportPath[path]); - }, - getProjectAnnotations: () => - composeConfigs([ - { parameters: { docs: { story: { inline: true } } } }, - this.projectAnnotations, - ]), - }); + const importFn = (path: Path) => { + return Promise.resolve(this.moduleExportsByImportPath[path]); + }; + const getProjectAnnotations = () => + composeConfigs([ + { parameters: { docs: { story: { inline: true } } } }, + this.projectAnnotations, + ]); + super(importFn, getProjectAnnotations, new Channel({})); + } + + async getStoryIndexFromServer() { + return this.storyIndex; } processMetaExports = (metaExports: MetaExports) => { @@ -59,7 +57,7 @@ export class ExternalPreview extends Prev const title = metaExports.default.title || this.titles.get(metaExports); - const csfFile = this.storyStore.processCSFFileWithCache( + const csfFile = this.storyStoreValue.processCSFFileWithCache( metaExports, importPath, title @@ -83,7 +81,7 @@ export class ExternalPreview extends Prev docsContext = () => { return new ExternalDocsContext( this.channel, - this.storyStore, + this.storyStoreValue, this.renderStoryToElement.bind(this), this.processMetaExports.bind(this) ); From e324ed5730034567384cc322a8c68f218d511577 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Sun, 14 Jan 2024 22:57:41 +1100 Subject: [PATCH 21/26] Fix unexpected formatting issues --- code/addons/docs/src/preview.ts | 17 ++++++++++------- code/lib/cli/src/upgrade.test.ts | 11 +++++++---- .../core-server/src/presets/common-manager.ts | 17 ++++++++++------- 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/code/addons/docs/src/preview.ts b/code/addons/docs/src/preview.ts index d983f454ccd2..991a7811b472 100644 --- a/code/addons/docs/src/preview.ts +++ b/code/addons/docs/src/preview.ts @@ -1,13 +1,16 @@ import type { PreparedStory } from '@storybook/types'; import { global } from '@storybook/global'; -const excludeTags = Object.entries(global.TAGS_OPTIONS ?? {}).reduce((acc, entry) => { - const [tag, option] = entry; - if ((option as any).excludeFromDocsStories) { - acc[tag] = true; - } - return acc; -}, {} as Record); +const excludeTags = Object.entries(global.TAGS_OPTIONS ?? {}).reduce( + (acc, entry) => { + const [tag, option] = entry; + if ((option as any).excludeFromDocsStories) { + acc[tag] = true; + } + return acc; + }, + {} as Record +); export const parameters: any = { docs: { diff --git a/code/lib/cli/src/upgrade.test.ts b/code/lib/cli/src/upgrade.test.ts index 69b85cbc1d2a..be025028c495 100644 --- a/code/lib/cli/src/upgrade.test.ts +++ b/code/lib/cli/src/upgrade.test.ts @@ -11,10 +11,13 @@ vi.mock('@storybook/telemetry'); vi.mock('./versions', async (importOriginal) => { const originalVersions = ((await importOriginal()) as { default: typeof versions }).default; return { - default: Object.keys(originalVersions).reduce((acc, key) => { - acc[key] = '8.0.0'; - return acc; - }, {} as Record), + default: Object.keys(originalVersions).reduce( + (acc, key) => { + acc[key] = '8.0.0'; + return acc; + }, + {} as Record + ), }; }); diff --git a/code/lib/core-server/src/presets/common-manager.ts b/code/lib/core-server/src/presets/common-manager.ts index 081722917468..0564f8e00b92 100644 --- a/code/lib/core-server/src/presets/common-manager.ts +++ b/code/lib/core-server/src/presets/common-manager.ts @@ -6,13 +6,16 @@ const STATIC_FILTER = 'static-filter'; addons.register(STATIC_FILTER, (api) => { // FIXME: this ensures the filter is applied after the first render // to avoid a strange race condition in Webkit only. - const excludeTags = Object.entries(global.TAGS_OPTIONS ?? {}).reduce((acc, entry) => { - const [tag, option] = entry; - if ((option as any).excludeFromSidebar) { - acc[tag] = true; - } - return acc; - }, {} as Record); + const excludeTags = Object.entries(global.TAGS_OPTIONS ?? {}).reduce( + (acc, entry) => { + const [tag, option] = entry; + if ((option as any).excludeFromSidebar) { + acc[tag] = true; + } + return acc; + }, + {} as Record + ); api.experimental_setFilter(STATIC_FILTER, (item) => { const tags = item.tags || []; From bee8765d10900999b90eb72d45d4d15eaf378e54 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Sun, 14 Jan 2024 22:58:30 +1100 Subject: [PATCH 22/26] Drop unused vars --- code/addons/a11y/src/a11yRunner.ts | 2 +- code/addons/a11y/src/components/A11yContext.tsx | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/code/addons/a11y/src/a11yRunner.ts b/code/addons/a11y/src/a11yRunner.ts index f37c1b5e5f7c..ec33803558c3 100644 --- a/code/addons/a11y/src/a11yRunner.ts +++ b/code/addons/a11y/src/a11yRunner.ts @@ -3,7 +3,7 @@ import { addons } from '@storybook/preview-api'; import { EVENTS } from './constants'; import type { A11yParameters } from './params'; -const { document, window: globalWindow } = global; +const { document } = global; const channel = addons.getChannel(); // Holds axe core running state diff --git a/code/addons/a11y/src/components/A11yContext.tsx b/code/addons/a11y/src/components/A11yContext.tsx index 5c7582604213..01e9c68c32ba 100644 --- a/code/addons/a11y/src/components/A11yContext.tsx +++ b/code/addons/a11y/src/components/A11yContext.tsx @@ -5,7 +5,6 @@ import { useChannel, useAddonState, useStorybookApi } from '@storybook/manager-a import { STORY_CHANGED, STORY_RENDERED } from '@storybook/core-events'; import { HIGHLIGHT } from '@storybook/addon-highlight'; import { ADDON_ID, EVENTS } from '../constants'; -import type { A11yParameters } from '../params'; export interface Results { passes: Result[]; From 4821d82e74fc45139d1b32472de7e6ce8598b321 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Sun, 14 Jan 2024 23:04:26 +1100 Subject: [PATCH 23/26] Allow react complaining about errors caught by a boundary --- code/vitest-setup.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/code/vitest-setup.ts b/code/vitest-setup.ts index efb10035936b..4c1d82c78407 100644 --- a/code/vitest-setup.ts +++ b/code/vitest-setup.ts @@ -10,6 +10,12 @@ const ignoreList = [ (error: any) => error.message.includes('react-async-component-lifecycle-hooks') && error.stack.includes('addons/knobs/src/components/__tests__/Options.js'), + // React will log this error even if you catch an error with a boundary. I guess it's to + // help in development. See https://github.com/facebook/react/issues/15069 + (error: any) => + error.message.match( + /React will try to recreate this component tree from scratch using the error boundary you provided/ + ), ]; const throwMessage = (type: any, message: any) => { From a21ac329aa9521cd7ec26df2059b490f1090304d Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Sun, 14 Jan 2024 23:09:11 +1100 Subject: [PATCH 24/26] Add deprecation note + pass through `loadStory` --- MIGRATION.md | 16 +++++++++++----- .../src/modules/preview-web/Preview.tsx | 7 +++++++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 6f31fbe477e9..547996e8aba6 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -63,7 +63,7 @@ - [Canvas Doc block properties](#canvas-doc-block-properties) - [`Primary` Doc block properties](#primary-doc-block-properties) - [`createChannel` from `@storybook/postmessage` and `@storybook/channel-websocket`](#createchannel-from-storybookpostmessage-and--storybookchannel-websocket) - - [StoryStore methods deprecated](#storystore-methods-deprecated) + - [StoryStore and methods deprecated](#storystore-and-methods-deprecated) - [From version 7.5.0 to 7.6.0](#from-version-750-to-760) - [CommonJS with Vite is deprecated](#commonjs-with-vite-is-deprecated) - [Using implicit actions during rendering is deprecated](#using-implicit-actions-during-rendering-is-deprecated) @@ -978,11 +978,17 @@ The `name` prop is now removed in favor of the `of` property. [More info](#doc-b The `createChannel` APIs from both `@storybook/channel-websocket` and `@storybook/postmessage` are now removed. Please use `createBrowserChannel` instead, from the `@storybook/channels` package. Additionally, the `PostmsgTransport` type is now removed in favor of `PostMessageTransport`. -#### StoryStore methods deprecated -The following methods on the `StoryStore` are deprecated and will be removed in 9.0: - - `store.fromId()` - please use `store.loadStory()` instead. - - `store.raw()` - please use `store.extract()` instead. + +#### StoryStore and methods deprecated + +The StoryStore (`__STORYBOOK_STORY_STORE__` and `__STORYBOOK_PREVIEW__.storyStore`) are deprecated, and will no longer be accessible in Storybook 9.0. + +In particular, the following methods on the `StoryStore` are deprecated and will be removed in 9.0: + - `store.fromId()` - please use `preview.loadStory({ storyId })` instead. + - `store.raw()` - please use `preview.extract()` instead. + +Note that both these methods require initialization, so you should await `preview.ready()`. ## From version 7.5.0 to 7.6.0 diff --git a/code/lib/preview-api/src/modules/preview-web/Preview.tsx b/code/lib/preview-api/src/modules/preview-web/Preview.tsx index 30fbf2650205..7e368f7457f3 100644 --- a/code/lib/preview-api/src/modules/preview-web/Preview.tsx +++ b/code/lib/preview-api/src/modules/preview-web/Preview.tsx @@ -377,6 +377,13 @@ export class Preview { } // API + async loadStory({ storyId }: { storyId: StoryId }) { + if (!this.storyStoreValue) + throw new CalledPreviewMethodBeforeInitializationError({ methodName: 'loadStory' }); + + return this.storyStoreValue.loadStory({ storyId }); + } + async extract(options?: { includeDocsOnly: boolean }) { if (!this.storyStoreValue) throw new CalledPreviewMethodBeforeInitializationError({ methodName: 'extract' }); From d75b6352f0b8be9405ea39d91570822cdc80a9a1 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 15 Jan 2024 11:05:30 +1100 Subject: [PATCH 25/26] Fix typing problems in PreviewWeb test --- .../modules/preview-web/PreviewWeb.test.ts | 47 +++++++++++-------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts index ec3aa4c3af81..2ffcc6241deb 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts @@ -53,6 +53,7 @@ import { teardownrenderToCanvas, } from './PreviewWeb.mockdata'; import { WebView } from './WebView'; +import type { StoryStore } from '../store'; const { history, document } = global; @@ -163,7 +164,7 @@ describe('PreviewWeb', () => { const preview = await createAndRenderPreview(); - expect(preview.storyStore!.globals.get()).toEqual({ a: 'c' }); + expect((preview.storyStore as StoryStore)!.globals.get()).toEqual({ a: 'c' }); }); it('emits the SET_GLOBALS event', async () => { @@ -202,7 +203,7 @@ describe('PreviewWeb', () => { const preview = await createAndRenderPreview(); - expect(preview.storyStore?.args.get('component-one--a')).toEqual({ + expect((preview.storyStore as StoryStore)?.args.get('component-one--a')).toEqual({ foo: 'url', one: 1, }); @@ -224,7 +225,7 @@ describe('PreviewWeb', () => { }); await preview.ready(); - expect(preview.storyStore!.globals.get()).toEqual({ a: 'b' }); + expect((preview.storyStore as StoryStore)!.globals.get()).toEqual({ a: 'b' }); }); }); @@ -780,7 +781,7 @@ describe('PreviewWeb', () => { emitter.emit(UPDATE_GLOBALS, { globals: { foo: 'bar' } }); - expect(preview.storyStore!.globals.get()).toEqual({ a: 'b' }); + expect((preview.storyStore as StoryStore)!.globals.get()).toEqual({ a: 'b' }); }); it('passes globals in context to renderToCanvas', async () => { @@ -856,7 +857,11 @@ describe('PreviewWeb', () => { updatedArgs: { new: 'arg' }, }); - expect(preview.storyStore?.args.get('component-one--a')).toEqual({ + expect( + (preview.storyStore as StoryStore as StoryStore)?.args.get( + 'component-one--a' + ) + ).toEqual({ foo: 'a', new: 'arg', one: 1, @@ -1119,8 +1124,9 @@ describe('PreviewWeb', () => { await waitForRender(); mockChannel.emit.mockClear(); - const story = await preview.storyStore?.loadStory({ storyId: 'component-one--a' }); - // @ts-expect-error (tom will fix this) + const story = await (preview.storyStore as StoryStore)?.loadStory({ + storyId: 'component-one--a', + }); preview.renderStoryToElement(story, 'story-element' as any, callbacks, {}); await waitForRender(); @@ -1158,8 +1164,9 @@ describe('PreviewWeb', () => { await waitForRender(); mockChannel.emit.mockClear(); - const story = await preview.storyStore?.loadStory({ storyId: 'component-one--a' }); - // @ts-expect-error (tom will fix this) + const story = await (preview.storyStore as StoryStore)?.loadStory({ + storyId: 'component-one--a', + }); preview.renderStoryToElement(story, 'story-element' as any, callbacks, { forceInitialArgs: true, }); @@ -2091,7 +2098,7 @@ describe('PreviewWeb', () => { updatedArgs: { foo: 'updated' }, }); await waitForRender(); - expect(preview.storyStore?.args.get('component-one--a')).toEqual({ + expect((preview.storyStore as StoryStore)?.args.get('component-one--a')).toEqual({ foo: 'updated', one: 1, }); @@ -2103,7 +2110,7 @@ describe('PreviewWeb', () => { }); await waitForSetCurrentStory(); await waitForRender(); - expect(preview.storyStore?.args.get('component-one--a')).toEqual({ + expect((preview.storyStore as StoryStore)?.args.get('component-one--a')).toEqual({ foo: 'updated', one: 1, }); @@ -2115,7 +2122,7 @@ describe('PreviewWeb', () => { }); await waitForSetCurrentStory(); await waitForRender(); - expect(preview.storyStore?.args.get('component-one--a')).toEqual({ + expect((preview.storyStore as StoryStore)?.args.get('component-one--a')).toEqual({ foo: 'updated', one: 1, }); @@ -2717,7 +2724,7 @@ describe('PreviewWeb', () => { mockFetchResult = { status: 200, json: mockStoryIndex, text: () => 'error text' }; preview.onStoryIndexChanged(); await waitForRender(); - expect(preview.storyStore?.args.get('component-one--a')).toEqual({ + expect((preview.storyStore as StoryStore)?.args.get('component-one--a')).toEqual({ foo: 'url', one: 1, }); @@ -3123,7 +3130,7 @@ describe('PreviewWeb', () => { }); await waitForSetCurrentStory(); await waitForRender(); - expect(preview.storyStore?.args.get('component-one--a')).toEqual({ + expect((preview.storyStore as StoryStore)?.args.get('component-one--a')).toEqual({ foo: 'updated', one: 1, }); @@ -3142,7 +3149,7 @@ describe('PreviewWeb', () => { }); await waitForSetCurrentStory(); await waitForRender(); - expect(preview.storyStore?.args.get('component-one--a')).toEqual({ + expect((preview.storyStore as StoryStore)?.args.get('component-one--a')).toEqual({ foo: 'updated', bar: 'edited', one: 1, @@ -3313,7 +3320,7 @@ describe('PreviewWeb', () => { preview.onGetProjectAnnotationsChanged({ getProjectAnnotations }); await waitForRender(); - expect(preview.storyStore!.globals.get()).toEqual({ a: 'c' }); + expect((preview.storyStore as StoryStore)!.globals.get()).toEqual({ a: 'c' }); }); }); @@ -3363,7 +3370,7 @@ describe('PreviewWeb', () => { preview.onGetProjectAnnotationsChanged({ getProjectAnnotations: newGetProjectAnnotations }); await waitForRender(); - expect(preview.storyStore!.globals.get()).toEqual({ a: 'edited' }); + expect((preview.storyStore as StoryStore)!.globals.get()).toEqual({ a: 'edited' }); }); it('emits SET_GLOBALS with new values', async () => { @@ -3389,7 +3396,7 @@ describe('PreviewWeb', () => { preview.onGetProjectAnnotationsChanged({ getProjectAnnotations: newGetProjectAnnotations }); await waitForRender(); - expect(preview.storyStore?.args.get('component-one--a')).toEqual({ + expect((preview.storyStore as StoryStore)?.args.get('component-one--a')).toEqual({ foo: 'a', one: 1, global: 'added', @@ -3514,7 +3521,9 @@ describe('PreviewWeb', () => { componentOneExports.b.play.mockImplementationOnce(async () => gate); // @ts-expect-error (not strict) preview.renderStoryToElement( - await preview.storyStore?.loadStory({ storyId: 'component-one--b' }), + await (preview.storyStore as StoryStore)?.loadStory({ + storyId: 'component-one--b', + }), {} as any ); await waitForRenderPhase('playing'); From 4084f371c43095fa4bbf67686640b423570dcb44 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 15 Jan 2024 11:15:59 +1100 Subject: [PATCH 26/26] Clean up error message a tiny bit! --- code/lib/preview-api/src/modules/preview-web/WebView.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/lib/preview-api/src/modules/preview-web/WebView.ts b/code/lib/preview-api/src/modules/preview-web/WebView.ts index e3a7d360ac2f..1046df03abba 100644 --- a/code/lib/preview-api/src/modules/preview-web/WebView.ts +++ b/code/lib/preview-api/src/modules/preview-web/WebView.ts @@ -137,7 +137,7 @@ export class WebView implements View { const parts = message.split('\n'); if (parts.length > 1) { [header] = parts; - detail = parts.slice(1).join('\n'); + detail = parts.slice(1).join('\n').replace(/^\n/, ''); } document.getElementById('error-message')!.innerHTML = ansiConverter.toHtml(header);