From 4c80c21eb0ac64f8b116a1b720ad0a7c0376b0ba Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Fri, 10 Jun 2022 22:49:41 +1000 Subject: [PATCH 01/49] Allow a teardown function to be returned from `renderToDOM` --- app/react/src/client/preview/render.tsx | 2 ++ examples/react-ts/.storybook/main.ts | 2 +- examples/react-ts/src/button.tsx | 19 ++++++++++----- examples/react-ts/src/button2.stories.tsx | 10 ++++++++ examples/react-ts/src/button3.stories.tsx | 9 ++++++++ lib/preview-web/src/Preview.tsx | 3 ++- lib/preview-web/src/StoryRender.ts | 28 +++++++++++++++-------- lib/store/src/types.ts | 10 +++++++- 8 files changed, 65 insertions(+), 18 deletions(-) create mode 100644 examples/react-ts/src/button2.stories.tsx create mode 100644 examples/react-ts/src/button3.stories.tsx diff --git a/app/react/src/client/preview/render.tsx b/app/react/src/client/preview/render.tsx index 94db184a3b6b..e2f305158a58 100644 --- a/app/react/src/client/preview/render.tsx +++ b/app/react/src/client/preview/render.tsx @@ -147,4 +147,6 @@ export async function renderToDOM( } await renderElement(element, domElement); + + return () => unmountElement(domElement); } diff --git a/examples/react-ts/.storybook/main.ts b/examples/react-ts/.storybook/main.ts index 752f85c6ac60..33fb89ce230f 100644 --- a/examples/react-ts/.storybook/main.ts +++ b/examples/react-ts/.storybook/main.ts @@ -37,7 +37,7 @@ const config: StorybookConfig = { }, features: { postcss: false, - // modernInlineRender: true, + modernInlineRender: true, storyStoreV7: !global.navigator?.userAgent?.match?.('jsdom'), buildStoriesJson: true, babelModeV7: true, diff --git a/examples/react-ts/src/button.tsx b/examples/react-ts/src/button.tsx index 99dddad4e2f7..714b5db4e0f6 100644 --- a/examples/react-ts/src/button.tsx +++ b/examples/react-ts/src/button.tsx @@ -1,4 +1,4 @@ -import React, { ComponentType, ButtonHTMLAttributes } from 'react'; +import React, { ComponentType, ButtonHTMLAttributes, useEffect } from 'react'; export interface ButtonProps extends ButtonHTMLAttributes { /** @@ -12,8 +12,15 @@ export interface ButtonProps extends ButtonHTMLAttributes { icon?: ComponentType; } -export const Button = ({ label = 'Hello', icon: Icon, ...props }: ButtonProps) => ( - -); +export const Button = ({ label = 'Hello', icon: Icon, ...props }: ButtonProps) => { + useEffect(() => { + const fn = () => console.log(`click ${label}`); + global.window.document.querySelector('body')?.addEventListener('click', fn); + return () => global.window.document.querySelector('body')?.removeEventListener('click', fn); + }); + return ( + + ); +}; diff --git a/examples/react-ts/src/button2.stories.tsx b/examples/react-ts/src/button2.stories.tsx new file mode 100644 index 000000000000..cfaa6df924d4 --- /dev/null +++ b/examples/react-ts/src/button2.stories.tsx @@ -0,0 +1,10 @@ +import { Button } from './button'; + +export default { + component: Button, + title: 'button2', +}; + +export const one = { args: { label: 'one' } }; +export const two = { args: { label: 'two' } }; +export const three = { args: { label: 'three' } }; diff --git a/examples/react-ts/src/button3.stories.tsx b/examples/react-ts/src/button3.stories.tsx new file mode 100644 index 000000000000..941d8f436f3b --- /dev/null +++ b/examples/react-ts/src/button3.stories.tsx @@ -0,0 +1,9 @@ +import { Button } from './button'; + +export default { + component: Button, + title: 'button3', +}; + +export const four = { args: { label: 'four' } }; +export const five = { args: { label: 'five' } }; diff --git a/lib/preview-web/src/Preview.tsx b/lib/preview-web/src/Preview.tsx index 2eae90f861bf..a206506774f8 100644 --- a/lib/preview-web/src/Preview.tsx +++ b/lib/preview-web/src/Preview.tsx @@ -23,6 +23,7 @@ import { StoryIndex, PromiseLike, WebProjectAnnotations, + RenderToDOM, } from '@storybook/store'; import { StoryRender } from './StoryRender'; @@ -45,7 +46,7 @@ export class Preview { importFn?: ModuleImportFn; - renderToDOM: WebProjectAnnotations['renderToDOM']; + renderToDOM: RenderToDOM; storyRenders: StoryRender[] = []; diff --git a/lib/preview-web/src/StoryRender.ts b/lib/preview-web/src/StoryRender.ts index d0ede7cc1223..7bbb7f31f400 100644 --- a/lib/preview-web/src/StoryRender.ts +++ b/lib/preview-web/src/StoryRender.ts @@ -6,7 +6,13 @@ import { StoryContextForLoaders, StoryContext, } from '@storybook/csf'; -import { Story, RenderContext, StoryStore } from '@storybook/store'; +import { + Story, + RenderContext, + StoryStore, + RenderToDOM, + TeardownRenderToDOM, +} from '@storybook/store'; import { Channel } from '@storybook/addons'; import { STORY_RENDER_PHASE_CHANGED, STORY_RENDERED } from '@storybook/core-events'; @@ -62,13 +68,12 @@ export class StoryRender implements Render {}; + constructor( public channel: Channel, public store: StoryStore, - private renderToScreen: ( - renderContext: RenderContext, - canvasElement: HTMLElement - ) => void | Promise, + private renderToScreen: RenderToDOM, private callbacks: RenderContextCallbacks, public id: StoryId, public viewMode: ViewMode, @@ -190,9 +195,10 @@ export class StoryRender implements Render - this.renderToScreen(renderContext, this.canvasElement) - ); + await this.runPhase(abortSignal, 'rendering', async () => { + this.teardownRender = + (await this.renderToScreen(renderContext, this.canvasElement)) || (() => {}); + }); this.notYetRendered = false; if (abortSignal.aborted) return; @@ -241,7 +247,11 @@ export class StoryRender implements Render setTimeout(resolve, 0)); } diff --git a/lib/store/src/types.ts b/lib/store/src/types.ts index d52a0f2588e6..6271fab4cb5c 100644 --- a/lib/store/src/types.ts +++ b/lib/store/src/types.ts @@ -29,9 +29,17 @@ export type ModuleExports = Record; export type PromiseLike = Promise | SynchronousPromise; export type ModuleImportFn = (path: Path) => PromiseLike; +type MaybePromise = Promise | T; + +export type TeardownRenderToDOM = () => MaybePromise; +export type RenderToDOM = ( + context: RenderContext, + element: Element +) => MaybePromise; + export type WebProjectAnnotations = ProjectAnnotations & { - renderToDOM?: (context: RenderContext, element: Element) => Promise | void; + renderToDOM?: RenderToDOM; }; export type NormalizedProjectAnnotations = From 06e4c29adc194c49b81ffc93427e35c53cebffee Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Wed, 15 Jun 2022 19:10:36 +1000 Subject: [PATCH 02/49] Add integration tests for teardown function --- addons/docs/src/blocks/Story.tsx | 1 + lib/preview-web/src/PreviewWeb.mockdata.ts | 5 +- lib/preview-web/src/PreviewWeb.test.ts | 134 ++++++++++++++++----- 3 files changed, 111 insertions(+), 29 deletions(-) diff --git a/addons/docs/src/blocks/Story.tsx b/addons/docs/src/blocks/Story.tsx index e61fcb321512..877e3b89cfb1 100644 --- a/addons/docs/src/blocks/Story.tsx +++ b/addons/docs/src/blocks/Story.tsx @@ -120,6 +120,7 @@ function makeGate(): [Promise, () => void] { } const Story: FunctionComponent = (props) => { + console.log('Story'); const context = useContext(DocsContext); const channel = addons.getChannel(); const storyRef = useRef(); diff --git a/lib/preview-web/src/PreviewWeb.mockdata.ts b/lib/preview-web/src/PreviewWeb.mockdata.ts index ae2eadf8c707..2bca04fe2ac2 100644 --- a/lib/preview-web/src/PreviewWeb.mockdata.ts +++ b/lib/preview-web/src/PreviewWeb.mockdata.ts @@ -7,7 +7,7 @@ import { STORY_RENDER_PHASE_CHANGED, STORY_THREW_EXCEPTION, } from '@storybook/core-events'; -import { StoryIndex } from '@storybook/store'; +import { StoryIndex, TeardownRenderToDOM } from '@storybook/store'; import { RenderPhase } from './PreviewWeb'; export const componentOneExports = { @@ -32,12 +32,13 @@ export const importFn = jest.fn(async (path) => { return path === './src/ComponentOne.stories.js' ? componentOneExports : componentTwoExports; }); +export const teardownRenderToDOM: jest.Mock = jest.fn(); export const projectAnnotations = { globals: { a: 'b' }, globalTypes: {}, decorators: [jest.fn((s) => s())], render: jest.fn(), - renderToDOM: jest.fn(), + renderToDOM: jest.fn().mockReturnValue(teardownRenderToDOM), }; export const getProjectAnnotations = () => projectAnnotations; diff --git a/lib/preview-web/src/PreviewWeb.test.ts b/lib/preview-web/src/PreviewWeb.test.ts index 8d58970be041..4620b661ebe5 100644 --- a/lib/preview-web/src/PreviewWeb.test.ts +++ b/lib/preview-web/src/PreviewWeb.test.ts @@ -44,6 +44,7 @@ import { waitForRender, waitForQuiescence, waitForRenderPhase, + teardownRenderToDOM, } from './PreviewWeb.mockdata'; jest.mock('./WebView'); @@ -120,7 +121,8 @@ beforeEach(() => { componentOneExports.default.loaders[0].mockReset().mockImplementation(async () => ({ l: 7 })); componentOneExports.default.parameters.docs.container.mockClear(); componentOneExports.a.play.mockReset(); - projectAnnotations.renderToDOM.mockReset(); + teardownRenderToDOM.mockReset(); + projectAnnotations.renderToDOM.mockReset().mockReturnValue(teardownRenderToDOM); projectAnnotations.render.mockClear(); projectAnnotations.decorators[0].mockClear(); (ReactDOM.render as any as jest.Mock) @@ -470,7 +472,7 @@ describe('PreviewWeb', () => { it('renders exception if renderToDOM throws', async () => { const error = new Error('error'); - projectAnnotations.renderToDOM.mockImplementationOnce(() => { + projectAnnotations.renderToDOM.mockImplementation(() => { throw error; }); @@ -519,9 +521,7 @@ describe('PreviewWeb', () => { it('renders error if the story calls showError', async () => { const error = { title: 'title', description: 'description' }; - projectAnnotations.renderToDOM.mockImplementationOnce((context) => - context.showError(error) - ); + projectAnnotations.renderToDOM.mockImplementation((context) => context.showError(error)); document.location.search = '?id=component-one--a'; const preview = await createAndRenderPreview(); @@ -535,7 +535,7 @@ describe('PreviewWeb', () => { it('renders exception if the story calls showException', async () => { const error = new Error('error'); - projectAnnotations.renderToDOM.mockImplementationOnce((context) => + projectAnnotations.renderToDOM.mockImplementation((context) => context.showException(error) ); @@ -562,7 +562,7 @@ describe('PreviewWeb', () => { it('does not show error display if the render function throws IGNORED_EXCEPTION', async () => { document.location.search = '?id=component-one--a'; - projectAnnotations.renderToDOM.mockImplementationOnce(() => { + projectAnnotations.renderToDOM.mockImplementation(() => { throw IGNORED_EXCEPTION; }); @@ -837,7 +837,7 @@ describe('PreviewWeb', () => { const [gate, openGate] = createGate(); document.location.search = '?id=component-one--a'; - projectAnnotations.renderToDOM.mockImplementationOnce(async () => gate); + projectAnnotations.renderToDOM.mockImplementation(async () => gate); await new PreviewWeb().initialize({ importFn, getProjectAnnotations }); await waitForRenderPhase('rendering'); @@ -876,7 +876,7 @@ describe('PreviewWeb', () => { it('works if it is called directly from inside non async renderToDOM', async () => { document.location.search = '?id=component-one--a'; - projectAnnotations.renderToDOM.mockImplementationOnce(() => { + projectAnnotations.renderToDOM.mockImplementation(() => { emitter.emit(UPDATE_STORY_ARGS, { storyId: 'component-one--a', updatedArgs: { new: 'arg' }, @@ -912,7 +912,7 @@ describe('PreviewWeb', () => { componentOneExports.a.play.mockImplementationOnce(async () => gate); const renderToDOMCalled = new Promise((resolve) => { - projectAnnotations.renderToDOM.mockImplementationOnce(() => { + projectAnnotations.renderToDOM.mockImplementation(() => { resolve(null); }); }); @@ -1296,7 +1296,7 @@ describe('PreviewWeb', () => { const [gate, openGate] = createGate(); document.location.search = '?id=component-one--a'; - projectAnnotations.renderToDOM.mockImplementationOnce(async () => gate); + projectAnnotations.renderToDOM.mockImplementation(async () => gate); await new PreviewWeb().initialize({ importFn, getProjectAnnotations }); await waitForRenderPhase('rendering'); @@ -1463,6 +1463,20 @@ describe('PreviewWeb', () => { expect(projectAnnotations.renderToDOM).not.toHaveBeenCalled(); }); + it('does NOT call renderToDOMs teardown', async () => { + document.location.search = '?id=component-one--a'; + await createAndRenderPreview(); + + projectAnnotations.renderToDOM.mockClear(); + emitter.emit(SET_CURRENT_STORY, { + storyId: 'component-one--a', + viewMode: 'story', + }); + await waitForSetCurrentStory(); + + expect(teardownRenderToDOM).not.toHaveBeenCalled(); + }); + // For https://github.com/storybookjs/storybook/issues/17214 it('does NOT render a second time if preparing', async () => { document.location.search = '?id=component-one--a'; @@ -1510,6 +1524,20 @@ describe('PreviewWeb', () => { }); describe('when changing story in story viewMode', () => { + it('calls renderToDOMs teardown', async () => { + document.location.search = '?id=component-one--a'; + await createAndRenderPreview(); + + projectAnnotations.renderToDOM.mockClear(); + emitter.emit(SET_CURRENT_STORY, { + storyId: 'component-one--b', + viewMode: 'story', + }); + await waitForSetCurrentStory(); + + expect(teardownRenderToDOM).toHaveBeenCalled(); + }); + it('updates URL', async () => { document.location.search = '?id=component-one--a'; await createAndRenderPreview(); @@ -1645,7 +1673,7 @@ describe('PreviewWeb', () => { const preview = await createAndRenderPreview(); const error = new Error('error'); - projectAnnotations.renderToDOM.mockImplementationOnce(() => { + projectAnnotations.renderToDOM.mockImplementation(() => { throw error; }); @@ -1666,9 +1694,7 @@ describe('PreviewWeb', () => { const preview = await createAndRenderPreview(); const error = { title: 'title', description: 'description' }; - projectAnnotations.renderToDOM.mockImplementationOnce((context) => - context.showError(error) - ); + projectAnnotations.renderToDOM.mockImplementation((context) => context.showError(error)); mockChannel.emit.mockClear(); emitter.emit(SET_CURRENT_STORY, { @@ -1690,7 +1716,7 @@ describe('PreviewWeb', () => { const preview = await createAndRenderPreview(); const error = new Error('error'); - projectAnnotations.renderToDOM.mockImplementationOnce((context) => + projectAnnotations.renderToDOM.mockImplementation((context) => context.showException(error) ); @@ -1811,7 +1837,7 @@ describe('PreviewWeb', () => { const [gate, openGate] = createGate(); document.location.search = '?id=component-one--a'; - projectAnnotations.renderToDOM.mockImplementationOnce(async () => gate); + projectAnnotations.renderToDOM.mockImplementation(async () => gate); await new PreviewWeb().initialize({ importFn, getProjectAnnotations }); await waitForRenderPhase('rendering'); @@ -1937,6 +1963,19 @@ describe('PreviewWeb', () => { }); describe('when changing from story viewMode to docs', () => { + it('calls renderToDOMs teardown', async () => { + document.location.search = '?id=component-one--a'; + await createAndRenderPreview(); + + emitter.emit(SET_CURRENT_STORY, { + storyId: 'component-one--a', + viewMode: 'docs', + }); + await waitForSetCurrentStory(); + + expect(teardownRenderToDOM).toHaveBeenCalled(); + }); + it('updates URL', async () => { document.location.search = '?id=component-one--a'; await createAndRenderPreview(); @@ -2199,7 +2238,7 @@ describe('PreviewWeb', () => { const preview = await createAndRenderPreview(); const error = new Error('error'); - projectAnnotations.renderToDOM.mockImplementationOnce(() => { + projectAnnotations.renderToDOM.mockImplementation(() => { throw error; }); @@ -2217,9 +2256,7 @@ describe('PreviewWeb', () => { it('renders error if the story calls showError', async () => { const error = { title: 'title', description: 'description' }; - projectAnnotations.renderToDOM.mockImplementationOnce((context) => - context.showError(error) - ); + projectAnnotations.renderToDOM.mockImplementation((context) => context.showError(error)); document.location.search = '?id=component-one--a&viewMode=docs'; const preview = await createAndRenderPreview(); @@ -2241,7 +2278,7 @@ describe('PreviewWeb', () => { it('renders exception if the story calls showException', async () => { const error = new Error('error'); - projectAnnotations.renderToDOM.mockImplementationOnce((context) => + projectAnnotations.renderToDOM.mockImplementation((context) => context.showException(error) ); @@ -2347,6 +2384,17 @@ describe('PreviewWeb', () => { : componentTwoExports; }); + it('calls renderToDOMs teardown', async () => { + document.location.search = '?id=component-one--a'; + const preview = await createAndRenderPreview(); + mockChannel.emit.mockClear(); + + preview.onStoriesChanged({ importFn: newImportFn }); + await waitForRender(); + + expect(teardownRenderToDOM).toHaveBeenCalled(); + }); + it('does not emit STORY_UNCHANGED', async () => { document.location.search = '?id=component-one--a'; const preview = await createAndRenderPreview(); @@ -2491,7 +2539,7 @@ describe('PreviewWeb', () => { const preview = await createAndRenderPreview(); const error = new Error('error'); - projectAnnotations.renderToDOM.mockImplementationOnce(() => { + projectAnnotations.renderToDOM.mockImplementation(() => { throw error; }); @@ -2508,9 +2556,7 @@ describe('PreviewWeb', () => { const preview = await createAndRenderPreview(); const error = { title: 'title', description: 'description' }; - projectAnnotations.renderToDOM.mockImplementationOnce((context) => - context.showError(error) - ); + projectAnnotations.renderToDOM.mockImplementation((context) => context.showError(error)); mockChannel.emit.mockClear(); preview.onStoriesChanged({ importFn: newImportFn }); @@ -2528,7 +2574,7 @@ describe('PreviewWeb', () => { const preview = await createAndRenderPreview(); const error = new Error('error'); - projectAnnotations.renderToDOM.mockImplementationOnce((context) => + projectAnnotations.renderToDOM.mockImplementation((context) => context.showException(error) ); @@ -2629,6 +2675,17 @@ describe('PreviewWeb', () => { : newComponentTwoExports; }); + it('does NOT call renderToDOMs teardown', async () => { + document.location.search = '?id=component-one--a'; + const preview = await createAndRenderPreview(); + + mockChannel.emit.mockClear(); + preview.onStoriesChanged({ importFn: newImportFn }); + await waitForEvents([STORY_UNCHANGED]); + + expect(teardownRenderToDOM).not.toHaveBeenCalled(); + }); + it('emits STORY_UNCHANGED', async () => { document.location.search = '?id=component-one--a'; const preview = await createAndRenderPreview(); @@ -2754,6 +2811,17 @@ describe('PreviewWeb', () => { }, }; + it('calls renderToDOMs teardown', async () => { + document.location.search = '?id=component-one--a'; + const preview = await createAndRenderPreview(); + + mockChannel.emit.mockClear(); + preview.onStoriesChanged({ importFn: newImportFn, storyIndex: newStoryIndex }); + await waitForEvents([STORY_MISSING]); + + expect(teardownRenderToDOM).toHaveBeenCalled(); + }); + it('renders loading error', async () => { document.location.search = '?id=component-one--a'; const preview = await createAndRenderPreview(); @@ -2920,6 +2988,18 @@ describe('PreviewWeb', () => { }); }); + it('calls renderToDOMs teardown', async () => { + document.location.search = '?id=component-one--a'; + const preview = await createAndRenderPreview(); + + projectAnnotations.renderToDOM.mockClear(); + mockChannel.emit.mockClear(); + preview.onGetProjectAnnotationsChanged({ getProjectAnnotations: newGetProjectAnnotations }); + await waitForRender(); + + expect(teardownRenderToDOM).toHaveBeenCalled(); + }); + it('rerenders the current story with new global meta-generated context', async () => { document.location.search = '?id=component-one--a'; const preview = await createAndRenderPreview(); From 95d3f32bd184faa49cd8073ff5a4b67a0ccf8bfe Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 16 Jun 2022 12:24:30 +1000 Subject: [PATCH 03/49] Update addons/docs/src/blocks/Story.tsx Co-authored-by: Michael Shilman --- addons/docs/src/blocks/Story.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/addons/docs/src/blocks/Story.tsx b/addons/docs/src/blocks/Story.tsx index 877e3b89cfb1..e61fcb321512 100644 --- a/addons/docs/src/blocks/Story.tsx +++ b/addons/docs/src/blocks/Story.tsx @@ -120,7 +120,6 @@ function makeGate(): [Promise, () => void] { } const Story: FunctionComponent = (props) => { - console.log('Story'); const context = useContext(DocsContext); const channel = addons.getChannel(); const storyRef = useRef(); From 3a576721975b079ba02260e0db6065605668cb0d Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Fri, 17 Jun 2022 13:48:45 +1000 Subject: [PATCH 04/49] Update snapshots --- .../src/__snapshots__/storyshots.test.ts.snap | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/examples/react-ts/src/__snapshots__/storyshots.test.ts.snap b/examples/react-ts/src/__snapshots__/storyshots.test.ts.snap index 409f2a8ab57e..a3430706744a 100644 --- a/examples/react-ts/src/__snapshots__/storyshots.test.ts.snap +++ b/examples/react-ts/src/__snapshots__/storyshots.test.ts.snap @@ -4041,6 +4041,51 @@ exports[`Storyshots Demo/Examples / Emoji Button With Args 1`] = ` `; +exports[`Storyshots Demo/button2 One 1`] = ` + +`; + +exports[`Storyshots Demo/button2 Three 1`] = ` + +`; + +exports[`Storyshots Demo/button2 Two 1`] = ` + +`; + +exports[`Storyshots Demo/button3 Five 1`] = ` + +`; + +exports[`Storyshots Demo/button3 Four 1`] = ` + +`; + exports[`Storyshots Docs/ButtonMdx Basic 1`] = `