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
@@ -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)
@@ -61,7 +61,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)
@@ -279,6 +279,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.
Original file line number Diff line number Diff line change
@@ -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' },
219 changes: 143 additions & 76 deletions code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -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(
@@ -112,6 +114,7 @@ describe('StoryRender', () => {
applyLoaders: jest.fn(),
unboundStoryFn: jest.fn(),
playFunction: jest.fn(),
prepareContext: jest.fn((ctx) => ctx),
};

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

@@ -179,10 +180,11 @@ export class StoryRender<TRenderer extends Renderer> implements Render<TRenderer
const abortSignal = (this.abortController as AbortController).signal;

try {
const getCurrentContext = () => ({
...this.storyContext(),
...(this.renderOptions.forceInitialArgs && { args: initialArgs }),
});
const getCurrentContext = () =>
prepareContext({
...this.storyContext(),
...(this.renderOptions.forceInitialArgs && { args: initialArgs }),
} as StoryContext);

let loadedContext: Awaited<ReturnType<typeof applyLoaders>>;
await this.runPhase(abortSignal, 'loading', async () => {
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
@@ -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,
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
@@ -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 } })
@@ -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`', () => {
@@ -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 } })
@@ -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 } })
@@ -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 } } })
@@ -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 } } })
@@ -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: {} }));
});
});
@@ -681,6 +730,7 @@ describe('prepareMeta', () => {
unboundStoryFn,
undecoratedStoryFn,
playFunction,
prepareContext,
...expectedPreparedMeta
} = preparedStory;

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

Original file line number Diff line number Diff line change
@@ -80,7 +80,7 @@ export function composeStory<TRenderer extends Renderer = Renderer, TArgs extend
args: { ...story.initialArgs, ...extraArgs },
};

return story.unboundStoryFn(context as StoryContext);
return story.unboundStoryFn(story.prepareContext(context as StoryContext));
};

composedStory.storyName = storyName;
1 change: 1 addition & 0 deletions code/lib/types/src/modules/story.ts
Original file line number Diff line number Diff line change
@@ -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<