storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84.62k stars 9.31k forks source link

[Bug]: Vue3 Automatic inferenced argTypes don't work well #23079

Open a1mersnow opened 1 year ago

a1mersnow commented 1 year ago

Describe the bug

I'm using latest storybook whose verison is 7.0.20. I'm using vite+vue preset. I have a Component, and I wrote stories for it, but the automatic inferenced argTypes don't work well:

image

There are events, but I can't see any log in Actions Panel. In order to see logs, I need to manually write:

const meta: Meta<typeof Editor> = {
  component: Component,
  argTypes: {
    onReady: {},
  },
}

Now, there are logs, however, the Controls panel looks no ok:

image

I think, the auto inferenced argTypes are like this:

{
  ready: {
    table: {
      category: 'events'
    }
  }
}

Maybe, we can improve it to:

{
  onReady: {
    name: 'ready',
    table: {
      category: 'events'
    }
  }
}

so the parameters.actions.argTypesRegex: "^on[A-Z].*" can automatically catch all events.

To Reproduce

No response

System

No response

Additional context

No response

tfoxy commented 1 year ago

I created a custom extractArgTypes for the SB docs module that improves many of the types for my project, specially with TS. One of the improvements is setting events to the prop on{EventName}.

// docsParameters.ts
import {
  DocgenPropType,
  hasDocgen,
  extractComponentProps,
  convert,
} from '@storybook/docs-tools'
import type { InputType, SBType } from '@storybook/types'
import type { Prop } from 'vue'

type DocgenExtendedPropType = DocgenPropType & {
  elements?: DocgenExtendedPropType[]
  names?: [string]
}

const SECTIONS = ['props', 'events', 'slots', 'methods'] as const

function toEventName(name: string) {
  return `on${name.charAt(0).toUpperCase()}${name.slice(1)}`
}

function getUnionTypes(
  docgenType: DocgenExtendedPropType | undefined,
): string[] {
  if (docgenType?.name === 'Array')
    return [`${docgenType.elements?.map((v) => v.name).join('|')}[]`]
  return (
    docgenType?.elements?.flatMap((v) =>
      v.elements ? getUnionTypes(v) : [v.name],
    ) ?? []
  )
}

function isStringType(unionTypes: string[]) {
  return (
    unionTypes.length > 0 &&
    unionTypes.every(
      (t) => t === 'string' || (t.startsWith('"') && t.endsWith('"')),
    )
  )
}

/**
 * Same as Storybook `extractArgTypes` with the following changes:
 * - Remove control from events and methods.
 * - Add `on` prefix to events, so that `actions: { argTypesRegex: '^on[A-Z].*' }` can be used.
 * - Get event types from TS `defineEmits`. docgen info has it in `names` prop.
 * - Set types on `update:[prop]` events based on [prop] type
 * - Add all props not defined by docgen info in a group called "other props"
 * - Expand union type and use radio/select control when values are strings
 * - Expand array types
 * @see https://github.com/storybookjs/storybook/blob/d5ca2f42838c9f5a3e556a5e819e58f0deff522e/code/renderers/vue3/src/docs/extractArgTypes.ts
 */
