Skip to content

Core: Implement afterEach #29584

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Closed
kasperpeulen wants to merge 7 commits into from
Closed

Core: Implement afterEach #29584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a913610
Implement afterEach
kasperpeulen Nov 11, 2024
3065626
Test afterEach
kasperpeulen Nov 12, 2024
83af2a6
Add afterEach normalizers
kasperpeulen Nov 12, 2024
8029b37
Fix test
kasperpeulen Nov 12, 2024
b918869
Bump @storybook/csf
kasperpeulen Nov 12, 2024
312b25d
Make dedicated order-of-hooks test
kasperpeulen Nov 12, 2024
e038e8b
Fix tests
kasperpeulen Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion code/addons/links/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
"prep": "jiti ../../../scripts/prepare/addon-bundle.ts"
},
"dependencies": {
"@storybook/csf": "^0.1.11",
"@storybook/csf": "0.1.12--canary.109.1cc9957.0",
"@storybook/global": "^5.0.0",
"ts-dedent": "^2.0.0"
},
Expand Down
2 changes: 1 addition & 1 deletion code/addons/test/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
"prep": "jiti ../../../scripts/prepare/addon-bundle.ts"
},
"dependencies": {
"@storybook/csf": "^0.1.11",
"@storybook/csf": "0.1.12--canary.109.1cc9957.0",
"@storybook/global": "^5.0.0",
"@storybook/icons": "^1.2.12",
"@storybook/instrumenter": "workspace:*",
Expand Down
2 changes: 1 addition & 1 deletion code/core/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@
"prep": "jiti ./scripts/prep.ts"
},
"dependencies": {
"@storybook/csf": "^0.1.11",
"@storybook/csf": "0.1.12--canary.109.1cc9957.0",
"better-opn": "^3.0.2",
"browser-assert": "^1.2.1",
"esbuild": "^0.18.0 || ^0.19.0 || ^0.20.0 || ^0.21.0 || ^0.22.0 || ^0.23.0 || ^0.24.0",
Expand Down
1 change: 1 addition & 0 deletions code/core/src/preview-api/modules/preview-web/render/StoryRender.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const buildStory = (overrides: Partial<PreparedStory> = {}): PreparedStory =>
tags: [],
applyLoaders: vi.fn(),
applyBeforeEach: vi.fn(),
applyAfterEach: vi.fn(),
unboundStoryFn: vi.fn(),
playFunction: vi.fn(),
mount: (context: StoryContext) => () => mountSpy(context),
Expand Down
10 changes: 9 additions & 1 deletion code/core/src/preview-api/modules/preview-web/render/StoryRender.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export type RenderPhase =
| 'rendering'
| 'playing'
| 'played'
| 'afterEach'
| 'completed'
| 'aborted'
| 'errored';
Expand Down Expand Up @@ -132,7 +133,9 @@ export class StoryRender<TRenderer extends Renderer> implements Render<TRenderer
}

isPending() {
return ['loading', 'beforeEach', 'rendering', 'playing'].includes(this.phase as RenderPhase);
return ['loading', 'beforeEach', 'rendering', 'playing', 'afterEach'].includes(
this.phase as RenderPhase
);
}

async renderToElement(canvasElement: TRenderer['canvasElement']) {
Expand Down Expand Up @@ -180,6 +183,7 @@ export class StoryRender<TRenderer extends Renderer> implements Render<TRenderer
tags,
applyLoaders,
applyBeforeEach,
applyAfterEach,
unboundStoryFn,
playFunction,
runStep,
Expand Down Expand Up @@ -318,6 +322,10 @@ export class StoryRender<TRenderer extends Renderer> implements Render<TRenderer
} else {
await this.runPhase(abortSignal, 'played');
}

