diff --git a/code/e2e-tests/preview-web.spec.ts b/code/e2e-tests/preview-api.spec.ts similarity index 57% rename from code/e2e-tests/preview-web.spec.ts rename to code/e2e-tests/preview-api.spec.ts index b4a96c4b6001..662a638bc84c 100644 --- a/code/e2e-tests/preview-web.spec.ts +++ b/code/e2e-tests/preview-api.spec.ts @@ -5,7 +5,9 @@ import { SbPage } from './util'; const storybookUrl = process.env.STORYBOOK_URL || 'http://localhost:8001'; const templateName = process.env.STORYBOOK_TEMPLATE_NAME || ''; -test.describe('preview-web', () => { +const wait = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +test.describe('preview-api', () => { test.beforeEach(async ({ page }) => { await page.goto(storybookUrl); @@ -50,4 +52,40 @@ test.describe('preview-web', () => { await sbPage.previewRoot().getByRole('button').getByText('Submit').first().press('Alt+s'); await expect(sbPage.page.locator('.sidebar-container')).not.toBeVisible(); }); + + // if rerenders were interleaved the button would have rendered "Error: Interleaved loaders. Changed arg" + test('should only render once at a time when rapidly changing args', async ({ page }) => { + const sbPage = new SbPage(page); + await sbPage.navigateToStory('lib/preview-api/rendering', 'slow-loader'); + + const root = sbPage.previewRoot(); + + const labelControl = await sbPage.page.locator('#control-label'); + + await expect(root.getByText('Loaded. Click me')).toBeVisible(); + await expect(labelControl).toBeVisible(); + + await labelControl.fill(''); + await labelControl.type('Changed arg', { delay: 50 }); + await labelControl.blur(); + + await expect(root.getByText('Loaded. Changed arg')).toBeVisible(); + }); + + test('should reload plage when remounting while loading', async ({ page }) => { + const sbPage = new SbPage(page); + await sbPage.navigateToStory('lib/preview-api/rendering', 'slow-loader'); + + const root = sbPage.previewRoot(); + + await expect(root.getByText('Loaded. Click me')).toBeVisible(); + + await sbPage.page.getByRole('button', { name: 'Remount component' }).click(); + await wait(200); + await sbPage.page.getByRole('button', { name: 'Remount component' }).click(); + + // the loading spinner indicates the iframe is being fully reloaded + await expect(sbPage.previewIframe().locator('.sb-preparing-story > .sb-loader')).toBeVisible(); + await expect(root.getByText('Loaded. Click me')).toBeVisible(); + }); }); 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 aac800d6ac52..23329caf5d29 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 @@ -907,64 +907,74 @@ describe('PreviewWeb', () => { }); describe('while story is still rendering', () => { - it('runs loaders again', async () => { + it('runs loaders again after renderToCanvas is done', async () => { + // Arrange - set up a gate to control when the loaders run const [loadersRanGate, openLoadersRanGate] = createGate(); const [blockLoadersGate, openBlockLoadersGate] = createGate(); document.location.search = '?id=component-one--a'; - componentOneExports.default.loaders[0].mockImplementationOnce(async () => { + componentOneExports.default.loaders[0].mockImplementationOnce(async (input) => { openLoadersRanGate(); return blockLoadersGate; }); + // Act - render the first time await new PreviewWeb(importFn, getProjectAnnotations).ready(); await loadersRanGate; + // Assert - loader to be called the first time + expect(componentOneExports.default.loaders[0]).toHaveBeenCalledOnce(); expect(componentOneExports.default.loaders[0]).toHaveBeenCalledWith( expect.objectContaining({ args: { foo: 'a', one: 'mapped-1' }, }) ); - componentOneExports.default.loaders[0].mockClear(); + // Act - update the args (while loader is still running) emitter.emit(UPDATE_STORY_ARGS, { storyId: 'component-one--a', updatedArgs: { new: 'arg' }, }); - await waitForRender(); - expect(componentOneExports.default.loaders[0]).toHaveBeenCalledWith( - expect.objectContaining({ - args: { foo: 'a', new: 'arg', one: 'mapped-1' }, - }) - ); + // Arrange - open the gate to let the loader finish and wait for render + openBlockLoadersGate({ l: 8 }); + await waitForRender(); - // Story gets rendered with updated args - expect(projectAnnotations.renderToCanvas).toHaveBeenCalledTimes(1); + // Assert - renderToCanvas to be called the first time with initial args + expect(projectAnnotations.renderToCanvas).toHaveBeenCalledOnce(); expect(projectAnnotations.renderToCanvas).toHaveBeenCalledWith( expect.objectContaining({ - forceRemount: true, // Wasn't yet rendered so we need to force remount + forceRemount: true, storyContext: expect.objectContaining({ - loaded: { l: 7 }, // This is the value returned by the *second* loader call + loaded: { l: 8 }, // This is the value returned by the *first* loader call args: { foo: 'a', new: 'arg', one: 'mapped-1' }, }), }), 'story-element' ); + // Assert - loaders are not run again yet + expect(componentOneExports.default.loaders[0]).toHaveBeenCalledOnce(); - // Now let the first loader call resolve + // Arrange - wait for loading and rendering to finish a second time mockChannel.emit.mockClear(); - projectAnnotations.renderToCanvas.mockClear(); - openBlockLoadersGate({ l: 8 }); await waitForRender(); + // Assert - loader is called a second time with updated args + await vi.waitFor(() => { + expect(projectAnnotations.renderToCanvas).toHaveBeenCalledTimes(2); + expect(componentOneExports.default.loaders[0]).toHaveBeenCalledWith( + expect.objectContaining({ + args: { foo: 'a', new: 'arg', one: 'mapped-1' }, + }) + ); + }); - // Now the first call comes through, but picks up the new args - // Note this isn't a particularly realistic case (the second loader being quicker than the first) - expect(projectAnnotations.renderToCanvas).toHaveBeenCalledTimes(1); + // Assert - renderToCanvas is called a second time with updated args + expect(projectAnnotations.renderToCanvas).toHaveBeenCalledTimes(2); expect(projectAnnotations.renderToCanvas).toHaveBeenCalledWith( expect.objectContaining({ + forceRemount: false, storyContext: expect.objectContaining({ - loaded: { l: 8 }, + loaded: { l: 7 }, // This is the value returned by the *second* loader call args: { foo: 'a', new: 'arg', one: 'mapped-1' }, }), }), @@ -972,7 +982,7 @@ describe('PreviewWeb', () => { ); }); - it('renders a second time if renderToCanvas is running', async () => { + it('renders a second time after the already running renderToCanvas is done', async () => { const [gate, openGate] = createGate(); document.location.search = '?id=component-one--a'; @@ -986,11 +996,9 @@ describe('PreviewWeb', () => { updatedArgs: { new: 'arg' }, }); - // Now let the renderToCanvas call resolve + // Now let the first renderToCanvas call resolve openGate(); - await waitForRender(); - - expect(projectAnnotations.renderToCanvas).toHaveBeenCalledTimes(2); + expect(projectAnnotations.renderToCanvas).toHaveBeenCalledTimes(1); expect(projectAnnotations.renderToCanvas).toHaveBeenCalledWith( expect.objectContaining({ forceRemount: true, @@ -1001,39 +1009,14 @@ describe('PreviewWeb', () => { }), 'story-element' ); - expect(projectAnnotations.renderToCanvas).toHaveBeenCalledWith( - expect.objectContaining({ - forceRemount: false, - storyContext: expect.objectContaining({ - loaded: { l: 7 }, - args: { foo: 'a', new: 'arg', one: 'mapped-1' }, - }), - }), - 'story-element' - ); - }); - it('works if it is called directly from inside non async renderToCanvas', async () => { - document.location.search = '?id=component-one--a'; - projectAnnotations.renderToCanvas.mockImplementation(() => { - emitter.emit(UPDATE_STORY_ARGS, { - storyId: 'component-one--a', - updatedArgs: { new: 'arg' }, - }); - }); - await createAndRenderPreview(); + // Wait for the second render to finish + mockChannel.emit.mockClear(); + await waitForRender(); + await waitForRenderPhase('rendering'); + // Expect the second render to have the updated args expect(projectAnnotations.renderToCanvas).toHaveBeenCalledTimes(2); - expect(projectAnnotations.renderToCanvas).toHaveBeenCalledWith( - expect.objectContaining({ - forceRemount: true, - storyContext: expect.objectContaining({ - loaded: { l: 7 }, - args: { foo: 'a', one: 'mapped-1' }, - }), - }), - 'story-element' - ); expect(projectAnnotations.renderToCanvas).toHaveBeenCalledWith( expect.objectContaining({ forceRemount: false, diff --git a/code/lib/preview-api/src/modules/preview-web/render/StoryRender.test.ts b/code/lib/preview-api/src/modules/preview-web/render/StoryRender.test.ts index 7b21484558a6..d9c5bdcc0863 100644 --- a/code/lib/preview-api/src/modules/preview-web/render/StoryRender.test.ts +++ b/code/lib/preview-api/src/modules/preview-web/render/StoryRender.test.ts @@ -81,6 +81,58 @@ describe('StoryRender', () => { expect(story.playFunction).not.toHaveBeenCalled(); }); + it('only rerenders once when triggered multiple times while pending', async () => { + // Arrange - setup StoryRender and async gate blocking applyLoaders + const [loaderGate, openLoaderGate] = createGate(); + const story = { + id: 'id', + title: 'title', + name: 'name', + tags: [], + applyLoaders: vi.fn(() => loaderGate), + unboundStoryFn: vi.fn(), + playFunction: vi.fn(), + prepareContext: vi.fn(), + }; + const store = { getStoryContext: () => ({}), cleanupStory: vi.fn() }; + const renderToScreen = vi.fn(); + const render = new StoryRender( + new Channel({}), + store as any, + renderToScreen, + {} as any, + entry.id, + 'story', + { autoplay: true }, + story as any + ); + // Arrange - render (blocked by loaders) + render.renderToElement({} as any); + expect(story.applyLoaders).toHaveBeenCalledOnce(); + expect(render.phase).toBe('loading'); + + // Act - rerender 3x + render.rerender(); + render.rerender(); + render.rerender(); + + // Assert - still loading, not yet rendered + expect(story.applyLoaders).toHaveBeenCalledOnce(); + expect(render.phase).toBe('loading'); + expect(renderToScreen).not.toHaveBeenCalled(); + + // Act - finish loading + openLoaderGate(); + + // Assert - loaded and rendered twice, played once + await vi.waitFor(async () => { + console.log(render.phase); + expect(story.applyLoaders).toHaveBeenCalledTimes(2); + expect(renderToScreen).toHaveBeenCalledTimes(2); + expect(story.playFunction).toHaveBeenCalledOnce(); + }); + }); + describe('teardown', () => { it('throws PREPARE_ABORTED if torndown during prepare', async () => { const [importGate, openImportGate] = createGate(); diff --git a/code/lib/preview-api/src/modules/preview-web/render/StoryRender.ts b/code/lib/preview-api/src/modules/preview-web/render/StoryRender.ts index 406b3deb3b95..74e88bfecd2c 100644 --- a/code/lib/preview-api/src/modules/preview-web/render/StoryRender.ts +++ b/code/lib/preview-api/src/modules/preview-web/render/StoryRender.ts @@ -56,6 +56,8 @@ export class StoryRender implements Render {}; @@ -268,10 +270,27 @@ export class StoryRender implements Render { + loadedLabel = 'Loading...'; + await new Promise((resolve) => setTimeout(resolve, 1000)); + loadedLabel = loadedLabel === 'Loading...' ? 'Loaded.' : 'Error: Interleaved loaders.'; + return { label: loadedLabel }; + }, + ], + decorators: [ + (storyFn: any, context: any) => + storyFn({ + args: { + ...context.args, + label: `${context.loaded.label} ${context.args.label}`, + }, + }), + ], +};