export function extractArgTypes(component: any) {
  if (!hasDocgen(component)) return null
  const results: Record<string, InputType> = {}
  SECTIONS.forEach((section) => {
    extractComponentProps(component, section).forEach(
      ({ propDef, docgenInfo, jsDocTags }) => {
        const { name, type, description, defaultValue, required } = propDef,
          sbType = section === 'props' ? convert(docgenInfo) : { name: 'void' }
        const docgenType: DocgenExtendedPropType | undefined = docgenInfo.type
        const unionTypes = getUnionTypes(docgenType)
        const summary =
          unionTypes.join(' | ') || type?.summary || docgenType?.names?.[0]
        const inputType: InputType = {
          name,
          description,
          type: isStringType(unionTypes)
            ? { required, name: 'string' }
            : { required, ...sbType },
          table: {
            type: {
              ...type,
              summary,
            },
            jsDocTags,
            defaultValue,
            category: section,
          },
        }
        if (section === 'events') {
          inputType.control = null
          inputType.type = 'function'
          const propName = /^update:(.+)$/.exec(name)?.[1]
          const result = results[propName!]
          if (result) {
            inputType.table.type = result.table.type
          }
        } else if (isStringType(unionTypes)) {
          const options: (string | undefined)[] = unionTypes
            .map((t) => /^"(.+)"$/.exec(t)?.[1])
            .filter(Boolean)
          if (!required) options.unshift(undefined)
          inputType.options = options
          inputType.control = {
            type: options.length <= 5 ? 'radio' : 'select',
            labels: { undefined: '𝘢𝘯π˜₯𝘦𝘧π˜ͺ𝘯𝘦π˜₯' },
          }
        }
        const argName = section === 'events' ? toEventName(name) : name
        results[argName] = inputType
      },
    )
  })
  Object.entries(component.props as Prop<any>[]).forEach(([name, prop]) => {
    if (name in results) return
    const {
      default: defaultValue,
      required,
      type,
    } = typeof prop === 'function' || Array.isArray(prop)
      ? ({ type: prop } satisfies Prop<any>)
      : prop
    const sbType: SBType =
      type === String
        ? { name: 'string' }
        : type === Number
        ? { name: 'number' }
        : type === Boolean
        ? { name: 'boolean' }
        : type === Function
        ? { name: 'function' }
        : type === Object
        ? { name: 'object', value: {} }
        : type === Array
        ? {
            name: 'array',
            value: { name: 'other', value: 'Unknown' },
          }
        : { name: 'other', value: 'Unknown' }
    results[name] = {
      name,
      type: { required, ...sbType },
      table: {
        type: {
          summary: sbType.name === 'other' ? 'unknown' : sbType.name,
        },
        defaultValue,
        category: 'other props',
      },
    }
  })
  return results
}
// preview.ts
import { extractArgTypes } from './docsParameters'

export default {
  parameters: {
    docs: { extractArgTypes }
  }
}

Many of the changes here are going to be present/improved if https://github.com/storybookjs/storybook/pull/22285 is merged, but I believe this issue will not be resolved. Checking the extractArgTypes function:

https://github.com/storybookjs/storybook/blob/d07bfc520e0a03d597058e1321e64a20dc8222f3/code/renderers/vue/src/docs/extractArgTypes.ts#L63-L75

It still has the same name.

If you want to create PR to fix this, the change would be in

https://github.com/storybookjs/storybook/blob/next/code/renderers/vue/src/docs/extractArgTypes.ts#L74

