[RFC] Explicit Actions Args Mocking

[kasperpeulen]

Status: accepted; championed by @kasperpeulen

🖼️ Overview

Outcome

We want a CSF story to render the same no matter if argTypes inference (which requires docgen provided by addon-docs) is available or not. Ideally, argTypes, even when explicitly specified, shouldn’t influence story rendering at all.

Purpose

1. This will make CSF files less magical and more portable, so that CSF files will render the same in a test environment where docgen is not available.
2. This allows users and (test) integrators to run or build storybook without docgen, boosting the user performance and allows tools to give quicker feedback.
3. This will make sure that we can one day lazy load docgen, without changing how stories are rendered.

📚 Context

When creating a new storybook with storybook init the following actions parameter is automatically set:

const preview: Preview = {
  parameters: { 
    actions: { argTypesRegex: "^on[A-Z].*" },
  },
};

export default preview;

This code will mark any argType as an action when it matches this regex.

Those story argTypes can be specified manually, but more often are inferred by analysing the source code in node, using libraries such as react-docgen-typescript.

When argTypes are considered actions by this regex, @storybook/addon-actions will “enhance” the user provided args so that an action function is provided for all the component props that start with onX (onClick, onKeyDown etc.)

This feature can be quite useful, the action function will automatically log those user events to the action panel:

Next to that, we enhance those actions args to be jest spies with @storybook/addon-interactions so that you can assert on events in the play function:

export const ClickTheButton = {
  play: async ({ args, canvasElement }) => {
    const canvas = within(canvasElement);
    await userEvent.click(canvas.getByRole('button'));
    await expect(args.onClick).toHaveBeenCalled();
  }
} satisfies Story;

The rendering problem

This feature is quite magical and implicit. Usually argTypes are not explicitly written down but inferred using docgen. If docgen is not enabled, it won’t work. For example, when you import a CSF file directly in a unit test with portable stories (using the composeStories function of the renderer).

Next to that, argTypes inference is one of our biggest performance bottleneck, as it often based on typescript which can be slow, and much slower than just building the stories itself.

We would like to get to a world where users can disable docgen in storybook and can even use CSF files outside of the storybook environment. There are two important scenario’s where this is not the case at the moment.

Required action args

If a story requires an action prop (e.g. onLoad) to render, it will break when inferred argTypes are not available.

export function Component(props: ComponentProps) {
  useEffect(() => {
    props.onLoad();
  });
  return <></>;
}

The play function

The play function is called during rendering the story, and might use action args:

export const ClickTheButton = {
  play: async ({ args, canvasElement }) => {
    const canvas = within(canvasElement);
    await userEvent.click(canvas.getByRole('button'));
    await expect(args.onClick).toHaveBeenCalled();
  }
} satisfies Story;

Testing integrations might want to build storybook to test if the play function assertions are correct. If those integrations can build storybook without docgen enabled, this will boost the performance of the feedback such an integration can give. However, this play function will now fail when docgen is disabled.

The TypeScript problem

Because of this magical regex based action arg enhancing, we need to somehow tell TypeScript that all action args are optional (as they will get a default value). But the argTypes inference works using a regex "^on[A-Z].*" that can be overridden by users, and it is not possible to make TS mark action args as optional based on this user provided regex.

At this moment, we mark any void function as optional. As event handlers are usual void functions, and if they are not, we won’t be able to mock them correctly anyway (as they require a return value). However, it still can be that a user writes an action arg like handleSubmit that is a void function, and will be marked as optional by TS, even though this action might not match the user provided regex.

This gives us the the following problems:

1. We might mark action args to eagerly as optional, even though they not match the user provided regex, and no default value will is assigned.
2. We mark action args as optional, even when argTypes inference is not enabled (portable stories, no addon-docs)
3. Another problem is that this very magical, hard to explain, and we got complaints from users that would like us not to manipulate their TS types in such a way: [Bug]: args types incorrectly make () => void properties optional #23068

💡 Proposed Solution

Explicit action args when needed for rendering

The user should explicitly provide required action args and action args used in the play function using a new package called @storybook/test:

import { fn } from '@storybook/test';

export const ClickTheButton = {
  args: {
    onClick: fn(),
  },
  play: async ({ args, canvasElement }) => {
    const canvas = within(canvasElement);
    await userEvent.click(canvas.getByRole('button'));
    await expect(args.onClick).toHaveBeenCalled();
  }
} satisfies Story;

Those explicit action args will be assigned a mock function that is compatible with both jest and vitest. This package should replace @storybook/jest and as start could be the same except for this renaming:

