DecoratorFunction Type and DecoratorFn

[cgatian]

While migrating from 6.3.1 => 6.4.1, I'm seeing some incompatible types, which look to be caused by migrations to use @storybook/csf types. Or I could be totally misunderstanding how this should work...

The Story type defined in @storybook/react contains a decorators property defined by csf in BaseAnnotations:

export declare type BaseAnnotations<TFramework extends AnyFramework = AnyFramework, TArgs = Args> = {
    decorators?: DecoratorFunction<TFramework, TArgs>[];

This story definition expects decorator functions to be defined with a specific framework as well as args defined.

But the existing DecoratorFn, from @storybook/react no longer satisfies this requirement. In order to get existing decorator functions to work, you need to modify the types used.

6.3.1:

import { DecoratorFn } from '@storybook/react';
import React from 'react';

export const cmpDecorator: DecoratorFn = (fn, context) => (
  <MyComponent>
    {fn(context)}
  </MyComponent>
);

6.4.1:

import { DecoratorFunction } from '@storybook/csf';
import { ReactFramework } from '@storybook/react';
import { StoryFnReactReturnType } from '@storybook/react/dist/ts3.9/client/preview/types';

export const cmpDecorator: DecoratorFunction<ReactFramework> = (
  fn,
  context,
): StoryFnReactReturnType => (
  <MyComponent>
    {fn(context)}
  </MyComponent>
);

Here's a simple example that replicates the problem:

import { DecoratorFn, Story } from '@storybook/react';

type myArgs = {test: string};

const myStory: Story<myArgs> = () => <div>Test</div>;

const myDecorator = (): DecoratorFn => (story, context) =>
  (
    <div>
      Yay!
      {story()}
    </div>
  );

myStory.decorators = [myDecorator()];  // Error: Property 'test' is missing in type 'Args' but required in type 'myArgs'.

[shilman]

@tmeasday @yannbf do we need to add this to MIGRATION.md somewhere?

[tmeasday]

@cgatian I am not quite following here, it is definitely possible we messed the types up so don't get me wrong! But I not 100% clear on what is going wrong here.

In your first example, '@storybook/react''s DecoratorFn = '@storybook/csf''s DecoratorFunction<ReturnTypeFramework<ReactFramework['storyResult']>>;

Which isn't exactly the same thing as DecoratorFunction<ReactFramework> but is pretty close! What is the issue you are seeing?

In your second example, the issue seems quite simply that you are applying a decorator to a story with a defined TArgs so your decorator should be scoped to the same TArgs. Is that not the way it worked before? We could probably relax the type of Story.decorators to allow any Args rather than the specific TArgs the Story is for..

We were trying not to make breaking changes here so if we have we should probably ease the types.

[tmeasday]

Right. So you can work around the problem by using DecoratorFunction<ReactFramework, myArgs> where DecoratorFunction is from @storybook/csf, and ReactFramework is from @storybook/react. But of course you'd need to make changes here which isn't ideal.

https://codesandbox.io/s/sb-decorator-types-forked-t3zuy?file=/src/index.tsx

I guess we should relax the types here to allow the old code to still work. That would mean making the Story.decorators type not be as strict as we might want it to do be. Thoughts @shilman @yannbf?

[shilman]

Is this something that can be handled in a codemod/automigration? If we feel like the new types are "right" I'd rather do the work to get existing users onto the new types smoothly, even if it's a breaking change. On the other hand, if the types are still in flux we should probably allow the old types and then figure out the migration path in 7.0.

[cgatian]

@tmeasday do you mind forking my codesandbox or pasting the code showing the solution?

[tmeasday]

@cgatian did my link above not work? codesandbox.io/s/sb-decorator-types-forked-t3zuy?file=/src/index.tsx

[cgatian]

We could probably relax the type of Story.decorators to allow any Args rather than the specific TArgs the Story is for

This would be ideal (for me 😄 ) and also match the previous behavior.

---

My pain to consider with the workaround approach:

• Sometimes the concrete Arg type is not collocated with the decorator because the decorator didn't care about any of the Typed Args.
• Add new import from csf and remove usage of DecoratorFn

Alternatively, we could use generic types and Story.decorator won't complain, but that doesn't seem ideal as you loose some information about this being a React Storybook Decorator.

export const cmpDecorator = (Story: React.ComponentType): JSX.Element => (
  <div><Story/></div>
);

[camillef]

I have run into the same issue on my project, where I've created a decorator generator function, that creates a decorator for wrapping a story with a react-router MemoryRouter with some supplied props. It looks something like

const getWithRouterDecorator = (
  routerProps?: MemoryRouterProps
): DecoratorFn => {
  const withRouter = (
    Story: PartialStoryFn<JSX.Element>,
    storyContext: StoryContext
  ): JSX.Element => (
    <MemoryRouter {...routerProps}>
      <Story {...storyContext} />
    </MemoryRouter>
  );
  return withRouter;
};

Upgrading to 6.4.0+ broke my typing: https://codesandbox.io/s/sb-withrouter-decorator-9d61j?file=/src/index.tsx

Following your suggestion, I'd have to introduce a generic to my getWithRouterDecorator and do something like

const getWithRouterDecorator = <ComponentProps extends Args,>(
  routerProps?: MemoryRouterProps
): DecoratorFunction<ReactFramework, ComponentProps> => {
  const withRouter = (
    Story: PartialStoryFn<JSX.Element>,
    storyContext: StoryContext
  ): JSX.Element => (
    <MemoryRouter {...routerProps}>
      <Story {...storyContext} />
    </MemoryRouter>
  );
  return withRouter;
};

but this still doesn't satisfy typing: https://codesandbox.io/s/sb-withrouter-decorator-forked-teblw?file=/src/index.tsx:251-640

I'm not sure how best to work around this, other than coercing the returned type, which I'd rather not do:

  return withRouter as DecoratorFunction<ReactFramework, ComponentProps>;