Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core: Story context is prepared before for supporting fine grained updates #20755

Merged
merged 11 commits into from
Jan 27, 2023
10 changes: 8 additions & 2 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ describe('StoryRender', () => {
applyLoaders: jest.fn(),
unboundStoryFn: jest.fn(),
playFunction: jest.fn(),
prepareContext: jest.fn(),
};

const render = new StoryRender(
Expand All @@ -85,6 +86,7 @@ describe('StoryRender', () => {
applyLoaders: jest.fn(),
unboundStoryFn: jest.fn(),
playFunction: jest.fn(),
prepareContext: jest.fn(),
};

const render = new StoryRender(
Expand Down Expand Up @@ -112,6 +114,7 @@ describe('StoryRender', () => {
applyLoaders: jest.fn(),
unboundStoryFn: jest.fn(),
playFunction: jest.fn(),
prepareContext: jest.fn((ctx) => ctx),
};

const renderToScreen = jest.fn();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ export class StoryRender<TRenderer extends Renderer> implements Render<TRenderer
applyLoaders,
unboundStoryFn,
playFunction,
prepareContext,
initialArgs,
} = this.story;

Expand Down Expand Up @@ -195,15 +196,15 @@ export class StoryRender<TRenderer extends Renderer> implements Render<TRenderer
return;
}

const renderStoryContext: StoryContext<TRenderer> = {
const renderStoryContext: StoryContext<TRenderer> = prepareContext({
tmeasday marked this conversation as resolved.
Show resolved Hide resolved
...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
...getCurrentContext(),
abortSignal,
// We should consider parameterizing the story types with TRenderer['canvasElement'] in the future
canvasElement: canvasElement as any,
};
});
const renderContext: RenderContext<TRenderer> = {
componentId,
title,
Expand Down
3 changes: 3 additions & 0 deletions code/lib/preview-api/src/modules/store/StoryStore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,7 @@ describe('StoryStore', () => {
"fileName": "./src/ComponentOne.stories.js",
},
"playFunction": undefined,
"prepareContext": [Function],
"story": "A",
"storyFn": [Function],
"subcomponents": undefined,
Expand Down Expand Up @@ -784,6 +785,7 @@ describe('StoryStore', () => {
"fileName": "./src/ComponentOne.stories.js",
},
"playFunction": undefined,
"prepareContext": [Function],
"story": "B",
"storyFn": [Function],
"subcomponents": undefined,
Expand Down Expand Up @@ -829,6 +831,7 @@ describe('StoryStore', () => {
"fileName": "./src/ComponentTwo.stories.js",
},
"playFunction": undefined,
"prepareContext": [Function],
"story": "C",
"storyFn": [Function],
"subcomponents": undefined,
Expand Down
64 changes: 57 additions & 7 deletions code/lib/preview-api/src/modules/store/csf/prepareStory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -410,8 +410,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 } })
Expand Down Expand Up @@ -471,6 +471,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`', () => {
Expand All @@ -491,11 +535,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 } })
Expand All @@ -516,11 +561,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 } })
Expand All @@ -541,11 +587,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 } } })
Expand All @@ -566,11 +613,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 } } })
Expand All @@ -589,11 +637,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: {} }));
});
});
Expand Down Expand Up @@ -681,6 +730,7 @@ describe('prepareMeta', () => {
unboundStoryFn,
undecoratedStoryFn,
playFunction,
prepareContext,
...expectedPreparedMeta
} = preparedStory;

Expand Down
41 changes: 24 additions & 17 deletions code/lib/preview-api/src/modules/store/csf/prepareStory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,23 +60,10 @@ export function prepareStory<TRenderer extends Renderer>(
};

const undecoratedStoryFn: LegacyStoryFn<TRenderer> = (context: StoryContext<TRenderer>) => {
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<TRenderer>)(includedContext.args, includedContext)
: (render as LegacyStoryFn<TRenderer>)(includedContext);
? (render as ArgsStoryFn<TRenderer>)(context.args, context)
: (render as LegacyStoryFn<TRenderer>)(context);
};

// Currently it is only possible to set these globally
Expand All @@ -98,8 +85,15 @@ export function prepareStory<TRenderer extends Renderer>(
if (!render) throw new Error(`No render function available for storyId '${id}'`);

const decoratedStoryFn = applyHooks<TRenderer>(applyDecorators)(undecoratedStoryFn, decorators);
const unboundStoryFn = (context: StoryContext<TRenderer>) => {
const unboundStoryFn = (context: StoryContext<TRenderer>) => 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<TRenderer>) => {
let finalContext: StoryContext<TRenderer> = context;

if (global.FEATURES?.argTypeTargetsV7) {
const argsByTarget = groupArgsByTarget(context);
finalContext = {
Expand All @@ -110,7 +104,19 @@ export function prepareStory<TRenderer extends Renderer>(
};
}

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;
Expand Down Expand Up @@ -139,6 +145,7 @@ export function prepareStory<TRenderer extends Renderer>(
unboundStoryFn,
applyLoaders,
playFunction,
prepareContext,
};
}

Expand Down
1 change: 1 addition & 0 deletions code/lib/types/src/modules/story.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ export type PreparedStory<TRenderer extends Renderer = Renderer> =
context: StoryContextForLoaders<TRenderer>
) => Promise<StoryContextForLoaders<TRenderer> & { loaded: StoryContext<TRenderer>['loaded'] }>;
playFunction?: (context: StoryContext<TRenderer>) => Promise<void> | void;
prepareContext: (context: StoryContext<TRenderer>) => StoryContext<TRenderer>;
};

export type PreparedMeta<TRenderer extends Renderer = Renderer> = Omit<
Expand Down