const argName = section === 'events' ? `on${name.charAt(0).toUpperCase()}${name.slice(1)}` : name;
results[argName] = {
shengslogar commented 7 months ago

I wasn't surprised when I came across this with Storybook v7, but this is still the case in v8, with improved Vue support, and I'm confused as to how this was ever originally intended to work.

To restate the problem, given:

// Component.vue

<script setup lang="ts">
defineEmits(['someEvent']);
</script>
// Component.stories.ts

import Component from 'Component.vue'
import type { Meta, StoryObj } from '@storybook/vue3'

const meta = {
  component: Component
} satisfies Meta<typeof Component>

Then:

  1. Storybook will render someEvent under the "Events" panel image
  2. When the event is fired, Storybook will NOT log an action

Now, if we try and declare this event manually:

// Component.stories.ts

import Component from 'Component.vue'
import type { Meta, StoryObj } from '@storybook/vue3'
+++ import { fn } from '@storybook/test';
+++ import { action } from '@storybook/addon-actions';

const meta = {
  component: Component,
  +++ args: {
  +++   onSomeEvent: fn(), // This works
  +++   onSomeEvent: action('onSomeEvent'), // This works
  +++ },
  +++ argTypes: {
  +++   onSomeEvent: {
  +++     action: 'onSomeEvent', // This also works
  +++   }
  +++ }
} satisfies Meta<typeof Component>
  1. Each option, fn(), action, or declaring under argTypes, successfully fires an action (it's unclear to me from reading the docs which version is preferred)
  2. However, TypeScript will error out if we try and define a someEvent property; only onSomeEvent satisfies Meta<typeof Component>

So, it seems onSomeEvent is being correctly inferred, hence the type safety. If that's the case, then:

  1. Why is someEvent and not onSomeEvent being rendered to the "Events" panel?
  2. Why does argTypesRegex: '^on[A-Z].*' have no effect here? (see edit below)

If it weren't for this issue already being open, I would've thought I was making a horribly elementary mistake.

EDIT: after reading back through the original ticket, I'm realizing the issue I'm describing hasn't changed. argTypesRegex does work as expected. If I were to define argTypes: { onSomeEvent: { } } (as described in the original ticket), Storybook automatically assigns an action.

The only new issue my comment is raising, then, is that type safety expects events to be prefixed by on, while inference is discovering and assigning events without this prefix. Once inference is fixed, this will no longer be a problem.

kasperpeulen commented 7 months ago

@shengslogar Let me try to clear up some confusion. TLDR, you are right :)

Storybook works as a Vue h invocation, so for the following component:

<script setup lang="ts">
const props = defineProps({
  label: String,
  year: Number,
});
defineEmits(['someEvent']);
</script>
<template>
  <div @click="$emit('someEvent')">
    <slot :text="`Hello ${props.label} from the slot`" :year="props.year"></slot>
  </div>
</template>
const meta = {
  component: SomeComponent,
}

const Default = {
  args: {
    label: 'Some text',
    year: 2022,
    default: ({ text, year }) => `${text}, ${year}`,
    onSomeEvent: fn(),
  },
}

Under the hood just calls the h function:

h(meta.component, Default.args, filterSlotArgs(Default.args))

The convention to prefix events with on comes from Vue itself, see:

image

So I think you are right, we need to make sure that our docgen analysis prefixes the inferred argTypes with on if they are events. WDYT @larsrickert @chakAs3 ?

larsrickert commented 7 months ago

So I think you are right, we need to make sure that our docgen analysis prefixes the inferred argTypes with on if they are events. WDYT @larsrickert @chakAs3 ?

I am not sure about this since then the events are shown with the on prefix inside the table which is incorrect because when using the vue component, there is no on prefix:

<MyComponent @some-event />

So using @on-some-event is actually invalid. So the on prefix is only needed (if needed at all) internally in Storybook.

But yes I agree that this is confusing because the correct event name is "someEvent" but it needs to be defines as onSomeEvent. We have the same issue in our current project, there we created our own function to define events which will define the onSomeEvent as action but hide it inside the table so only someEvent is shown to our users.

I guess the main issue here would be solved if events would automatically be logged/defined as action so there is no need to define it argType, right?

rdnsr commented 2 weeks ago

Hello everyone. The assignment of the descriptor works because there are event fields in the object that is passed to the Args Table.


import { StoryObj, Meta } from '@storybook/vue3';
import AuthorizedUser, { AuthorizedUserProps } from './AuthorizedUser.vue';

const meta: Meta<typeof AuthorizedUser> =
{
    title: 'Layout/AuthorizedUser',
    component: AuthorizedUser,
    argTypes: {
        user: {
            description: "User object",
        },
        "profile-click": {
            description: "test description",
        }
    },
    tags: ['autodocs'],
    args: {
        user: {
            name: "Test user name",
            img: "/images/lk/avatar.jpg",
        }
    }
};
export default meta;

type AuthorizedUserStory = StoryObj<AuthorizedUserProps>;

export const Default: AuthorizedUserStory = {
    render: (args) => ({
        components: { AuthorizedUser },
        setup() {
            return { args };
        },
        template: `<AuthorizedUser v-bind="args"/>`,
    }),
};

Image

The problem is that the Meta type does not take them and swears at the lack of a field. It seems to me that the thing is that it takes only slots and props without emits. There is an inconsistency, they are drawn in the Args table component, but they cannot be configured.

//file node_modules\@storybook\vue3\dist\public-types-bcb3e860.d.ts
type Meta<TCmpOrArgs = Args> = ComponentAnnotations<VueRenderer, ComponentPropsOrProps<TCmpOrArgs>>;

type ComponentPropsAndSlots<C> = ComponentProps<C> & ExtractSlots<C>;
type ComponentPropsOrProps<TCmpOrArgs> = TCmpOrArgs extends Constructor<any> ? ComponentPropsAndSlots<TCmpOrArgs> : TCmpOrArgs extends FunctionalComponent<any> ? ComponentPropsAndSlots<TCmpOrArgs> : TCmpOrArgs;