From 08c3f247615c41d0b547cbf8d64d1db36d8ff392 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Thu, 4 Apr 2024 12:44:12 +0200 Subject: [PATCH 1/5] reload window when tearing down during 'loading' phase --- .../preview-web/render/StoryRender.test.ts | 183 +++++++++++++++--- .../modules/preview-web/render/StoryRender.ts | 5 +- 2 files changed, 155 insertions(+), 33 deletions(-) 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 df4b37b840ae..e4942c11aab8 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 @@ -15,43 +15,17 @@ const entry = { importPath: './component.stories.ts', } as StoryIndexEntry; -const createGate = (): [Promise, (_?: any) => void] => { - let openGate = (_?: any) => {}; - const gate = new Promise((resolve) => { +const createGate = (): [Promise, () => void] => { + let openGate = () => {}; + const gate = new Promise((resolve) => { openGate = resolve; }); return [gate, openGate]; }; -describe('StoryRender', () => { - it('throws PREPARE_ABORTED if torndown during prepare', async () => { - const [importGate, openImportGate] = createGate(); - const mockStore = { - loadStory: vi.fn(async () => { - await importGate; - return {}; - }), - cleanupStory: vi.fn(), - }; - - const render = new StoryRender( - new Channel({}), - mockStore as unknown as StoryStore, - vi.fn(), - {} as any, - entry.id, - 'story' - ); - - const preparePromise = render.prepare(); - - render.teardown(); - - openImportGate(); - - await expect(preparePromise).rejects.toThrowError(PREPARE_ABORTED); - }); +window.location = { reload: vi.fn() } as any; +describe('StoryRender', () => { it('does run play function if passed autoplay=true', async () => { const story = { id: 'id', @@ -105,4 +79,151 @@ describe('StoryRender', () => { await render.renderToElement({} as any); expect(story.playFunction).not.toHaveBeenCalled(); }); + + describe('teardown', () => { + const teardownAndWaitForReload = (render: StoryRender) => { + // 1. immediately teardown the story + render.teardown(); + + return new Promise((resolve) => { + setInterval(() => { + try { + // 2. assert that the window is reloaded and move on + expect(window.location.reload).toHaveBeenCalledOnce(); + resolve(); + } catch { + // empty catch to ignore the assertion failing + } + }, 0); + }); + }; + + it('throws PREPARE_ABORTED if torndown during prepare', async () => { + const [importGate, openImportGate] = createGate(); + const mockStore = { + loadStory: vi.fn(async () => { + await importGate; + return {}; + }), + cleanupStory: vi.fn(), + }; + + const render = new StoryRender( + new Channel({}), + mockStore as unknown as StoryStore, + vi.fn(), + {} as any, + entry.id, + 'story' + ); + + const preparePromise = render.prepare(); + + render.teardown(); + + openImportGate(); + + await expect(preparePromise).rejects.toThrowError(PREPARE_ABORTED); + }); + + it('reloads the page when tearing down during loading', async () => { + const story = { + id: 'id', + title: 'title', + name: 'name', + tags: [], + applyLoaders: vi.fn(), + unboundStoryFn: vi.fn(), + playFunction: vi.fn(), + prepareContext: vi.fn(), + }; + const store = { getStoryContext: () => ({}), cleanupStory: vi.fn() }; + + const render = new StoryRender( + new Channel({}), + store as any, + vi.fn() as any, + {} as any, + entry.id, + 'story', + { autoplay: true }, + story as any + ); + + story.applyLoaders.mockImplementation(() => teardownAndWaitForReload(render)); + + await render.renderToElement({} as any); + + expect(story.applyLoaders).toHaveBeenCalledOnce(); + expect(store.cleanupStory).toHaveBeenCalledOnce(); + expect(window.location.reload).toHaveBeenCalledOnce(); + }); + + it('reloads the page when tearing down during rendering', async () => { + const story = { + id: 'id', + title: 'title', + name: 'name', + tags: [], + applyLoaders: vi.fn(), + 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, + {} as any, + entry.id, + 'story', + { autoplay: true }, + story as any + ); + + renderToScreen.mockImplementation(() => teardownAndWaitForReload(render)); + + await render.renderToElement({} as any); + + expect(renderToScreen).toHaveBeenCalledOnce(); + expect(store.cleanupStory).toHaveBeenCalledOnce(); + expect(window.location.reload).toHaveBeenCalledOnce(); + }); + + it('reloads the page when tearing down during playing', async () => { + const story = { + id: 'id', + title: 'title', + name: 'name', + tags: [], + applyLoaders: vi.fn(), + unboundStoryFn: vi.fn(), + playFunction: vi.fn(), + prepareContext: vi.fn(), + }; + const store = { getStoryContext: () => ({}), cleanupStory: vi.fn() }; + + const render = new StoryRender( + new Channel({}), + store as any, + vi.fn() as any, + {} as any, + entry.id, + 'story', + { autoplay: true }, + story as any + ); + + story.playFunction.mockImplementation(() => teardownAndWaitForReload(render)); + + await render.renderToElement({} as any); + + expect(story.playFunction).toHaveBeenCalledOnce(); + expect(store.cleanupStory).toHaveBeenCalledOnce(); + expect(window.location.reload).toHaveBeenCalledOnce(); + }); + }); }); 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 72b554d38cd2..f17759baf8f0 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 @@ -120,7 +120,8 @@ export class StoryRender implements Render implements Render Date: Fri, 5 Apr 2024 12:35:43 +0200 Subject: [PATCH 2/5] remove unnecessary test --- .../modules/preview-web/PreviewWeb.test.ts | 33 ------------------- 1 file changed, 33 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 e20af13752a6..0506167666b9 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 @@ -2141,39 +2141,6 @@ describe('PreviewWeb', () => { window.location = { ...originalLocation, reload: originalLocation.reload }; }); - it('stops initial story after loaders if running', async () => { - const [gate, openGate] = createGate(); - componentOneExports.default.loaders[0].mockImplementationOnce(async () => gate); - - document.location.search = '?id=component-one--a'; - await new PreviewWeb(importFn, getProjectAnnotations).ready(); - await waitForRenderPhase('loading'); - - emitter.emit(SET_CURRENT_STORY, { - storyId: 'component-one--b', - viewMode: 'story', - }); - await waitForSetCurrentStory(); - await waitForRender(); - - // Now let the loader resolve - openGate({ l: 8 }); - await waitForRender(); - - // Story gets rendered with updated args - expect(projectAnnotations.renderToCanvas).toHaveBeenCalledTimes(1); - expect(projectAnnotations.renderToCanvas).toHaveBeenCalledWith( - expect.objectContaining({ - forceRemount: true, - storyContext: expect.objectContaining({ - id: 'component-one--b', - loaded: { l: 7 }, - }), - }), - 'story-element' - ); - }); - it('aborts render for initial story', async () => { const [gate, openGate] = createGate(); From 6f6fe5fc843fb5a53eb44ce012491be45a909062 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Fri, 5 Apr 2024 13:22:35 +0200 Subject: [PATCH 3/5] improve readability of StoryRender.test flow --- .../preview-web/render/StoryRender.test.ts | 58 ++++++++++++++----- 1 file changed, 42 insertions(+), 16 deletions(-) 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 e4942c11aab8..5b000454397b 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 @@ -22,6 +22,7 @@ const createGate = (): [Promise, () => void] => { }); return [gate, openGate]; }; +const tick = () => new Promise((resolve) => setTimeout(resolve, 0)); window.location = { reload: vi.fn() } as any; @@ -127,18 +128,19 @@ describe('StoryRender', () => { }); it('reloads the page when tearing down during loading', 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(), + applyLoaders: vi.fn(() => loaderGate), unboundStoryFn: vi.fn(), playFunction: vi.fn(), prepareContext: vi.fn(), }; const store = { getStoryContext: () => ({}), cleanupStory: vi.fn() }; - const render = new StoryRender( new Channel({}), store as any, @@ -150,16 +152,24 @@ describe('StoryRender', () => { story as any ); - story.applyLoaders.mockImplementation(() => teardownAndWaitForReload(render)); - - await render.renderToElement({} as any); - + // Act - render, blocked by loaders, teardown + // ... Assert - window is reloaded + const renderPromise = render.renderToElement({} as any); expect(story.applyLoaders).toHaveBeenCalledOnce(); + await teardownAndWaitForReload(render); + + // Assert - everything is actually cleaned up, just in case expect(store.cleanupStory).toHaveBeenCalledOnce(); expect(window.location.reload).toHaveBeenCalledOnce(); + + // clear dangling promise + openLoaderGate(); + await renderPromise; }); it('reloads the page when tearing down during rendering', async () => { + // Arrange - setup StoryRender and async gate blocking renderToScreen + const [renderGate, openRenderGate] = createGate(); const story = { id: 'id', title: 'title', @@ -171,7 +181,7 @@ describe('StoryRender', () => { prepareContext: vi.fn(), }; const store = { getStoryContext: () => ({}), cleanupStory: vi.fn() }; - const renderToScreen = vi.fn(); + const renderToScreen = vi.fn(() => renderGate); const render = new StoryRender( new Channel({}), @@ -184,16 +194,25 @@ describe('StoryRender', () => { story as any ); - renderToScreen.mockImplementation(() => teardownAndWaitForReload(render)); - - await render.renderToElement({} as any); - + // Act - render, blocked by renderToScreen, teardown + // ... Assert - window is reloaded + const renderPromise = render.renderToElement({} as any); + await tick(); // go from 'loading' to 'rendering' phase expect(renderToScreen).toHaveBeenCalledOnce(); + await teardownAndWaitForReload(render); + + // Assert - everything is actually cleaned up, just in case expect(store.cleanupStory).toHaveBeenCalledOnce(); expect(window.location.reload).toHaveBeenCalledOnce(); + + // clear dangling promise + openRenderGate(); + await renderPromise; }); it('reloads the page when tearing down during playing', async () => { + // Arrange - setup StoryRender and async gate blocking playing + const [playGate, openPlayGate] = createGate(); const story = { id: 'id', title: 'title', @@ -201,7 +220,7 @@ describe('StoryRender', () => { tags: [], applyLoaders: vi.fn(), unboundStoryFn: vi.fn(), - playFunction: vi.fn(), + playFunction: vi.fn(() => playGate), prepareContext: vi.fn(), }; const store = { getStoryContext: () => ({}), cleanupStory: vi.fn() }; @@ -217,13 +236,20 @@ describe('StoryRender', () => { story as any ); - story.playFunction.mockImplementation(() => teardownAndWaitForReload(render)); - - await render.renderToElement({} as any); - + // Act - render, blocked by renderToScreen, teardown + // ... Assert - window is reloaded + const renderPromise = render.renderToElement({} as any); + await tick(); // go from 'loading' to 'playing' phase expect(story.playFunction).toHaveBeenCalledOnce(); + await teardownAndWaitForReload(render); + + // Assert - everything is actually cleaned up, just in case expect(store.cleanupStory).toHaveBeenCalledOnce(); expect(window.location.reload).toHaveBeenCalledOnce(); + + // clear dangling promise + openPlayGate(); + await renderPromise; }); }); }); From 1c1003411d529759f2655adbd1a050662cc3a0c5 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Fri, 5 Apr 2024 13:26:37 +0200 Subject: [PATCH 4/5] assert render phase --- .../preview-web/render/StoryRender.test.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) 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 5b000454397b..7974840c58c2 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 @@ -152,10 +152,11 @@ describe('StoryRender', () => { story as any ); - // Act - render, blocked by loaders, teardown - // ... Assert - window is reloaded + // Act - render, blocked by loaders const renderPromise = render.renderToElement({} as any); expect(story.applyLoaders).toHaveBeenCalledOnce(); + expect(render.phase).toBe('loading'); + // Act & Assert - teardown, assert window is reloaded await teardownAndWaitForReload(render); // Assert - everything is actually cleaned up, just in case @@ -194,11 +195,12 @@ describe('StoryRender', () => { story as any ); - // Act - render, blocked by renderToScreen, teardown - // ... Assert - window is reloaded + // Act - render, blocked by renderToScreen const renderPromise = render.renderToElement({} as any); await tick(); // go from 'loading' to 'rendering' phase expect(renderToScreen).toHaveBeenCalledOnce(); + expect(render.phase).toBe('rendering'); + // Act & Assert - teardown, assert window is reloaded await teardownAndWaitForReload(render); // Assert - everything is actually cleaned up, just in case @@ -236,11 +238,12 @@ describe('StoryRender', () => { story as any ); - // Act - render, blocked by renderToScreen, teardown - // ... Assert - window is reloaded + // Act - render, blocked by playFn const renderPromise = render.renderToElement({} as any); await tick(); // go from 'loading' to 'playing' phase expect(story.playFunction).toHaveBeenCalledOnce(); + expect(render.phase).toBe('playing'); + // Act & Assert - teardown, assert window is reloaded await teardownAndWaitForReload(render); // Assert - everything is actually cleaned up, just in case From cf6788efefabef41093c176cb752a1c0f9f76527 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Fri, 5 Apr 2024 19:47:59 +0200 Subject: [PATCH 5/5] simplify StoryRender tests --- .../preview-web/render/StoryRender.test.ts | 80 +++++++------------ 1 file changed, 27 insertions(+), 53 deletions(-) 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 7974840c58c2..57822b9de6dd 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 @@ -82,23 +82,6 @@ describe('StoryRender', () => { }); describe('teardown', () => { - const teardownAndWaitForReload = (render: StoryRender) => { - // 1. immediately teardown the story - render.teardown(); - - return new Promise((resolve) => { - setInterval(() => { - try { - // 2. assert that the window is reloaded and move on - expect(window.location.reload).toHaveBeenCalledOnce(); - resolve(); - } catch { - // empty catch to ignore the assertion failing - } - }, 0); - }); - }; - it('throws PREPARE_ABORTED if torndown during prepare', async () => { const [importGate, openImportGate] = createGate(); const mockStore = { @@ -129,7 +112,7 @@ describe('StoryRender', () => { it('reloads the page when tearing down during loading', async () => { // Arrange - setup StoryRender and async gate blocking applyLoaders - const [loaderGate, openLoaderGate] = createGate(); + const [loaderGate] = createGate(); const story = { id: 'id', title: 'title', @@ -152,25 +135,22 @@ describe('StoryRender', () => { story as any ); - // Act - render, blocked by loaders - const renderPromise = render.renderToElement({} as any); + // Act - render (blocked by loaders), teardown + render.renderToElement({} as any); expect(story.applyLoaders).toHaveBeenCalledOnce(); expect(render.phase).toBe('loading'); - // Act & Assert - teardown, assert window is reloaded - await teardownAndWaitForReload(render); - - // Assert - everything is actually cleaned up, just in case - expect(store.cleanupStory).toHaveBeenCalledOnce(); - expect(window.location.reload).toHaveBeenCalledOnce(); + render.teardown(); - // clear dangling promise - openLoaderGate(); - await renderPromise; + // Assert - window is reloaded + await vi.waitFor(() => { + expect(window.location.reload).toHaveBeenCalledOnce(); + expect(store.cleanupStory).toHaveBeenCalledOnce(); + }); }); it('reloads the page when tearing down during rendering', async () => { // Arrange - setup StoryRender and async gate blocking renderToScreen - const [renderGate, openRenderGate] = createGate(); + const [renderGate] = createGate(); const story = { id: 'id', title: 'title', @@ -195,26 +175,23 @@ describe('StoryRender', () => { story as any ); - // Act - render, blocked by renderToScreen - const renderPromise = render.renderToElement({} as any); + // Act - render (blocked by renderToScreen), teardown + render.renderToElement({} as any); await tick(); // go from 'loading' to 'rendering' phase expect(renderToScreen).toHaveBeenCalledOnce(); expect(render.phase).toBe('rendering'); - // Act & Assert - teardown, assert window is reloaded - await teardownAndWaitForReload(render); - - // Assert - everything is actually cleaned up, just in case - expect(store.cleanupStory).toHaveBeenCalledOnce(); - expect(window.location.reload).toHaveBeenCalledOnce(); + render.teardown(); - // clear dangling promise - openRenderGate(); - await renderPromise; + // Assert - window is reloaded + await vi.waitFor(() => { + expect(window.location.reload).toHaveBeenCalledOnce(); + expect(store.cleanupStory).toHaveBeenCalledOnce(); + }); }); it('reloads the page when tearing down during playing', async () => { // Arrange - setup StoryRender and async gate blocking playing - const [playGate, openPlayGate] = createGate(); + const [playGate] = createGate(); const story = { id: 'id', title: 'title', @@ -238,21 +215,18 @@ describe('StoryRender', () => { story as any ); - // Act - render, blocked by playFn - const renderPromise = render.renderToElement({} as any); + // Act - render (blocked by playFn), teardown + render.renderToElement({} as any); await tick(); // go from 'loading' to 'playing' phase expect(story.playFunction).toHaveBeenCalledOnce(); expect(render.phase).toBe('playing'); - // Act & Assert - teardown, assert window is reloaded - await teardownAndWaitForReload(render); - - // Assert - everything is actually cleaned up, just in case - expect(store.cleanupStory).toHaveBeenCalledOnce(); - expect(window.location.reload).toHaveBeenCalledOnce(); + render.teardown(); - // clear dangling promise - openPlayGate(); - await renderPromise; + // Assert - window is reloaded + await vi.waitFor(() => { + expect(window.location.reload).toHaveBeenCalledOnce(); + expect(store.cleanupStory).toHaveBeenCalledOnce(); + }); }); }); });