storybookjs / storybook

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

[Bug]: In Vue 3 JSX or `h` renderer Controls won't cause re-render and update of the component #23437

Open floroz opened 1 year ago

floroz commented 1 year ago

Describe the bug

Setup

import type { Meta, StoryObj } from '@storybook/vue3';

import Button from './Button.vue';
import { h } from 'vue';

// More on how to set up stories at: https://storybook.js.org/docs/vue/writing-stories/introduction
const meta = {
  title: 'Example/Button',
  component: Button,
  // This component will have an automatically generated docsPage entry: https://storybook.js.org/docs/vue/writing-docs/autodocs
  tags: ['autodocs'],
  argTypes: {
    size: { control: 'select', options: ['small', 'medium', 'large'] },
    backgroundColor: { control: 'color' },
    onClick: { action: 'clicked' },
  },
  args: { primary: false, label: "Click me" }, // default value
} satisfies Meta<typeof Button>;

export default meta;
type Story = StoryObj<typeof meta>;

Default render with template - WORKS

export const Primary: Story = {
  args: {
    primary: true,
    label: 'Button',
  },
  render: (args) => ({
    components: { Button },
    setup() {
      return { args };
    },
    template: '<Button v-bind="args" />',
  })
};

Render with JSX - DOESN'T WORKS

export const Large: Story = {
  args: {
    label: 'Button',
    size: 'large',
  },
  // @ts-expect-error
  render: (args) => <Button {...args} />,
};

Render withh renderer - DOESN'T WORKS

export const Secondary: Story = {
  args: {
    primary: false,
    label: 'Button',
  },
  render: (args) => h(Button, args),
};

To Reproduce

https://github.com/floroz/storybook-vue3-renderer-bug/blob/main/src/stories/Button.stories.tsx

System

System:
    OS: macOS 13.2
    CPU: (8) arm64 Apple M1 Pro
  Binaries:
    Node: 18.16.0 - ~/.nvm/versions/node/v18.16.0/bin/node
    npm: 9.5.1 - ~/.nvm/versions/node/v18.16.0/bin/npm
  Browsers:
    Chrome: 114.0.5735.198
    Edge: 114.0.1823.51
    Safari: 16.3
  npmPackages:
    @storybook/addon-essentials: ^7.0.27 => 7.0.27 
    @storybook/addon-interactions: ^7.0.27 => 7.0.27 
    @storybook/addon-links: ^7.0.27 => 7.0.27 
    @storybook/blocks: ^7.0.27 => 7.0.27 
    @storybook/testing-library: ^0.0.14-next.2 => 0.0.14-next.2 
    @storybook/vue3: ^7.0.27 => 7.0.27 
    @storybook/vue3-vite: ^7.0.27 => 7.0.27

Additional context

No response

floroz commented 1 year ago

So it seems that if I return the h function or JSX from within the setup function, the component updates correctly.

