storybookjs / storybook

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

[Bug]: controlled components work in a strange way in Docs when implemented by `useArgs` #25380

Open Key5n opened 9 months ago

Key5n commented 9 months ago

Describe the bug

See the video in this issue.

https://github.com/Key5n/storybook-useargs-bug/issues/1#issue-2059248804

When I click the first checkbox in Docs, changes take place in the checkbox. When I click the second checkbox, changes doesn't take place in the second checkbox, but in the first checkbox (moreover only once).

This bug happens only in Docs. I couldn't why this bug happens.

I show the case using useState for controlled component as well. The aim of this is to show you that this bug doesn't occur using useState in Docs.

How can I solve this bug?

If this bug has already been reported, I apologize.

To Reproduce

  1. clone this repo
  2. npm install
  3. npm run storybook
  4. move to docs in Checkbox component and click checkbox

System

$ npx storybook@latest info

Storybook Environment Info:

  System:
    OS: Linux 6.1 Manjaro Linux
    CPU: (16) x64 AMD Ryzen 7 5800X 8-Core Processor
    Shell: 5.9 - /usr/bin/zsh
  Binaries:
    Node: 18.18.0 - ~/.volta/tools/image/node/18.18.0/bin/node
    Yarn: 1.22.19 - ~/.volta/tools/image/yarn/1.22.19/bin/yarn
    npm: 9.8.1 - ~/.volta/tools/image/npm/9.8.1/bin/npm <----- active
    pnpm: 8.9.0 - ~/.volta/bin/pnpm
  npmPackages:
    @storybook/addon-essentials: ^7.6.6 => 7.6.6 
    @storybook/addon-interactions: ^7.6.6 => 7.6.6 
    @storybook/addon-links: ^7.6.6 => 7.6.6 
    @storybook/addon-onboarding: ^1.0.10 => 1.0.10 
    @storybook/addon-styling-webpack: ^0.0.5 => 0.0.5 
    @storybook/blocks: ^7.6.6 => 7.6.6 
    @storybook/nextjs: ^7.6.6 => 7.6.6 
    @storybook/preview-api: ^7.6.6 => 7.6.6 
    @storybook/react: ^7.6.6 => 7.6.6 
    @storybook/test: ^7.6.6 => 7.6.6 
    eslint-plugin-storybook: ^0.6.15 => 0.6.15 
    storybook: ^7.6.6 => 7.6.6 


### Additional context

I hit this bug when I follow this docs.
https://storybook.js.org/docs/writing-stories/args#setting-args-from-within-a-story
shilman commented 9 months ago

@Key5n thanks for filing. what's happening is that we have a kind of special case for the "primary" (aka first) story in the list. it shows up twice on the page, and the controls only control the top instance. the next instance always displays the initial args. the way we handle the primary story is a little hacky, but as a workaround, i'd recommend writing controlled stories with a useState hook instead. @tmeasday do you have any other suggestions?

Key5n commented 9 months ago

@shilman Thank you for replying. Although you suggest using useState, controlled component implemented by useState has a problem.

The problem is that component with useState cannot be controlled by Controls (I refer to the panel at the bottom of the screen) in component-detailed screen.

When I click the checkbox with useState, the value in Controls doesn’t change. This isn’t due to storybook, but the relationship between useState and React’s props. You can see this from my video as well.

If you know how to control components with useState and avoid this problem, let me know.

This problem doesn’t apply to components with useArgs, so I prefer useArgs.

tmeasday commented 8 months ago

OK, so it's clear that you want the controls state to sync with the component's state, which makes sense. So useArgs is the right thing to use.

I think probably what's off here is that you can affect the args in the second "static" version of the story, which doesn't respond to args changes. Perhaps we should add some behaviour to useArgs so that the update function is a no-op inside such stories. I don't really remember if that'd be easy or hard to do.

WDYT @JReinhold?

Alternatively we could give you a way of opting out of the "static"-ness of the second rendering of the story (or a way to remove it entirely).

JReinhold commented 8 months ago

Perhaps we should add some behaviour to useArgs so that the update function is a no-op inside such stories. I don't really remember if that'd be easy or hard to do.

Sorry I don't quite understand what you're suggesting here @tmeasday

@Key5n if you configure your own autodocs page by setting parameters.docs.page (globally in preview.js) you have two options:

1. Remove the primary story

use the includePrimary prop on the Stories block:

// Replace your-framework with the framework you are using (e.g., react, vue3)
import { Preview } from '@storybook/your-framework';

import { Title, Subtitle, Description, Primary, Controls, Stories } from '@storybook/blocks';

const preview: Preview = {
  parameters: {
    actions: { argTypesRegex: '^on[A-Z].*' },
    controls: {
      matchers: {
        color: /(background|color)$/i,
        date: /Date$/,
      },
    },
    docs: {
      page: () => (
        <>
          <Title />
          <Subtitle />
          <Description />
          <Primary />
          <Controls />
          <Stories includePrimary={false} /> 👈👈👈👈👈
        </>
      ),
    },
  },
};

export default preview;

2. Make the primary story dynamic

However if you do this, the story above the Controls panel will be in sync with the story below - this is why this is disabled by default.

It's similar to the first workaround except you need to implement your own Stories component. You can probably copy-paste the existing one from here, and then set the prop __forceInitialArgs={false}.

tmeasday commented 8 months ago

Sorry I don't quite understand what you're suggesting here @tmeasday

I'm suggesting that if you are rendering a story with forceInitialArgs set, then there is no point in the useArgs hook having an updateArgs callback that does anything, given the story will always render with the initial args. It'll just lead to weird behaviour like what we are seeing here.

JReinhold commented 8 months ago

I'm suggesting that if you are rendering a story with forceInitialArgs set, then there is no point in the useArgs hook having an updateArgs callback that does anything, given the story will always render with the initial args. It'll just lead to weird behaviour like what we are seeing here.

I see what you mean, so that would stop the primary, controllable story from reacting to changes in the static story.

kellyawang commented 5 months ago

I am running into a similar issue.

I noticed in the Docs view, the onChange event is actually being triggered with the new value (radio1), but seems like the re-render is not happening. Whereas in the individual story view, the onChange event is triggered with radio1, and the story is re-rendered.

This is my controlled story:

export const Controlled: Story = {
    args: {
        groupLabel: 'Radio Group Label',
        value: 'radio2',
    },
    render: function Render(args) {
        const [{ value }, updateArgs] = useArgs()
        function onChange(newValue: string) {
            // Call the provided callback
            // This is used for the Actions tab
            args.onChange?.(newValue)
            // Update the arg in Storybook
            updateArgs({ value: newValue })
        }
        // Forward all args and overwrite onChange
        return <RadioGroup {...args} value={value} onChange={onChange} />
    },
}

What would you recommend as a workaround to get the Story working on the Docs view?

tmeasday commented 5 months ago

@JReinhold @kasperpeulen -- this is interesting, a use case very relevant to other discussions we've been having.

@kellyawang - would it be possible to put together a minimal reproduction of the behaviour you are describing? https://storybook.js.org/docs/contribute/how-to-reproduce