await this.runPhase(abortSignal, 'afterEach', async () => {
await applyAfterEach(context);
});
} catch (error) {
// Remove the loading screen, even if there was an error before rendering
this.callbacks.showStoryDuringRender?.();
Expand Down
3 changes: 3 additions & 0 deletions code/core/src/preview-api/modules/store/StoryStore.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,7 @@ describe('StoryStore', () => {
expect(store.raw()).toMatchInlineSnapshot(`
[
{
"applyAfterEach": [Function],
"applyBeforeEach": [Function],
"applyLoaders": [Function],
"argTypes": {
Expand Down Expand Up @@ -698,6 +699,7 @@ describe('StoryStore', () => {
"usesMount": false,
},
{
"applyAfterEach": [Function],
"applyBeforeEach": [Function],
"applyLoaders": [Function],
"argTypes": {
Expand Down Expand Up @@ -751,6 +753,7 @@ describe('StoryStore', () => {
"usesMount": false,
},
{
"applyAfterEach": [Function],
"applyBeforeEach": [Function],
"applyLoaders": [Function],
"argTypes": {
Expand Down
10 changes: 10 additions & 0 deletions code/core/src/preview-api/modules/store/csf/composeConfigs.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ describe('composeConfigs', () => {
loaders: [],
beforeAll: expect.any(Function),
beforeEach: [],
afterEach: [],
runStep: expect.any(Function),
tags: [],
});
Expand Down Expand Up @@ -53,6 +54,7 @@ describe('composeConfigs', () => {
loaders: [],
beforeAll: expect.any(Function),
beforeEach: [],
afterEach: [],
runStep: expect.any(Function),
tags: [],
});
Expand Down Expand Up @@ -85,6 +87,7 @@ describe('composeConfigs', () => {
loaders: [],
beforeAll: expect.any(Function),
beforeEach: [],
afterEach: [],
runStep: expect.any(Function),
tags: [],
});
Expand Down Expand Up @@ -123,6 +126,7 @@ describe('composeConfigs', () => {
loaders: [],
beforeAll: expect.any(Function),
beforeEach: [],
afterEach: [],
runStep: expect.any(Function),
tags: [],
});
Expand Down Expand Up @@ -164,6 +168,7 @@ describe('composeConfigs', () => {
loaders: [],
beforeAll: expect.any(Function),
beforeEach: [],
afterEach: [],
runStep: expect.any(Function),
tags: [],
});
Expand Down Expand Up @@ -196,6 +201,7 @@ describe('composeConfigs', () => {
loaders: ['1', '2', '3', '4'],
beforeAll: expect.any(Function),
beforeEach: [],
afterEach: [],
runStep: expect.any(Function),
tags: [],
});
Expand Down Expand Up @@ -228,6 +234,7 @@ describe('composeConfigs', () => {
loaders: ['1', '2', '3'],
beforeAll: expect.any(Function),
beforeEach: [],
afterEach: [],
runStep: expect.any(Function),
tags: [],
});
Expand Down Expand Up @@ -256,6 +263,7 @@ describe('composeConfigs', () => {
loaders: [],
beforeAll: expect.any(Function),
beforeEach: [],
afterEach: [],
runStep: expect.any(Function),
tags: [],
});
Expand Down Expand Up @@ -285,6 +293,7 @@ describe('composeConfigs', () => {
loaders: [],
beforeAll: expect.any(Function),
beforeEach: [],
afterEach: [],
runStep: expect.any(Function),
tags: [],
});
Expand Down Expand Up @@ -317,6 +326,7 @@ describe('composeConfigs', () => {
loaders: [],
beforeAll: expect.any(Function),
beforeEach: [],
afterEach: [],
render: 'render-2',
renderToCanvas: 'renderToCanvas-2',
applyDecorators: 'applyDecorators-2',
Expand Down
1 change: 1 addition & 0 deletions code/core/src/preview-api/modules/store/csf/composeConfigs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export function composeConfigs<TRenderer extends Renderer>(
loaders: getArrayField(moduleExportList, 'loaders'),
beforeAll: composeBeforeAllHooks(beforeAllHooks),
beforeEach: getArrayField(moduleExportList, 'beforeEach'),
afterEach: getArrayField(moduleExportList, 'afterEach'),
render: getSingletonField(moduleExportList, 'render'),
renderToCanvas: getSingletonField(moduleExportList, 'renderToCanvas'),
renderToDOM: getSingletonField(moduleExportList, 'renderToDOM'), // deprecated
Expand Down
2 changes: 2 additions & 0 deletions code/core/src/preview-api/modules/store/csf/normalizeProjectAnnotations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export function normalizeProjectAnnotations<TRenderer extends Renderer>({
decorators,
loaders,
beforeEach,
afterEach,
globals,
initialGlobals,
...annotations
Expand All @@ -44,6 +45,7 @@ export function normalizeProjectAnnotations<TRenderer extends Renderer>({
decorators: normalizeArrays(decorators),
loaders: normalizeArrays(loaders),
beforeEach: normalizeArrays(beforeEach),
afterEach: normalizeArrays(afterEach),
argTypesEnhancers: [
...(argTypesEnhancers || []),
inferArgTypes,
Expand Down
4 changes: 4 additions & 0 deletions code/core/src/preview-api/modules/store/csf/normalizeStory.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ describe('normalizeStory', () => {
const meta = { id: 'title', title: 'title' };
expect(normalizeStory('storyExport', storyFn, meta)).toMatchInlineSnapshot(`
{
"afterEach": [],
"argTypes": {},
"args": {},
"beforeEach": [],
Expand Down Expand Up @@ -123,6 +124,7 @@ describe('normalizeStory', () => {
const normalized = normalizeStory('storyExport', storyObj, meta);
expect(normalized).toMatchInlineSnapshot(`
{
"afterEach": [],
"argTypes": {},
"args": {},
"beforeEach": [],
Expand Down Expand Up @@ -152,6 +154,7 @@ describe('normalizeStory', () => {
const { moduleExport, ...normalized } = normalizeStory('storyExport', storyObj, meta);
expect(normalized).toMatchInlineSnapshot(`
{
"afterEach": [],
"argTypes": {
"storyArgType": {
"name": "storyArgType",
Expand Down Expand Up @@ -202,6 +205,7 @@ describe('normalizeStory', () => {
const { moduleExport, ...normalized } = normalizeStory('storyExport', storyObj, meta);
expect(normalized).toMatchInlineSnapshot(`
{
"afterEach": [],
"argTypes": {
"storyArgType": {
"name": "storyArgType",
Expand Down
5 changes: 5 additions & 0 deletions code/core/src/preview-api/modules/store/csf/normalizeStory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ export function normalizeStory<TRenderer extends Renderer>(
...normalizeArrays(storyObject.beforeEach),
...normalizeArrays(story?.beforeEach),
];
const afterEach = [
...normalizeArrays(storyObject.afterEach),
...normalizeArrays(story?.afterEach),
];
const { render, play, tags = [], globals = {} } = storyObject;

// eslint-disable-next-line no-underscore-dangle
Expand All @@ -75,6 +79,7 @@ export function normalizeStory<TRenderer extends Renderer>(
argTypes: normalizeInputTypes(argTypes),
loaders,
beforeEach,
afterEach,
globals,
...(render && { render }),
...(userStoryFn && { userStoryFn }),
Expand Down
2 changes: 2 additions & 0 deletions code/core/src/preview-api/modules/store/csf/portable-stories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,4 +408,6 @@ async function runStory<TRenderer extends Renderer>(
}
await playFunction(context);
}

await story.applyAfterEach(context);
Comment on lines +411 to +412

@greptile-apps greptile-apps Bot Nov 11, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: afterEach should run even if playFunction throws an error. Consider wrapping the play function in try/finally to ensure afterEach always executes.

}
1 change: 1 addition & 0 deletions code/core/src/preview-api/modules/store/csf/prepareStory.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,7 @@ describe('prepareMeta', () => {
story,
applyLoaders,
applyBeforeEach,
applyAfterEach,
originalStoryFn,
unboundStoryFn,
undecoratedStoryFn,
Expand Down
15 changes: 15 additions & 0 deletions code/core/src/preview-api/modules/store/csf/prepareStory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,20 @@ export function prepareStory<TRenderer extends Renderer>(
return cleanupCallbacks;
};

const applyAfterEach = async (context: StoryContext<TRenderer>): Promise<void> => {
const reversedFinalizers = [
...normalizeArrays(projectAnnotations.afterEach),
...normalizeArrays(componentAnnotations.afterEach),
...normalizeArrays(storyAnnotations.afterEach),
].reverse();
for (const finalizer of reversedFinalizers) {
if (context.abortSignal.aborted) {
return;
}
Comment on lines +102 to +104

@greptile-apps greptile-apps Bot Nov 11, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider handling errors from finalizers to ensure all cleanup runs even if one fails

await finalizer(context);
}
};

const undecoratedStoryFn = (context: StoryContext<TRenderer>) =>
(context.originalStoryFn as ArgsStoryFn<TRenderer>)(context.args, context);

Expand Down Expand Up @@ -150,6 +164,7 @@ export function prepareStory<TRenderer extends Renderer>(
unboundStoryFn,
applyLoaders,
applyBeforeEach,
applyAfterEach,
playFunction,
runStep,
mount,
Expand Down
1 change: 1 addition & 0 deletions code/core/src/types/modules/story.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ export type PreparedStory<TRenderer extends Renderer = Renderer> =
unboundStoryFn: LegacyStoryFn<TRenderer>;
applyLoaders: (context: StoryContext<TRenderer>) => Promise<StoryContext<TRenderer>['loaded']>;
applyBeforeEach: (context: StoryContext<TRenderer>) => Promise<CleanupCallback[]>;
applyAfterEach: (context: StoryContext<TRenderer>) => Promise<void>;
playFunction?: (context: StoryContext<TRenderer>) => Promise<void> | void;
runStep: StepRunner<TRenderer>;
mount: (context: StoryContext<TRenderer>) => () => Promise<Canvas>;
Expand Down
2 changes: 1 addition & 1 deletion code/lib/blocks/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
"prep": "jiti ../../../scripts/prepare/bundle.ts"
},
"dependencies": {
"@storybook/csf": "^0.1.11",
"@storybook/csf": "0.1.12--canary.109.1cc9957.0",
"@storybook/icons": "^1.2.12",
"ts-dedent": "^2.0.0"
},
Expand Down
2 changes: 1 addition & 1 deletion code/lib/codemod/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
"@babel/preset-env": "^7.24.4",
"@babel/types": "^7.24.0",
"@storybook/core": "workspace:*",
"@storybook/csf": "^0.1.11",
"@storybook/csf": "0.1.12--canary.109.1cc9957.0",
"@types/cross-spawn": "^6.0.2",
"cross-spawn": "^7.0.3",
"es-toolkit": "^1.22.0",
Expand Down
2 changes: 1 addition & 1 deletion code/lib/source-loader/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
"prep": "jiti ../../../scripts/prepare/bundle.ts"
},
"dependencies": {
"@storybook/csf": "^0.1.11",
"@storybook/csf": "0.1.12--canary.109.1cc9957.0",
"es-toolkit": "^1.22.0",
"estraverse": "^5.2.0",
"prettier": "^3.1.1"
Expand Down
2 changes: 1 addition & 1 deletion code/lib/test/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
"prep": "jiti ../../../scripts/prepare/bundle.ts"
},
"dependencies": {
"@storybook/csf": "^0.1.11",
"@storybook/csf": "0.1.12--canary.109.1cc9957.0",
"@storybook/global": "^5.0.0",
"@storybook/instrumenter": "workspace:*",
"@testing-library/dom": "10.4.0",
Expand Down
36 changes: 1 addition & 35 deletions code/lib/test/template/stories/before-each.stories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,45 +3,11 @@ import { expect, getByRole, mocked, spyOn, userEvent } from '@storybook/test';

const meta = {
component: globalThis.Components.Button,
loaders() {
spyOn(console, 'log').mockName('console.log');
console.log('1 - [from loaders]');
},
beforeEach() {
console.log('2 - [from meta beforeEach]');
},
args: { label: 'Button' },
};

export default meta;

export const BeforeEachOrder = {
parameters: { chromatic: { disable: true } },
beforeEach() {
console.log('3 - [from story beforeEach]');
},
decorators: (StoryFn: any) => {
console.log('4 - [from decorator]');
return StoryFn();
},
args: {
label: 'Button',
onClick: () => {
console.log('5 - [from onClick]');
},
},
async play({ canvasElement }: any) {
await userEvent.click(getByRole(canvasElement, 'button'));

await expect(mocked(console.log).mock.calls).toEqual([
['1 - [from loaders]'],
['2 - [from meta beforeEach]'],
['3 - [from story beforeEach]'],
['4 - [from decorator]'],
['5 - [from onClick]'],
]);
},
};

export const before_each_and_loaders_can_extend_context = {
parameters: { chromatic: { disable: true } },
loaders(context) {
Expand Down
Loading