import { expect, jest } from '@storybook/jest';
import { expect, mock } from '@storybook/test';

The jest export only contained the stuff from jest-mock anyway, and ideally we should replace it with the vitest implementation, as vitest is ESM based and can be treeshaken. Also renaming the package from jest → test makes sure that people understand that the play function doesn’t actually requires a jest test runner in anyway. And that play function can be called in vitest tests as well.

In the future we want to use this package to contain more test specific functionalities, such as utilities to mock dates, timers, strings etc. Possibly even mocking modules?

Attach action logger to implementation when addon-actions is installed

The idea is that mock.fn is just a very generic mock function, and can be used in a test environment as well. But if used in storybook, and when the addon-actions is installed, it should trigger a channel event that logs to the action panel. We can use an arg enhancer for that:

export const attachActionsToFunctionMocks: ArgsEnhancer<Renderer> = (context) => {
  // this is mutating the mock instance, might be better to override the args instead
  // not sure about the consequences
  Object.entries(context.initialArgs).forEach(([key, value]) => {
    if (
      typeof value === 'function' &&
      'getMockImplementation' in value &&
      'mockImplementation' in value
    ) {
      const oldImplementation = value.getMockImplementation();
      value.mockImplementation((...args: unknown[]) => {
        action(key);
        return oldImplementation(...args);
      });
    }
  });
};

Deprecate implicit action args used during rendering

In a new minor release for Storybook 7, we will show a runtime deprecation warning whenever an action arg is being called when rendering the story, whenever the action arg is not explicitly provided , but implicitly by argTypes or argTypesRegex.

We include playing the play function as part of rendering the story in this definition. So when actions are being invoked by the play function, or when actions are being asserted, it will also show a runtime warning.

In Storybook 8, we will upgrade this warning to an actual error.

When such an “implicit” action (using argTypesRegex) is being called after rendering, it won’t throw an error. In this way the action panel should keep working as it is today.

Remove the TS magic

When an action arg is required, we currently do some TS magic to turn this action arg optional instead:

type ActionArgs<TArgs> = {
  // This can be read as: filter TArgs on functions where we can assign a void function to that function.
  // The docs addon argsEnhancers can only safely provide a default value for void functions.
  // Other kind of required functions should be provided by the user.
  [P in keyof TArgs as TArgs[P] extends (...args: any[]) => any
    ? ((...args: any[]) => void) extends TArgs[P]
      ? P
      : never
    : never]: TArgs[P];
};

However, with this proposal, we actually want to trigger people writing their required action args out, so that the story doesn’t break when argTypes inference is not available.

As this is a breaking change, we will delay this change to 8.0.

Remove argTypesRegex from sb init template

At this moment, we automatically set the argTypesRegex in .storybook/main.ts

We do think that this regex can be useful when you automatically want to log a zillion possible event handlers. Think about a fat UI component, such as found in MUI:

However, we don’t want to set this by default anymore, and make it an opt-in when users think it is useful in their specific scenario.

Risks

• The plan is to make sure the action function errors when implicitly provided and called during story rendering. This might be hard to get right, as it is quite stateful. What if arg are being updated, the story is replayed, etc.

Unresolved questions

• Should @storybook/test be in the monorepo, or be released independently

Alternatives considered / Abandoned Ideas

• Args mocking via explicit argTypes

🚀 Rollout Plan

How do we plan to release these deliverables to customers? Should we opt for incremental release? Should the feature be hidden behind a feature flag? How are we planning to monitor and alert on any issues related to this project?

In a 7.x release

• Release the @storybook/test package for explicit action args
• Runtime warning when action args generated from docgen are used in story rendering or the play function
• Remove argTypesRegex from sb init

In the 8.0 release

• Runtime error when action args generated from docgen are used in story rendering or the play function.
• Remove the TS magic of making required action args optional

[kasperpeulen]

I have been working on implementing this in this PR: #23888
You can try it out in this canary release: 0.0.0-pr-23888-sha-032c0a61.

It basically re-exports an instrumented version of @vitest/spy and @vitest/expect flavoured chai.
While @storybook/jest was based on jest-mock and expect, which both are from the jest monorepo.

Using @vitest/spy over jest-mock seems like an easy choice, as @vitest/spy is compatible with it, but is ESM based, browser friendly and seems therefore like a more modern version that fits well with storybook. For example, we had issues upgrading jest-mock as it can not tree-shaken to exclude node-specific modules:
#23570

However, I'm not entirely sure about using chai (extended by @vitest/expect in the same way as vitest does that). I will list some pros and cons:

