From 466a547e6a461f374dfc9bd528a09d032b0ee6bc Mon Sep 17 00:00:00 2001 From: Mauricio Rivera Date: Mon, 23 Jan 2023 12:10:22 -0500 Subject: [PATCH 1/8] Implemented prepared context. --- .../preview-web/render/StoryRender.test.ts | 2 + .../modules/preview-web/render/StoryRender.ts | 17 +++-- .../modules/store/csf/prepareStory.test.ts | 64 +++++++++++++++++-- .../src/modules/store/csf/prepareStory.ts | 41 +++++++----- code/lib/types/src/modules/story.ts | 1 + 5 files changed, 97 insertions(+), 28 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 d40adca995f0..b95cbe5d3383 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 @@ -59,6 +59,7 @@ describe('StoryRender', () => { applyLoaders: jest.fn(), unboundStoryFn: jest.fn(), playFunction: jest.fn(), + prepareContext: jest.fn(), }; const render = new StoryRender( @@ -85,6 +86,7 @@ describe('StoryRender', () => { applyLoaders: jest.fn(), unboundStoryFn: jest.fn(), playFunction: jest.fn(), + prepareContext: jest.fn(), }; const render = new StoryRender( 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 89c7422872cd..362aec1fb7e4 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 @@ -154,8 +154,17 @@ export class StoryRender implements Render implements Render = { + const renderStoryContext: StoryContext = prepareContext({ ...loadedContext!, // By this stage, it is possible that new args/globals have been received for this story // and we need to ensure we render it with the new values @@ -189,7 +198,7 @@ export class StoryRender implements Render = { componentId, title, diff --git a/code/lib/preview-api/src/modules/store/csf/prepareStory.test.ts b/code/lib/preview-api/src/modules/store/csf/prepareStory.test.ts index a84c5796b8ce..f1c7e2fb411b 100644 --- a/code/lib/preview-api/src/modules/store/csf/prepareStory.test.ts +++ b/code/lib/preview-api/src/modules/store/csf/prepareStory.test.ts @@ -468,8 +468,8 @@ describe('prepareStory', () => { { render: renderMock } ); - const context = { args: story.initialArgs, ...story }; - story.undecoratedStoryFn(context as any); + const context = story.prepareContext({ args: story.initialArgs, ...story } as any); + story.undecoratedStoryFn(context); expect(renderMock).toHaveBeenCalledWith( { one: 'mapped', two: 2, three: 3 }, expect.objectContaining({ args: { one: 'mapped', two: 2, three: 3 } }) @@ -529,6 +529,50 @@ describe('prepareStory', () => { hooks.clean(); }); + + it('prepared context is applied to decorators', () => { + const renderMock = jest.fn(); + let ctx1; + let ctx2; + let ctx3; + + const globalDecorator = jest.fn((fn, ctx) => { + ctx1 = ctx; + return fn(); + }); + const componentDecorator = jest.fn((fn, ctx) => { + ctx2 = ctx; + return fn(); + }); + const storyDecorator = jest.fn((fn, ctx) => { + ctx3 = ctx; + return fn(); + }); + const story = prepareStory( + { + id, + name, + argTypes: { + one: { name: 'one', type: { name: 'string' }, mapping: { 1: 'mapped-1' } }, + }, + args: { one: 1 }, + decorators: [storyDecorator], + moduleExport, + }, + { id, title, decorators: [componentDecorator] }, + { render: renderMock, decorators: [globalDecorator] } + ); + + const hooks = new HooksContext(); + const context = story.prepareContext({ args: story.initialArgs, hooks, ...story } as any); + story.unboundStoryFn(context); + + expect(ctx1).toMatchObject({ args: { one: 'mapped-1' } }); + expect(ctx2).toMatchObject({ args: { one: 'mapped-1' } }); + expect(ctx3).toMatchObject({ args: { one: 'mapped-1' } }); + + hooks.clean(); + }); }); describe('with `FEATURES.argTypeTargetsV7`', () => { @@ -549,11 +593,12 @@ describe('prepareStory', () => { { render: renderMock } ); - firstStory.unboundStoryFn({ + const context = firstStory.prepareContext({ args: firstStory.initialArgs, hooks: new HooksContext(), ...firstStory, } as any); + firstStory.unboundStoryFn(context); expect(renderMock).toHaveBeenCalledWith( { a: 1 }, expect.objectContaining({ args: { a: 1 }, allArgs: { a: 1, b: 2 } }) @@ -574,11 +619,12 @@ describe('prepareStory', () => { { render: renderMock } ); - firstStory.unboundStoryFn({ + const context = firstStory.prepareContext({ args: firstStory.initialArgs, hooks: new HooksContext(), ...firstStory, } as any); + firstStory.unboundStoryFn(context); expect(renderMock).toHaveBeenCalledWith( { a: 1 }, expect.objectContaining({ args: { a: 1 }, allArgs: { a: 1, b: 2 } }) @@ -599,11 +645,12 @@ describe('prepareStory', () => { { render: renderMock } ); - firstStory.unboundStoryFn({ + const context = firstStory.prepareContext({ args: firstStory.initialArgs, hooks: new HooksContext(), ...firstStory, } as any); + firstStory.unboundStoryFn(context); expect(renderMock).toHaveBeenCalledWith( { a: 1 }, expect.objectContaining({ argsByTarget: { [NO_TARGET_NAME]: { a: 1 }, foo: { b: 2 } } }) @@ -624,11 +671,12 @@ describe('prepareStory', () => { { render: renderMock } ); - firstStory.unboundStoryFn({ + const context = firstStory.prepareContext({ args: firstStory.initialArgs, hooks: new HooksContext(), ...firstStory, } as any); + firstStory.unboundStoryFn(context); expect(renderMock).toHaveBeenCalledWith( {}, expect.objectContaining({ argsByTarget: { foo: { b: 2 } } }) @@ -647,11 +695,12 @@ describe('prepareStory', () => { { render: renderMock } ); - firstStory.unboundStoryFn({ + const context = firstStory.prepareContext({ args: firstStory.initialArgs, hooks: new HooksContext(), ...firstStory, } as any); + firstStory.unboundStoryFn(context); expect(renderMock).toHaveBeenCalledWith({}, expect.objectContaining({ argsByTarget: {} })); }); }); @@ -739,6 +788,7 @@ describe('prepareMeta', () => { unboundStoryFn, undecoratedStoryFn, playFunction, + prepareContext, ...expectedPreparedMeta } = preparedStory; diff --git a/code/lib/preview-api/src/modules/store/csf/prepareStory.ts b/code/lib/preview-api/src/modules/store/csf/prepareStory.ts index 9398a8ee084e..3c3a760b84ee 100644 --- a/code/lib/preview-api/src/modules/store/csf/prepareStory.ts +++ b/code/lib/preview-api/src/modules/store/csf/prepareStory.ts @@ -71,23 +71,10 @@ export function prepareStory( }; const undecoratedStoryFn: LegacyStoryFn = (context: StoryContext) => { - const mappedArgs = Object.entries(context.args).reduce((acc, [key, val]) => { - const mapping = context.argTypes[key]?.mapping; - acc[key] = mapping && val in mapping ? mapping[val] : val; - return acc; - }, {} as Args); - - const includedArgs = Object.entries(mappedArgs).reduce((acc, [key, val]) => { - const argType = context.argTypes[key] || {}; - if (includeConditionalArg(argType, mappedArgs, context.globals)) acc[key] = val; - return acc; - }, {} as Args); - - const includedContext = { ...context, args: includedArgs }; const { passArgsFirst: renderTimePassArgsFirst = true } = context.parameters; return renderTimePassArgsFirst - ? (render as ArgsStoryFn)(includedContext.args, includedContext) - : (render as LegacyStoryFn)(includedContext); + ? (render as ArgsStoryFn)(context.args, context) + : (render as LegacyStoryFn)(context); }; // Currently it is only possible to set these globally @@ -109,8 +96,15 @@ export function prepareStory( if (!render) throw new Error(`No render function available for storyId '${id}'`); const decoratedStoryFn = applyHooks(applyDecorators)(undecoratedStoryFn, decorators); - const unboundStoryFn = (context: StoryContext) => { + const unboundStoryFn = (context: StoryContext) => decoratedStoryFn(context); + + // prepareContext is invoked at StoryRender.render() + // the context is prepared before invoking the render function, instead of here directly + // to ensure args don't loose there special properties set by the renderer + // eg. reactive proxies set by frameworks like SolidJS or Vue + const prepareContext = (context: StoryContext) => { let finalContext: StoryContext = context; + if (global.FEATURES?.argTypeTargetsV7) { const argsByTarget = groupArgsByTarget(context); finalContext = { @@ -121,7 +115,19 @@ export function prepareStory( }; } - return decoratedStoryFn(finalContext); + const mappedArgs = Object.entries(finalContext.args).reduce((acc, [key, val]) => { + const mapping = finalContext.argTypes[key]?.mapping; + acc[key] = mapping && val in mapping ? mapping[val] : val; + return acc; + }, {} as Args); + + const includedArgs = Object.entries(mappedArgs).reduce((acc, [key, val]) => { + const argType = finalContext.argTypes[key] || {}; + if (includeConditionalArg(argType, mappedArgs, finalContext.globals)) acc[key] = val; + return acc; + }, {} as Args); + + return { ...finalContext, args: includedArgs }; }; const play = storyAnnotations?.play || componentAnnotations.play; @@ -150,6 +156,7 @@ export function prepareStory( unboundStoryFn, applyLoaders, playFunction, + prepareContext, }; } diff --git a/code/lib/types/src/modules/story.ts b/code/lib/types/src/modules/story.ts index c1a5b9d6ffdd..7afeabc64b2b 100644 --- a/code/lib/types/src/modules/story.ts +++ b/code/lib/types/src/modules/story.ts @@ -84,6 +84,7 @@ export type PreparedStory = context: StoryContextForLoaders ) => Promise & { loaded: StoryContext['loaded'] }>; playFunction?: (context: StoryContext) => Promise | void; + prepareContext: (context: StoryContext) => StoryContext; }; export type PreparedMeta = Omit< From 1244c06ac8d66ae3caad8bb347b35b4caa8856cf Mon Sep 17 00:00:00 2001 From: Mauricio Rivera Date: Mon, 23 Jan 2023 17:09:23 -0500 Subject: [PATCH 2/8] Updated StoryRender.test.ts --- .../src/modules/preview-web/render/StoryRender.test.ts | 1 + 1 file changed, 1 insertion(+) 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 31236e7eacf5..25a3a3bbc27f 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 @@ -114,6 +114,7 @@ describe('StoryRender', () => { applyLoaders: jest.fn(), unboundStoryFn: jest.fn(), playFunction: jest.fn(), + prepareContext: jest.fn((ctx) => ctx), }; const renderToScreen = jest.fn(); From 8ac64a6049fce1f297866053867d631a8cf62c01 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Mon, 23 Jan 2023 23:40:18 +0100 Subject: [PATCH 3/8] fix failing snapshots --- code/lib/preview-api/src/modules/store/StoryStore.test.ts | 3 +++ 1 file changed, 3 insertions(+) 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 3e71abb50abe..8db335c84493 100644 --- a/code/lib/preview-api/src/modules/store/StoryStore.test.ts +++ b/code/lib/preview-api/src/modules/store/StoryStore.test.ts @@ -739,6 +739,7 @@ describe('StoryStore', () => { "fileName": "./src/ComponentOne.stories.js", }, "playFunction": undefined, + "prepareContext": [Function], "story": "A", "storyFn": [Function], "subcomponents": undefined, @@ -784,6 +785,7 @@ describe('StoryStore', () => { "fileName": "./src/ComponentOne.stories.js", }, "playFunction": undefined, + "prepareContext": [Function], "story": "B", "storyFn": [Function], "subcomponents": undefined, @@ -829,6 +831,7 @@ describe('StoryStore', () => { "fileName": "./src/ComponentTwo.stories.js", }, "playFunction": undefined, + "prepareContext": [Function], "story": "C", "storyFn": [Function], "subcomponents": undefined, From 297a9ee3bc8c57bcdddd28be2c16efd09c0c2b00 Mon Sep 17 00:00:00 2001 From: Mauricio Rivera Date: Mon, 23 Jan 2023 18:17:05 -0500 Subject: [PATCH 4/8] Updated MIGRATION.md --- MIGRATION.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index e71d9753d15c..bd0bb59fbf3d 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -22,7 +22,7 @@ - [Addon-a11y: Removed deprecated withA11y decorator](#addon-a11y-removed-deprecated-witha11y-decorator) - [Vite](#vite) - [Vite builder uses Vite config automatically](#vite-builder-uses-vite-config-automatically) - - [Vite cache moved to node\_modules/.cache/.vite-storybook](#vite-cache-moved-to-node_modulescachevite-storybook) + - [Vite cache moved to node_modules/.cache/.vite-storybook](#vite-cache-moved-to-node_modulescachevite-storybook) - [Webpack](#webpack) - [Webpack4 support discontinued](#webpack4-support-discontinued) - [Postcss removed](#postcss-removed) @@ -55,7 +55,7 @@ - [Dropped addon-docs manual babel configuration](#dropped-addon-docs-manual-babel-configuration) - [Dropped addon-docs manual configuration](#dropped-addon-docs-manual-configuration) - [Autoplay in docs](#autoplay-in-docs) - - [Removed STORYBOOK\_REACT\_CLASSES global](#removed-storybook_react_classes-global) + - [Removed STORYBOOK_REACT_CLASSES global](#removed-storybook_react_classes-global) - [7.0 Deprecations and default changes](#70-deprecations-and-default-changes) - [storyStoreV7 enabled by default](#storystorev7-enabled-by-default) - [`Story` type deprecated](#story-type-deprecated) @@ -273,6 +273,12 @@ A number of these changes can be made automatically by the Storybook CLI. To tak ### 7.0 breaking changes +#### Story context is prepared before for supporting fine grained updates + +This change modifies the way Storybook prepares stories to avoid reactive args to get lost for fine-grained updates JS frameworks as `SolidJS` or `Vue`. That's because those frameworks handle args/props as proxies behind the scenes to make reactivity work. So when `argType` mapping was done in `prepareStory` the Proxies were destroyed and args becomes a plain object again, losing the reactivity. + +For avoiding that, this change passes the mapped args instead of raw args at `renderToCanvas` so that the proxies stay intact. Also decorators will benefit from this as well by receiving mapped args instead of raw args. + #### Dropped support for Node 15 and below Storybook 7.0 requires **Node 16** or above. If you are using an older version of Node, you will need to upgrade or keep using Storybook 6 in the meantime. From 274373550fad8875fcca4f5a21dd3e94ac1adddd Mon Sep 17 00:00:00 2001 From: Mauricio Rivera Date: Tue, 24 Jan 2023 09:18:59 -0500 Subject: [PATCH 5/8] Impl. prepareContext at getCurrentContext and composedStory. --- .../src/modules/preview-web/render/StoryRender.ts | 13 +++++++------ .../src/modules/store/csf/testing-utils/index.ts | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) 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 741be1db123b..e7c1ae818fd0 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 @@ -180,10 +180,11 @@ export class StoryRender implements Render ({ - ...this.storyContext(), - ...(this.renderOptions.forceInitialArgs && { args: initialArgs }), - }); + const getCurrentContext = () => + prepareContext({ + ...this.storyContext(), + ...(this.renderOptions.forceInitialArgs && { args: initialArgs }), + } as StoryContext); let loadedContext: Awaited>; await this.runPhase(abortSignal, 'loading', async () => { @@ -196,7 +197,7 @@ export class StoryRender implements Render = prepareContext({ + const renderStoryContext: StoryContext = { ...loadedContext!, // By this stage, it is possible that new args/globals have been received for this story // and we need to ensure we render it with the new values @@ -204,7 +205,7 @@ export class StoryRender implements Render = { componentId, title, diff --git a/code/lib/preview-api/src/modules/store/csf/testing-utils/index.ts b/code/lib/preview-api/src/modules/store/csf/testing-utils/index.ts index 0b63fa6558aa..e2de5175f603 100644 --- a/code/lib/preview-api/src/modules/store/csf/testing-utils/index.ts +++ b/code/lib/preview-api/src/modules/store/csf/testing-utils/index.ts @@ -80,7 +80,7 @@ export function composeStory Date: Wed, 25 Jan 2023 12:18:32 -0500 Subject: [PATCH 6/8] Updated integration tests with mapped args. --- .../preview-web/PreviewWeb.mockdata.ts | 5 +- .../modules/preview-web/PreviewWeb.test.ts | 219 ++++++++++++------ 2 files changed, 146 insertions(+), 78 deletions(-) diff --git a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.mockdata.ts b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.mockdata.ts index 74af0dfbf99b..36441edc58af 100644 --- a/code/lib/preview-api/src/modules/preview-web/PreviewWeb.mockdata.ts +++ b/code/lib/preview-api/src/modules/preview-web/PreviewWeb.mockdata.ts @@ -18,14 +18,15 @@ export const componentOneExports = { title: 'Component One', argTypes: { foo: { type: { name: 'string' } }, + one: { name: 'one', type: { name: 'string' }, mapping: { 1: 'mapped-1' } }, }, loaders: [jest.fn()], parameters: { docs: { page: jest.fn(), container: jest.fn() }, }, }, - a: { args: { foo: 'a' }, play: jest.fn() }, - b: { args: { foo: 'b' }, play: jest.fn() }, + a: { args: { foo: 'a', one: 1 }, play: jest.fn() }, + b: { args: { foo: 'b', one: 1 }, play: jest.fn() }, }; export const componentTwoExports = { default: { title: 'Component Two' }, 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 766bc4edf117..9f6ef15f3666 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 @@ -219,6 +219,7 @@ describe('PreviewWeb', () => { expect(preview.storyStore.args.get('component-one--a')).toEqual({ foo: 'url', + one: 1, }); }); it('updates args from the URL', async () => { @@ -228,7 +229,7 @@ describe('PreviewWeb', () => { expect(mockChannel.emit).toHaveBeenCalledWith(STORY_ARGS_UPDATED, { storyId: 'component-one--a', - args: { foo: 'url' }, + args: { foo: 'url', one: 1 }, }); }); @@ -433,9 +434,12 @@ describe('PreviewWeb', () => { docs: expect.any(Object), fileName: './src/ComponentOne.stories.js', }, - initialArgs: { foo: 'a' }, - argTypes: { foo: { name: 'foo', type: { name: 'string' } } }, - args: { foo: 'a' }, + initialArgs: { foo: 'a', one: 1 }, + argTypes: { + foo: { name: 'foo', type: { name: 'string' } }, + one: { name: 'one', type: { name: 'string' }, mapping: { 1: 'mapped-1' } }, + }, + args: { foo: 'a', one: 1 }, }); }); @@ -451,9 +455,12 @@ describe('PreviewWeb', () => { docs: expect.any(Object), fileName: './src/ComponentOne.stories.js', }, - initialArgs: { foo: 'a' }, - argTypes: { foo: { name: 'foo', type: { name: 'string' } } }, - args: { foo: 'a' }, + initialArgs: { foo: 'a', one: 1 }, + argTypes: { + foo: { name: 'foo', type: { name: 'string' } }, + one: { name: 'one', type: { name: 'string' }, mapping: { 1: 'mapped-1' } }, + }, + args: { foo: 'a', one: 'mapped-1' }, }) ); }); @@ -473,9 +480,12 @@ describe('PreviewWeb', () => { fileName: './src/ComponentOne.stories.js', }, globals: { a: 'b' }, - initialArgs: { foo: 'a' }, - argTypes: { foo: { name: 'foo', type: { name: 'string' } } }, - args: { foo: 'a' }, + initialArgs: { foo: 'a', one: 1 }, + argTypes: { + foo: { name: 'foo', type: { name: 'string' } }, + one: { name: 'one', type: { name: 'string' }, mapping: { 1: 'mapped-1' } }, + }, + args: { foo: 'a', one: 'mapped-1' }, loaded: { l: 7 }, }), }), @@ -825,13 +835,13 @@ describe('PreviewWeb', () => { emitter.emit(UPDATE_STORY_ARGS, { storyId: 'component-one--a', - updatedArgs: { new: 'arg' }, + updatedArgs: { new: 'arg', one: 1 }, }); await waitForEvents([STORY_ARGS_UPDATED]); expect(mockChannel.emit).toHaveBeenCalledWith(STORY_ARGS_UPDATED, { storyId: 'component-one--a', - args: { foo: 'a', new: 'arg' }, + args: { foo: 'a', new: 'arg', one: 1 }, }); }); @@ -847,6 +857,7 @@ describe('PreviewWeb', () => { expect(preview.storyStore.args.get('component-one--a')).toEqual({ foo: 'a', new: 'arg', + one: 1, }); }); @@ -866,8 +877,8 @@ describe('PreviewWeb', () => { expect.objectContaining({ forceRemount: false, storyContext: expect.objectContaining({ - initialArgs: { foo: 'a' }, - args: { foo: 'a', new: 'arg' }, + initialArgs: { foo: 'a', one: 1 }, + args: { foo: 'a', new: 'arg', one: 'mapped-1' }, }), }), 'story-element' @@ -900,7 +911,7 @@ describe('PreviewWeb', () => { expect(componentOneExports.default.loaders[0]).toHaveBeenCalledWith( expect.objectContaining({ - args: { foo: 'a' }, + args: { foo: 'a', one: 'mapped-1' }, }) ); @@ -913,7 +924,7 @@ describe('PreviewWeb', () => { expect(componentOneExports.default.loaders[0]).toHaveBeenCalledWith( expect.objectContaining({ - args: { foo: 'a', new: 'arg' }, + args: { foo: 'a', new: 'arg', one: 'mapped-1' }, }) ); @@ -924,7 +935,7 @@ describe('PreviewWeb', () => { forceRemount: true, // Wasn't yet rendered so we need to force remount storyContext: expect.objectContaining({ loaded: { l: 7 }, // This is the value returned by the *second* loader call - args: { foo: 'a', new: 'arg' }, + args: { foo: 'a', new: 'arg', one: 'mapped-1' }, }), }), 'story-element' @@ -943,7 +954,7 @@ describe('PreviewWeb', () => { expect.objectContaining({ storyContext: expect.objectContaining({ loaded: { l: 8 }, - args: { foo: 'a', new: 'arg' }, + args: { foo: 'a', new: 'arg', one: 'mapped-1' }, }), }), 'story-element' @@ -974,7 +985,7 @@ describe('PreviewWeb', () => { forceRemount: true, storyContext: expect.objectContaining({ loaded: { l: 7 }, - args: { foo: 'a' }, + args: { foo: 'a', one: 'mapped-1' }, }), }), 'story-element' @@ -984,7 +995,7 @@ describe('PreviewWeb', () => { forceRemount: false, storyContext: expect.objectContaining({ loaded: { l: 7 }, - args: { foo: 'a', new: 'arg' }, + args: { foo: 'a', new: 'arg', one: 'mapped-1' }, }), }), 'story-element' @@ -1007,7 +1018,7 @@ describe('PreviewWeb', () => { forceRemount: true, storyContext: expect.objectContaining({ loaded: { l: 7 }, - args: { foo: 'a' }, + args: { foo: 'a', one: 'mapped-1' }, }), }), 'story-element' @@ -1017,7 +1028,7 @@ describe('PreviewWeb', () => { forceRemount: false, storyContext: expect.objectContaining({ loaded: { l: 7 }, - args: { foo: 'a', new: 'arg' }, + args: { foo: 'a', new: 'arg', one: 'mapped-1' }, }), }), 'story-element' @@ -1045,7 +1056,7 @@ describe('PreviewWeb', () => { forceRemount: true, storyContext: expect.objectContaining({ loaded: { l: 7 }, - args: { foo: 'a' }, + args: { foo: 'a', one: 'mapped-1' }, }), }), 'story-element' @@ -1065,7 +1076,7 @@ describe('PreviewWeb', () => { forceRemount: false, storyContext: expect.objectContaining({ loaded: { l: 7 }, - args: { foo: 'a', new: 'arg' }, + args: { foo: 'a', new: 'arg', one: 'mapped-1' }, }), }), 'story-element' @@ -1108,7 +1119,7 @@ describe('PreviewWeb', () => { expect(projectAnnotations.renderToCanvas).toHaveBeenCalledWith( expect.objectContaining({ storyContext: expect.objectContaining({ - args: { foo: 'a' }, + args: { foo: 'a', one: 'mapped-1' }, }), }), 'story-element' @@ -1125,7 +1136,7 @@ describe('PreviewWeb', () => { expect(projectAnnotations.renderToCanvas).toHaveBeenCalledWith( expect.objectContaining({ storyContext: expect.objectContaining({ - args: { foo: 'a', new: 'arg' }, + args: { foo: 'a', new: 'arg', one: 'mapped-1' }, }), }), 'story-element' @@ -1146,7 +1157,7 @@ describe('PreviewWeb', () => { expect(projectAnnotations.renderToCanvas).toHaveBeenCalledWith( expect.objectContaining({ storyContext: expect.objectContaining({ - args: { foo: 'a' }, + args: { foo: 'a', one: 'mapped-1' }, }), }), 'story-element' @@ -1227,7 +1238,7 @@ describe('PreviewWeb', () => { await waitForEvents([STORY_ARGS_UPDATED]); expect(mockChannel.emit).toHaveBeenCalledWith(STORY_ARGS_UPDATED, { storyId: 'component-one--a', - args: { foo: 'new' }, + args: { foo: 'new', one: 1 }, }); mockChannel.emit.mockClear(); @@ -1240,7 +1251,7 @@ describe('PreviewWeb', () => { expect(mockChannel.emit).toHaveBeenCalledWith(STORY_ARGS_UPDATED, { storyId: 'component-one--a', - args: { foo: 'a' }, + args: { foo: 'a', one: 1 }, }); }); @@ -1268,8 +1279,8 @@ describe('PreviewWeb', () => { expect.objectContaining({ forceRemount: false, storyContext: expect.objectContaining({ - initialArgs: { foo: 'a' }, - args: { foo: 'a', new: 'value' }, + initialArgs: { foo: 'a', one: 1 }, + args: { foo: 'a', new: 'value', one: 'mapped-1' }, }), }), 'story-element' @@ -1278,7 +1289,7 @@ describe('PreviewWeb', () => { await waitForEvents([STORY_ARGS_UPDATED]); expect(mockChannel.emit).toHaveBeenCalledWith(STORY_ARGS_UPDATED, { storyId: 'component-one--a', - args: { foo: 'a', new: 'value' }, + args: { foo: 'a', new: 'value', one: 1 }, }); expect(onUpdateArgsSpy).toHaveBeenCalledWith({ @@ -1309,8 +1320,8 @@ describe('PreviewWeb', () => { expect.objectContaining({ forceRemount: false, storyContext: expect.objectContaining({ - initialArgs: { foo: 'a' }, - args: { foo: 'a' }, + initialArgs: { foo: 'a', one: 1 }, + args: { foo: 'a', one: 'mapped-1' }, }), }), 'story-element' @@ -1319,12 +1330,12 @@ describe('PreviewWeb', () => { await waitForEvents([STORY_ARGS_UPDATED]); expect(mockChannel.emit).toHaveBeenCalledWith(STORY_ARGS_UPDATED, { storyId: 'component-one--a', - args: { foo: 'a' }, + args: { foo: 'a', one: 1 }, }); expect(onUpdateArgsSpy).toHaveBeenCalledWith({ storyId: 'component-one--a', - updatedArgs: { foo: 'a' }, + updatedArgs: { foo: 'a', one: 1 }, }); }); @@ -1350,8 +1361,8 @@ describe('PreviewWeb', () => { expect.objectContaining({ forceRemount: false, storyContext: expect.objectContaining({ - initialArgs: { foo: 'a' }, - args: { foo: 'a' }, + initialArgs: { foo: 'a', one: 1 }, + args: { foo: 'a', one: 'mapped-1' }, }), }), 'story-element' @@ -1360,12 +1371,12 @@ describe('PreviewWeb', () => { await waitForEvents([STORY_ARGS_UPDATED]); expect(mockChannel.emit).toHaveBeenCalledWith(STORY_ARGS_UPDATED, { storyId: 'component-one--a', - args: { foo: 'a' }, + args: { foo: 'a', one: 1 }, }); expect(onUpdateArgsSpy).toHaveBeenCalledWith({ storyId: 'component-one--a', - updatedArgs: { foo: 'a', new: undefined }, + updatedArgs: { foo: 'a', new: undefined, one: 1 }, }); }); @@ -1391,8 +1402,8 @@ describe('PreviewWeb', () => { expect.objectContaining({ forceRemount: false, storyContext: expect.objectContaining({ - initialArgs: { foo: 'a' }, - args: { foo: 'a' }, + initialArgs: { foo: 'a', one: 1 }, + args: { foo: 'a', one: 'mapped-1' }, }), }), 'story-element' @@ -1401,12 +1412,12 @@ describe('PreviewWeb', () => { await waitForEvents([STORY_ARGS_UPDATED]); expect(mockChannel.emit).toHaveBeenCalledWith(STORY_ARGS_UPDATED, { storyId: 'component-one--a', - args: { foo: 'a' }, + args: { foo: 'a', one: 1 }, }); expect(onUpdateArgsSpy).toHaveBeenCalledWith({ storyId: 'component-one--a', - updatedArgs: { foo: 'a' }, + updatedArgs: { foo: 'a', one: 1 }, }); }); }); @@ -1864,9 +1875,12 @@ describe('PreviewWeb', () => { docs: expect.any(Object), fileName: './src/ComponentOne.stories.js', }, - initialArgs: { foo: 'b' }, - argTypes: { foo: { name: 'foo', type: { name: 'string' } } }, - args: { foo: 'b' }, + initialArgs: { foo: 'b', one: 1 }, + argTypes: { + foo: { name: 'foo', type: { name: 'string' } }, + one: { name: 'one', type: { name: 'string' }, mapping: { 1: 'mapped-1' } }, + }, + args: { foo: 'b', one: 1 }, }); }); @@ -1890,9 +1904,12 @@ describe('PreviewWeb', () => { docs: expect.any(Object), fileName: './src/ComponentOne.stories.js', }, - initialArgs: { foo: 'b' }, - argTypes: { foo: { name: 'foo', type: { name: 'string' } } }, - args: { foo: 'b' }, + initialArgs: { foo: 'b', one: 1 }, + argTypes: { + foo: { name: 'foo', type: { name: 'string' } }, + one: { name: 'one', type: { name: 'string' }, mapping: { 1: 'mapped-1' } }, + }, + args: { foo: 'b', one: 'mapped-1' }, }) ); }); @@ -1920,9 +1937,12 @@ describe('PreviewWeb', () => { fileName: './src/ComponentOne.stories.js', }, globals: { a: 'b' }, - initialArgs: { foo: 'b' }, - argTypes: { foo: { name: 'foo', type: { name: 'string' } } }, - args: { foo: 'b' }, + initialArgs: { foo: 'b', one: 1 }, + argTypes: { + foo: { name: 'foo', type: { name: 'string' } }, + one: { name: 'one', type: { name: 'string' }, mapping: { 1: 'mapped-1' } }, + }, + args: { foo: 'b', one: 'mapped-1' }, loaded: { l: 7 }, }), }), @@ -2036,6 +2056,7 @@ describe('PreviewWeb', () => { await waitForRender(); expect(preview.storyStore.args.get('component-one--a')).toEqual({ foo: 'updated', + one: 1, }); mockChannel.emit.mockClear(); @@ -2047,6 +2068,7 @@ describe('PreviewWeb', () => { await waitForRender(); expect(preview.storyStore.args.get('component-one--a')).toEqual({ foo: 'updated', + one: 1, }); mockChannel.emit.mockClear(); @@ -2058,6 +2080,7 @@ describe('PreviewWeb', () => { await waitForRender(); expect(preview.storyStore.args.get('component-one--a')).toEqual({ foo: 'updated', + one: 1, }); }); @@ -2432,9 +2455,12 @@ describe('PreviewWeb', () => { docs: expect.any(Object), fileName: './src/ComponentOne.stories.js', }, - initialArgs: { foo: 'a' }, - argTypes: { foo: { name: 'foo', type: { name: 'string' } } }, - args: { foo: 'a' }, + initialArgs: { foo: 'a', one: 1 }, + argTypes: { + foo: { name: 'foo', type: { name: 'string' } }, + one: { name: 'one', type: { name: 'string' }, mapping: { 1: 'mapped-1' } }, + }, + args: { foo: 'a', one: 1 }, }); }); @@ -2458,9 +2484,12 @@ describe('PreviewWeb', () => { docs: expect.any(Object), fileName: './src/ComponentOne.stories.js', }, - initialArgs: { foo: 'a' }, - argTypes: { foo: { name: 'foo', type: { name: 'string' } } }, - args: { foo: 'a' }, + initialArgs: { foo: 'a', one: 1 }, + argTypes: { + foo: { name: 'foo', type: { name: 'string' } }, + one: { name: 'one', type: { name: 'string' }, mapping: { 1: 'mapped-1' } }, + }, + args: { foo: 'a', one: 'mapped-1' }, }) ); }); @@ -2488,9 +2517,12 @@ describe('PreviewWeb', () => { fileName: './src/ComponentOne.stories.js', }, globals: { a: 'b' }, - initialArgs: { foo: 'a' }, - argTypes: { foo: { name: 'foo', type: { name: 'string' } } }, - args: { foo: 'a' }, + initialArgs: { foo: 'a', one: 1 }, + argTypes: { + foo: { name: 'foo', type: { name: 'string' } }, + one: { name: 'one', type: { name: 'string' }, mapping: { 1: 'mapped-1' } }, + }, + args: { foo: 'a', one: 'mapped-1' }, loaded: { l: 7 }, }), }), @@ -2635,6 +2667,7 @@ describe('PreviewWeb', () => { await waitForRender(); expect(preview.storyStore.args.get('component-one--a')).toEqual({ foo: 'url', + one: 1, }); }); }); @@ -2697,9 +2730,12 @@ describe('PreviewWeb', () => { docs: expect.any(Object), fileName: './src/ComponentOne.stories.js', }, - initialArgs: { foo: 'edited' }, - argTypes: { foo: { name: 'foo', type: { name: 'string' } } }, - args: { foo: 'edited' }, + initialArgs: { foo: 'edited', one: 1 }, + argTypes: { + foo: { name: 'foo', type: { name: 'string' } }, + one: { name: 'one', type: { name: 'string' }, mapping: { 1: 'mapped-1' } }, + }, + args: { foo: 'edited', one: 1 }, }); }); @@ -2713,7 +2749,7 @@ describe('PreviewWeb', () => { expect(mockChannel.emit).toHaveBeenCalledWith(STORY_ARGS_UPDATED, { storyId: 'component-one--a', - args: { foo: 'edited' }, + args: { foo: 'edited', one: 1 }, }); }); @@ -2734,9 +2770,12 @@ describe('PreviewWeb', () => { docs: expect.any(Object), fileName: './src/ComponentOne.stories.js', }, - initialArgs: { foo: 'edited' }, - argTypes: { foo: { name: 'foo', type: { name: 'string' } } }, - args: { foo: 'edited' }, + initialArgs: { foo: 'edited', one: 1 }, + argTypes: { + foo: { name: 'foo', type: { name: 'string' } }, + one: { name: 'one', type: { name: 'string' }, mapping: { 1: 'mapped-1' } }, + }, + args: { foo: 'edited', one: 'mapped-1' }, }) ); }); @@ -2761,9 +2800,12 @@ describe('PreviewWeb', () => { fileName: './src/ComponentOne.stories.js', }, globals: { a: 'b' }, - initialArgs: { foo: 'edited' }, - argTypes: { foo: { name: 'foo', type: { name: 'string' } } }, - args: { foo: 'edited' }, + initialArgs: { foo: 'edited', one: 1 }, + argTypes: { + foo: { name: 'foo', type: { name: 'string' } }, + one: { name: 'one', type: { name: 'string' }, mapping: { 1: 'mapped-1' } }, + }, + args: { foo: 'edited', one: 'mapped-1' }, loaded: { l: 7 }, }), }), @@ -2792,7 +2834,7 @@ describe('PreviewWeb', () => { forceRemount: true, storyContext: expect.objectContaining({ id: 'component-one--a', - args: { foo: 'updated' }, + args: { foo: 'updated', one: 'mapped-1' }, }), }), 'story-element' @@ -3031,6 +3073,7 @@ describe('PreviewWeb', () => { await waitForRender(); expect(preview.storyStore.args.get('component-one--a')).toEqual({ foo: 'updated', + one: 1, }); // Update story A's args via HMR @@ -3050,6 +3093,7 @@ describe('PreviewWeb', () => { expect(preview.storyStore.args.get('component-one--a')).toEqual({ foo: 'updated', bar: 'edited', + one: 1, }); expect(projectAnnotations.renderToCanvas).toHaveBeenCalledWith( @@ -3057,7 +3101,7 @@ describe('PreviewWeb', () => { forceRemount: true, storyContext: expect.objectContaining({ id: 'component-one--a', - args: { foo: 'updated', bar: 'edited' }, + args: { foo: 'updated', bar: 'edited', one: 'mapped-1' }, }), }), 'story-element' @@ -3288,6 +3332,7 @@ describe('PreviewWeb', () => { expect(preview.storyStore.args.get('component-one--a')).toEqual({ foo: 'a', + one: 1, global: 'added', }); }); @@ -3302,7 +3347,7 @@ describe('PreviewWeb', () => { expect(mockChannel.emit).toHaveBeenCalledWith(STORY_ARGS_UPDATED, { storyId: 'component-one--a', - args: { foo: 'a', global: 'added' }, + args: { foo: 'a', one: 1, global: 'added' }, }); }); @@ -3330,7 +3375,7 @@ describe('PreviewWeb', () => { expect(projectAnnotations.renderToCanvas).toHaveBeenCalledWith( expect.objectContaining({ storyContext: expect.objectContaining({ - args: { foo: 'a', global: 'added' }, + args: { foo: 'a', one: 'mapped-1', global: 'added' }, globals: { a: 'edited' }, }), }), @@ -3479,15 +3524,26 @@ describe('PreviewWeb', () => { "name": "string", }, }, + "one": Object { + "mapping": Object { + "1": "mapped-1", + }, + "name": "one", + "type": Object { + "name": "string", + }, + }, }, "args": Object { "foo": "a", + "one": 1, }, "component": undefined, "componentId": "component-one", "id": "component-one--a", "initialArgs": Object { "foo": "a", + "one": 1, }, "kind": "Component One", "name": "A", @@ -3515,15 +3571,26 @@ describe('PreviewWeb', () => { "name": "string", }, }, + "one": Object { + "mapping": Object { + "1": "mapped-1", + }, + "name": "one", + "type": Object { + "name": "string", + }, + }, }, "args": Object { "foo": "b", + "one": 1, }, "component": undefined, "componentId": "component-one", "id": "component-one--b", "initialArgs": Object { "foo": "b", + "one": 1, }, "kind": "Component One", "name": "B", From 975dcd7dca77f71b8d5ddb30f8a8c803d9b21c18 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Thu, 26 Jan 2023 01:22:29 +0100 Subject: [PATCH 7/8] lower default depth of action events logged --- code/addons/actions/src/runtime/configureActions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/addons/actions/src/runtime/configureActions.ts b/code/addons/actions/src/runtime/configureActions.ts index 60bcc01baef5..aad3f744c488 100644 --- a/code/addons/actions/src/runtime/configureActions.ts +++ b/code/addons/actions/src/runtime/configureActions.ts @@ -1,7 +1,7 @@ import type { ActionOptions } from '../models'; export const config: ActionOptions = { - depth: 10, + depth: 6, clearOnStoryChange: true, limit: 50, }; From 5344bd17421860b917b0828a30c3851d7c0eeaff Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Thu, 26 Jan 2023 12:10:41 +0100 Subject: [PATCH 8/8] fix untargeted args causing telejson to miss cyclical structures --- code/addons/actions/src/runtime/configureActions.ts | 2 +- code/lib/preview-api/src/modules/store/args.test.ts | 4 ++-- code/lib/preview-api/src/modules/store/args.ts | 6 +++--- .../preview-api/src/modules/store/csf/prepareStory.test.ts | 4 ++-- code/lib/preview-api/src/modules/store/csf/prepareStory.ts | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/code/addons/actions/src/runtime/configureActions.ts b/code/addons/actions/src/runtime/configureActions.ts index aad3f744c488..60bcc01baef5 100644 --- a/code/addons/actions/src/runtime/configureActions.ts +++ b/code/addons/actions/src/runtime/configureActions.ts @@ -1,7 +1,7 @@ import type { ActionOptions } from '../models'; export const config: ActionOptions = { - depth: 6, + depth: 10, clearOnStoryChange: true, limit: 50, }; diff --git a/code/lib/preview-api/src/modules/store/args.test.ts b/code/lib/preview-api/src/modules/store/args.test.ts index 8d00cafaf945..e134129acaed 100644 --- a/code/lib/preview-api/src/modules/store/args.test.ts +++ b/code/lib/preview-api/src/modules/store/args.test.ts @@ -6,7 +6,7 @@ import { combineArgs, groupArgsByTarget, mapArgsToTypes, - NO_TARGET_NAME, + UNTARGETED, validateOptions, } from './args'; @@ -295,7 +295,7 @@ describe('groupArgsByTarget', () => { argTypes: { b: { name: 'b', target: 'group2' }, c: {} }, } as any); expect(groups).toEqual({ - [NO_TARGET_NAME]: { + [UNTARGETED]: { a: 1, c: 3, }, diff --git a/code/lib/preview-api/src/modules/store/args.ts b/code/lib/preview-api/src/modules/store/args.ts index 37790234dbad..4dcccb6261a8 100644 --- a/code/lib/preview-api/src/modules/store/args.ts +++ b/code/lib/preview-api/src/modules/store/args.ts @@ -144,14 +144,14 @@ export const deepDiff = (value: any, update: any): any => { return update; }; -export const NO_TARGET_NAME = ''; +export const UNTARGETED = 'UNTARGETED'; export function groupArgsByTarget({ args, argTypes, }: StoryContext) { const groupedArgs: Record> = {}; (Object.entries(args) as [keyof TArgs, any][]).forEach(([name, value]) => { - const { target = NO_TARGET_NAME } = (argTypes[name] || {}) as { target?: string }; + const { target = UNTARGETED } = (argTypes[name] || {}) as { target?: string }; groupedArgs[target] = groupedArgs[target] || {}; groupedArgs[target][name] = value; @@ -160,5 +160,5 @@ export function groupArgsByTarget({ } export function noTargetArgs(context: StoryContext) { - return groupArgsByTarget(context)[NO_TARGET_NAME]; + return groupArgsByTarget(context)[UNTARGETED]; } diff --git a/code/lib/preview-api/src/modules/store/csf/prepareStory.test.ts b/code/lib/preview-api/src/modules/store/csf/prepareStory.test.ts index 9709769cc4cd..fe491e25f107 100644 --- a/code/lib/preview-api/src/modules/store/csf/prepareStory.test.ts +++ b/code/lib/preview-api/src/modules/store/csf/prepareStory.test.ts @@ -5,7 +5,7 @@ import { expect } from '@jest/globals'; import type { Renderer, ArgsEnhancer, PlayFunctionContext, SBScalarType } from '@storybook/types'; import { addons, HooksContext } from '../../addons'; -import { NO_TARGET_NAME } from '../args'; +import { UNTARGETED } from '../args'; import { prepareStory, prepareMeta } from './prepareStory'; jest.mock('@storybook/global', () => ({ @@ -595,7 +595,7 @@ describe('prepareStory', () => { firstStory.unboundStoryFn(context); expect(renderMock).toHaveBeenCalledWith( { a: 1 }, - expect.objectContaining({ argsByTarget: { [NO_TARGET_NAME]: { a: 1 }, foo: { b: 2 } } }) + expect.objectContaining({ argsByTarget: { [UNTARGETED]: { a: 1 }, foo: { b: 2 } } }) ); }); diff --git a/code/lib/preview-api/src/modules/store/csf/prepareStory.ts b/code/lib/preview-api/src/modules/store/csf/prepareStory.ts index 4e6e86a61412..9053108e9e78 100644 --- a/code/lib/preview-api/src/modules/store/csf/prepareStory.ts +++ b/code/lib/preview-api/src/modules/store/csf/prepareStory.ts @@ -25,7 +25,7 @@ import { includeConditionalArg } from '@storybook/csf'; import { applyHooks } from '../../addons'; import { combineParameters } from '../parameters'; import { defaultDecorateStory } from '../decorators'; -import { groupArgsByTarget, NO_TARGET_NAME } from '../args'; +import { groupArgsByTarget, UNTARGETED } from '../args'; // Combine all the metadata about a story (both direct and inherited from the component/global scope) // into a "renderable" story function, with all decorators applied, parameters passed as context etc @@ -100,7 +100,7 @@ export function prepareStory( ...context, allArgs: context.args, argsByTarget, - args: argsByTarget[NO_TARGET_NAME] || {}, + args: argsByTarget[UNTARGETED] || {}, }; }