From 08abb0274dce9304d07ee6e3563330794d5f49f2 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 8 Sep 2022 16:30:10 +1000 Subject: [PATCH 1/4] Add a new `hidePlayFunctionExceptions` parameter If true, it doesn't exhibit play function exceptions in the UI. Set it to true from the interactions addon. --- .../addons/interactions/src/preset/preview.ts | 4 ++ .../preview-web/src/PreviewWeb.mockdata.ts | 2 +- code/lib/preview-web/src/PreviewWeb.test.ts | 65 +++++++++++++------ .../lib/preview-web/src/render/StoryRender.ts | 1 + 4 files changed, 51 insertions(+), 21 deletions(-) diff --git a/code/addons/interactions/src/preset/preview.ts b/code/addons/interactions/src/preset/preview.ts index 41eadcf626f5..499fd3415287 100644 --- a/code/addons/interactions/src/preset/preview.ts +++ b/code/addons/interactions/src/preset/preview.ts @@ -59,3 +59,7 @@ export const { step: runStep } = instrument( { step: (label: StepLabel, play: PlayFunction, context: PlayFunctionContext) => play(context) }, { intercept: true } ); + +export const parameters = { + hidePlayFunctionExceptions: true, +}; diff --git a/code/lib/preview-web/src/PreviewWeb.mockdata.ts b/code/lib/preview-web/src/PreviewWeb.mockdata.ts index aae1136c168c..99d92dbf140b 100644 --- a/code/lib/preview-web/src/PreviewWeb.mockdata.ts +++ b/code/lib/preview-web/src/PreviewWeb.mockdata.ts @@ -64,7 +64,7 @@ export const projectAnnotations = { renderToDOM: jest.fn().mockReturnValue(teardownRenderToDOM), parameters: { docs: { renderer: () => docsRenderer } }, }; -export const getProjectAnnotations = () => projectAnnotations; +export const getProjectAnnotations = jest.fn(() => projectAnnotations as any); export const storyIndex: StoryIndex = { v: 4, diff --git a/code/lib/preview-web/src/PreviewWeb.test.ts b/code/lib/preview-web/src/PreviewWeb.test.ts index 2efa7ab9a64d..7d5a2c1429fa 100644 --- a/code/lib/preview-web/src/PreviewWeb.test.ts +++ b/code/lib/preview-web/src/PreviewWeb.test.ts @@ -511,16 +511,13 @@ describe('PreviewWeb', () => { it('renders helpful message if renderToDOM is undefined', async () => { document.location.search = '?id=component-one--a'; + + getProjectAnnotations.mockReturnValueOnce({ + ...projectAnnotations, + renderToDOM: undefined, + }); const preview = new PreviewWeb(); - await expect( - preview.initialize({ - importFn, - getProjectAnnotations: () => ({ - ...getProjectAnnotations, - renderToDOM: undefined, - }), - }) - ).rejects.toThrow(); + await expect(preview.initialize({ importFn, getProjectAnnotations })).rejects.toThrow(); expect(preview.view.showErrorDisplay).toHaveBeenCalled(); expect((preview.view.showErrorDisplay as jest.Mock).mock.calls[0][0]) @@ -533,20 +530,48 @@ describe('PreviewWeb', () => { `); }); - it('emits but does not render exception if the play function throws', async () => { - const error = new Error('error'); - componentOneExports.a.play.mockImplementationOnce(() => { - throw error; + describe('when `hidePlayFunctionExceptions` is set', () => { + it('emits but does not render exception if the play function throws', async () => { + const error = new Error('error'); + componentOneExports.a.play.mockImplementationOnce(() => { + throw error; + }); + + getProjectAnnotations.mockReturnValueOnce({ + ...projectAnnotations, + parameters: { + ...projectAnnotations.parameters, + hidePlayFunctionExceptions: true, + }, + }); + + document.location.search = '?id=component-one--a'; + const preview = await createAndRenderPreview(); + + expect(mockChannel.emit).toHaveBeenCalledWith( + PLAY_FUNCTION_THREW_EXCEPTION, + serializeError(error) + ); + expect(preview.view.showErrorDisplay).not.toHaveBeenCalled(); }); + }); - document.location.search = '?id=component-one--a'; - const preview = await createAndRenderPreview(); + describe('when `hidePlayFunctionExceptions` is unset', () => { + it('emits AND renders exception if the play function throws', async () => { + const error = new Error('error'); + componentOneExports.a.play.mockImplementationOnce(() => { + throw error; + }); - expect(mockChannel.emit).toHaveBeenCalledWith( - PLAY_FUNCTION_THREW_EXCEPTION, - serializeError(error) - ); - expect(preview.view.showErrorDisplay).not.toHaveBeenCalled(); + document.location.search = '?id=component-one--a'; + const preview = await createAndRenderPreview(); + + expect(mockChannel.emit).toHaveBeenCalledWith( + PLAY_FUNCTION_THREW_EXCEPTION, + serializeError(error) + ); + expect(preview.view.showErrorDisplay).toHaveBeenCalled(); + }); }); it('renders exception if the story calls showException', async () => { diff --git a/code/lib/preview-web/src/render/StoryRender.ts b/code/lib/preview-web/src/render/StoryRender.ts index 0074e28f9c01..4f7adddfe91b 100644 --- a/code/lib/preview-web/src/render/StoryRender.ts +++ b/code/lib/preview-web/src/render/StoryRender.ts @@ -242,6 +242,7 @@ export class StoryRender implements Render { this.channel.emit(PLAY_FUNCTION_THREW_EXCEPTION, serializeError(error)); }); + if (!this.story.parameters.hidePlayFunctionExceptions) throw error; } this.disableKeyListeners = false; if (abortSignal.aborted) return; From c6d0b40f1e40ae9bca701f03d38024b661f3c8f1 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 8 Sep 2022 17:08:17 +1000 Subject: [PATCH 2/4] Allow updating project annotations before selection --- code/lib/preview-web/src/PreviewWeb.test.ts | 10 ++++++++++ code/lib/preview-web/src/PreviewWeb.tsx | 4 +++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/code/lib/preview-web/src/PreviewWeb.test.ts b/code/lib/preview-web/src/PreviewWeb.test.ts index 7d5a2c1429fa..4cb7a2011b49 100644 --- a/code/lib/preview-web/src/PreviewWeb.test.ts +++ b/code/lib/preview-web/src/PreviewWeb.test.ts @@ -3144,6 +3144,16 @@ describe('PreviewWeb', () => { }); }); + describe('with no selection', () => { + // eslint-disable-next-line jest/expect-expect + it('does not error', async () => { + const preview = await createAndRenderPreview(); + await preview.onGetProjectAnnotationsChanged({ + getProjectAnnotations: newGetProjectAnnotations, + }); + }); + }); + it('shows an error the new value throws', async () => { document.location.search = '?id=component-one--a'; const preview = await createAndRenderPreview(); diff --git a/code/lib/preview-web/src/PreviewWeb.tsx b/code/lib/preview-web/src/PreviewWeb.tsx index 49f07856b1b4..4c5d96eca8e9 100644 --- a/code/lib/preview-web/src/PreviewWeb.tsx +++ b/code/lib/preview-web/src/PreviewWeb.tsx @@ -181,7 +181,9 @@ export class PreviewWeb extends Preview Date: Mon, 12 Sep 2022 20:42:46 +1000 Subject: [PATCH 3/4] Add event checks --- code/lib/preview-web/src/PreviewWeb.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/code/lib/preview-web/src/PreviewWeb.test.ts b/code/lib/preview-web/src/PreviewWeb.test.ts index 4cb7a2011b49..12c9fc7c36c7 100644 --- a/code/lib/preview-web/src/PreviewWeb.test.ts +++ b/code/lib/preview-web/src/PreviewWeb.test.ts @@ -553,6 +553,10 @@ describe('PreviewWeb', () => { serializeError(error) ); expect(preview.view.showErrorDisplay).not.toHaveBeenCalled(); + expect(mockChannel.emit).not.toHaveBeenCalledWith( + STORY_THREW_EXCEPTION, + serializeError(error) + ); }); }); @@ -571,6 +575,10 @@ describe('PreviewWeb', () => { serializeError(error) ); expect(preview.view.showErrorDisplay).toHaveBeenCalled(); + expect(mockChannel.emit).toHaveBeenCalledWith( + STORY_THREW_EXCEPTION, + serializeError(error) + ); }); }); From d74ce3e54786655c973d062f3426b9ebc79f8620 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 12 Sep 2022 20:43:50 +1000 Subject: [PATCH 4/4] `hide` -> `throwPlayFunctionExceptions` --- code/addons/interactions/src/preset/preview.ts | 2 +- code/lib/preview-web/src/PreviewWeb.test.ts | 6 +++--- code/lib/preview-web/src/render/StoryRender.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/code/addons/interactions/src/preset/preview.ts b/code/addons/interactions/src/preset/preview.ts index 499fd3415287..6995d3037159 100644 --- a/code/addons/interactions/src/preset/preview.ts +++ b/code/addons/interactions/src/preset/preview.ts @@ -61,5 +61,5 @@ export const { step: runStep } = instrument( ); export const parameters = { - hidePlayFunctionExceptions: true, + throwPlayFunctionExceptions: false, }; diff --git a/code/lib/preview-web/src/PreviewWeb.test.ts b/code/lib/preview-web/src/PreviewWeb.test.ts index 12c9fc7c36c7..75fb1b8de23f 100644 --- a/code/lib/preview-web/src/PreviewWeb.test.ts +++ b/code/lib/preview-web/src/PreviewWeb.test.ts @@ -530,7 +530,7 @@ describe('PreviewWeb', () => { `); }); - describe('when `hidePlayFunctionExceptions` is set', () => { + describe('when `throwPlayFunctionExceptions` is set', () => { it('emits but does not render exception if the play function throws', async () => { const error = new Error('error'); componentOneExports.a.play.mockImplementationOnce(() => { @@ -541,7 +541,7 @@ describe('PreviewWeb', () => { ...projectAnnotations, parameters: { ...projectAnnotations.parameters, - hidePlayFunctionExceptions: true, + throwPlayFunctionExceptions: false, }, }); @@ -560,7 +560,7 @@ describe('PreviewWeb', () => { }); }); - describe('when `hidePlayFunctionExceptions` is unset', () => { + describe('when `throwPlayFunctionExceptions` is unset', () => { it('emits AND renders exception if the play function throws', async () => { const error = new Error('error'); componentOneExports.a.play.mockImplementationOnce(() => { diff --git a/code/lib/preview-web/src/render/StoryRender.ts b/code/lib/preview-web/src/render/StoryRender.ts index 4f7adddfe91b..ad1c6d8eb5c5 100644 --- a/code/lib/preview-web/src/render/StoryRender.ts +++ b/code/lib/preview-web/src/render/StoryRender.ts @@ -242,7 +242,7 @@ export class StoryRender implements Render { this.channel.emit(PLAY_FUNCTION_THREW_EXCEPTION, serializeError(error)); }); - if (!this.story.parameters.hidePlayFunctionExceptions) throw error; + if (this.story.parameters.throwPlayFunctionExceptions !== false) throw error; } this.disableKeyListeners = false; if (abortSignal.aborted) return;