Pros of using chai

• Will be tree shakable when chai 5 goes out
• Also smaller in total size, even though the public API of chai is much bigger
• Many people seem to want this: Supporting Vitest #17326. Allthough I'm not sure if this issue is really about using chai over expect. Would love feedback here!
• expect depends on more node specific stuff, which we have to patch to make it browser compatible (https://github.com/storybookjs/expect/blob/main/patches/jest-util%2B28.1.3.patch)
• The types of jest are in a global namespace, and live in @types/jest which can give unexpected issues, and sometimes hard to scope.

Bundlephobia chai (https://bundlephobia.com/package/chai@4.3.7)

Bundlephobia expect (https://bundlephobia.com/package/expect@29.6.4)

Cons of using chai

• The assertion messages of expect seem better and more extensive than chai
  Chai

Jest

• chai has a big public API, which might be confusing as it gives many different ways to do the same thing. Therefore, I have limited the TS definitions in the PR to only include the jest compatible API.

[IanVS]

Is it perhaps possible to give users a choice of assertion library? That was always once nice thing about test-runners that were decoupled from an assertion library (i.e. mocha). For instance, for those who care a lot about their assertion messages, they could continue using @storybook/jest, but by default @storybook/test might use chai?

[kasperpeulen]

That sounds reasonable to me. cc @yannbf
What are the biggest reasons you would prefer @storybook/jest @IanVS

I think all the information that we need to give a good diff is available:

If we process the error we receive with processError from @vitest/utils.
https://github.com/vitest-dev/vitest/blob/eac7776521bcf4e335771b1ab4f823f40ad9c4ff/packages/utils/src/error.ts#L89

[IanVS]

What are the biggest reasons you would prefer @storybook/jest

Just the assertion messages.

If we process the error we receive with processError

Does that mean creating our own assertion messages? It sounds like potentially a lot of work to me. I wonder if the output from vitest could be improved upstream instead.

[kasperpeulen]

@IanVS So vitest is also enriching the error from chai with processError from @vitest/utils. So we don't need to anything else, then just using the @vitest/utils package, which is not a lot of work.

[kasperpeulen]

Implemented it now:

See:
fb84613

[IanVS]

I have another TypeScript issue with automocked args, and I wonder if this proposal might also help to solve it as well.

Currently, TypeScript does not realize that such automocked args like onClick are actually mocks, so if I try to args.onClick.mockReset() in a play function, I get a TypeScript error. I wonder if all such args need to be explicitly defined, perhaps the types could be improved so that in the play function, they are typed as mock functions. I think technically this could be done today, if I define the arg in my args object, but it might be an even more important thing if nobody is relying on implicit mocking in the future.

[IanVS]

Oh and this is a bit more off-topic, but while we're at it we might consider calling mockReset() for such args automatically at the start of each render. Otherwise, HMR or hitting the "reload story" button can cause calls to leak over, and make interaction tests fail.

[kasperpeulen]

Currently, TypeScript does not realize that such automocked args like onClick are actually mocks, so if I try to args.onClick.mockReset() in a play function, I get a TypeScript error.

I have an idea for that! But not yet implemented. And it will probably only work when those args are set on meta (or if we ever would switch to factories).

Oh and this is a bit more off-topic, but while we're at it we might consider calling mockReset() for such args automatically at the start of each render.

Yes, I'm doing that!

storybook/code/lib/test/src/index.ts

Lines 10 to 16 in 2b9981a

	const channel = addons.getChannel();
	channel.on(FORCE_REMOUNT, () => spy.spies.forEach((mock) => mock.mockClear()));
	channel.on(STORY_RENDER_PHASE_CHANGED, ({ newPhase }) => {
	if (newPhase === 'loading') spy.spies.forEach((mock) => mock.mockClear());
	});

[IanVS]

And it will probably only work when those args are set on meta

You mean using the proposed solution of this RFC, right? I figured that the change to make them explicit might help unlock correct TypeScript typings, and it sounds like that's the case, from what you're saying. 🙌 I think that's another reason this RFC is a good idea.

[kasperpeulen]

Yes, correct, will only work with this RFC, not with the implicit args!

[kasperpeulen]

Got something working!

[yannbf]

Hey everyone! Just wanted to let you all know that this RFC was accepted, implemented and released in the prerelease of Storybook 7.6.0.

You can read the blog post here: https://storybook.js.org/blog/storybook-test/
If you have feedback, you can write issues on Github! Thank you!

[aaronadamsCA]

This is great news, thank you!