export const Secondary: Story = {
  args: {
    primary: false,
    label: 'Button',
  },
- render: (args) => h(Button, args),
+ render: (args) => ({
+ setup() {
+ return () => h(Button,args)
}

This is consistent with Vue Docs explanation of Render Functions.

Still not sure why the component was rendered in the first place.

The fact that it renders, but doesn't update, is that a Storybook bug in the way the Canvas is re-rendered then?

floroz commented 1 year ago

"Re-opening" this since the workaround breaks the @storybook/addon-docs.

When using the setup function inside the render,

  render: (args) => ({
    setup() {
      return () => <BTag {...args} />;
    },
  }),

the autodocs won't pick up the correct representation of the source code, and the undefined is probably the anonymous function used in the setup return.

Screenshot 2023-07-25 at 16 58 39

However, if I inline the JSX directly in the return of render

render: (args) => <BTag {...args} />,

The component is analysed correctly:

Screenshot 2023-07-25 at 17 03 15

but any change to args via controls won't trigger a re-render of the canvas

@chakAs3 do you know if this is something related to the vue-component-meta changes you are working on?

chakAs3 commented 1 year ago

"Re-opening" this since the workaround breaks the @storybook/addon-docs.

When using the setup function inside the render,

  render: (args) => ({
    setup() {
      return () => <BTag {...args} />;
    },
  }),

the autodocs won't pick up the correct representation of the source code, and the undefined is probably the anonymous function used in the setup return.

Screenshot 2023-07-25 at 16 58 39

However, if I inline the JSX directly in the return of render

render: (args) => <BTag {...args} />,

The component is analysed correctly:

Screenshot 2023-07-25 at 17 03 15

but any change to args via controls won't trigger a re-render of the canvas

@chakAs3 do you know if this is something related to the vue-component-meta changes you are working on?

this not related to this PR, i'm addressing this in another PR related to rendering implementation, however your first case i can get fix .

floroz commented 1 year ago

"Re-opening" this since the workaround breaks the @storybook/addon-docs. When using the setup function inside the render,

  render: (args) => ({
    setup() {
      return () => <BTag {...args} />;
    },
  }),

the autodocs won't pick up the correct representation of the source code, and the undefined is probably the anonymous function used in the setup return. Screenshot 2023-07-25 at 16 58 39 However, if I inline the JSX directly in the return of render

render: (args) => <BTag {...args} />,

The component is analysed correctly: Screenshot 2023-07-25 at 17 03 15 but any change to args via controls won't trigger a re-render of the canvas @chakAs3 do you know if this is something related to the vue-component-meta changes you are working on?

this not related to this PR, i'm addressing this in another PR related to rendering implementation, however your first case i can get fix .

That's great, thank you for your work on this!

When you open that PR tag me in, I can use our project to do some QA on the canary release and verify if it resolves that issue.

chakAs3 commented 1 year ago

That's great, thank you for your work on this!

It is my pleasure, please don't hesitate to ask me for an explanation, and for now can you use this repo and your template https://stackblitz.com/~/github.com/storybook-vue/storybook-vue-custom-render/pull/2.

some features are not yet merged to main or next branch, here you can test and let me know

floroz commented 1 year ago

That's great, thank you for your work on this!

It is my pleasure, please don't hesitate to ask me for an explanation, and for now can you use this repo and your template https://stackblitz.com/~/github.com/storybook-vue/storybook-vue-custom-render/pull/2.

some features are not yet merged to main or next branch, here you can test and let me know

Thank you @chakAs3.

I cloned your repo and reproduced the error:

Using render function to directly return JSX

The JSX syntax returned directly via render function

export const JSXSyntax: Story = {
  args: {
    label: 'Rendered Using JSX Syntax',
    size: 'small',
  },
  render(args) {
    return <JSXComponent {...args} />;
  }
};

is picked correctly by auto-docs

Screenshot 2023-07-30 at 11 09 08

But, if I try to use the controls to update the value of label I don't see any change unless I manually refresh the Story:

https://github.com/storybookjs/storybook/assets/45793869/9ec917d8-f387-4dbe-bb54-9dd7e92b3a0a

Returning a wrapper component in the render function to keep reactivity with args

If I return a component from the render function, the reactivity is kept

export const JSXSyntax: Story = {
  args: {
    label: ' Label from Story Args passed to JSXComponent',
  },
  render(args:Args) {
    return {
      setup() {
        return () => <JSXComponent {...args} />
      }
    };
  },
}

But now the Docs are broken:

Screenshot 2023-07-30 at 11 12 02

https://github.com/storybookjs/storybook/assets/45793869/549d5cc0-0c13-4ec1-a3d1-852c68e96e03

floroz commented 1 year ago

(Partial) Workaround

To partially restore the docs, you can pass the Component.name within the anonymous Vue Component to recreate the same docs

export const JSXSyntax: Story = {
  args: {
    label: ' Label from Story Args passed to JSXComponent',
  },
  render(args:Args) {
    return {
+      name: JSXComponent.name,
      setup() {
        return () => <JSXComponent {...args} />
      }
    };
  },
};

And now we've got both args reactivity and the correct docs being displayed.

https://github.com/storybookjs/storybook/assets/45793869/1cb1bf46-7a52-4f9f-980c-2388638e127b

Gotchas

However, this only works for components where all args are passed directly onto it. If we have a composed component, like an Accordion, this structure would still break the docs.

Composable Components are not inferred

Creating Stories of composable components does not render the correct output

In the example below, we need to use two components together, Radio and RadioGroup, and we need the setup function to setup some local state to make the Story interactive.

const meta: Meta<typeof BRadioGroup> = {
  title: "Components/Radio/RadioGroup",
  component: BRadioGroup,
  tags: ["autodocs"],
  argTypes: {
    colorScheme: {
      options: ["white", "black"],
      default: "black",
    },
  },
  args: {
    colorScheme: "black",
  },
  render: (args) => ({
    setup() {
      const model = ref("");
      return () => (
        <BRadioGroup
          {...args}
          modelValue={args.modelValue ?? model.value}
          onUpdate:modelValue={(v) => (model.value = v as string)}>
          <div style="display:flex; flex-direction: column; gap: 22px; align-items: flex-start;">
    {options.map((opt) => (
      <BRadio label={opt.label} value={opt.value} />
    ))}
  </div>
        </BRadioGroup>
      );
    },
  }),
};

Docs result: Screenshot 2023-07-30 at 12 46 53

If I transform the syntax to avoid a wrapper component and return JSX directly from the render function

render: (args) => {
    const model = ref("");
    return (
      <BRadioGroup
        {...args}
        modelValue={args.modelValue ?? model.value}
        onUpdate:modelValue={(v) => (model.value = v as string)}>
        {{
          default: () =>
            options.map((opt) => (
              <BRadio label={opt.label} value={opt.value} />
            )),
        }}
      </BRadioGroup>
    );
  },

I am still unable to see what is rendered in the default slot

Screenshot 2023-07-30 at 12 52 24

Restoring string template fixes addon-docs

If I revert to use the string template I see the results correctly:

render: (args) => {
    return {
      components: {
        BRadioGroup,
        BRadio,
      },
      setup() {
        const modelValue = ref([]);
        return { args, modelValue };
      },
      template: `
        <BRadioGroup v-bind="args" v-model="modelValue">
          <BRadio label="Option 1" value="option1" />
        </BRadioGroup>
      `,
    };
  },

Screenshot 2023-07-30 at 12 58 57

floroz commented 1 year ago

@chakAs3 hopefully the investigation above can be useful.

Using Vue string templates is a terrible experience for larger projects in terms of maintainability, and a sub-par experience when compared to using JSX in React Stories.

Hopefully there can be a fix soon to these issues and new documentation for teams using Vue who want to choose Storybook for their Components while keeping support for TypeScript and the ability to run auto-refactor without fiddling with string interpolations and find and replace operations.

chakAs3 commented 1 year ago

@chakAs3 hopefully the investigation above can be useful.

Using Vue string templates is a terrible experience for larger projects in terms of maintainability, and a sub-par experience when compared to using JSX in React Stories.

Hopefully there can be a fix soon to these issues and new documentation for teams using Vue who want to choose Storybook for their Components while keeping support for TypeScript and the ability to run auto-refactor without fiddling with string interpolations and find and replace operations.

Great thanks for the detailed investigation, for the component name the source snippet the Volar PR will fixe it. I'm also looking into other points, no worries there is always a fix, I will get back to you soon

vhoyer commented 6 months ago

I am still unable to see what is rendered in the default slot

in fact, we are unable to see any slot