storybookjs / storybook

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

[Bug]: Control for mapped argType doesn't map initial arg on story render #22809

Open Sidnioulz opened 1 year ago

Sidnioulz commented 1 year ago

Describe the bug

On Storybook 7 and Vue 3, when using a mapped argType and passing a value to the corresponding arg, the control for the argType renders as if no value was selected.

In other words, the initial arg value is not mapped by the control when it renders, so the control doesn't display the initial arg value as selected.

I deduced that the bug is the control init code not applying the mapping, because if I create a mapping where the ID and label match, then the control properly selects the value.

To Reproduce

Steps to reproduce the behaviour:

Code snippets

const Button = {
  props: ['variant'],
  template: '{{variant}}',
}

export default {
  argTypes: {
    variant: {
    control: {
        labels: {
            Primary: "Primary",
            Secondary: "Secondary",
        },
        type: "inline-radio"
    },
    mapping: {
        Primary: "primary",
        Secondary: "secondary",
    },
    options: [
        "Primary",
        "Secondary",
    ],
  },
  component: Button,
}

export const Foo = {
  args: {
    variant: "primary", // matches the mapped value!]
  },
}

System

Environment Info:

  System:
    OS: Linux 6.2 Arch Linux
    CPU: (8) x64 11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz
  Binaries:
    Node: 16.20.0 - /usr/bin/node
    Yarn: 1.22.19 - /usr/bin/yarn
    npm: 8.19.4 - ~/Development/design-system/node_modules/.bin/npm
  Browsers:
    Firefox: 111.0.1
  npmPackages:
    @storybook/addon-a11y: ^7.0.7 => 7.0.15 
    @storybook/addon-actions: ^7.0.7 => 7.0.15 
    @storybook/addon-essentials: ^7.0.7 => 7.0.15 
    @storybook/addon-interactions: ^7.0.8 => 7.0.15 
    @storybook/addon-links: ^7.0.7 => 7.0.15 
    @storybook/addon-measure: ^7.0.7 => 7.0.15 
    @storybook/addon-storysource: ^7.0.7 => 7.0.15 
    @storybook/addon-viewport: ^7.0.7 => 7.0.15 
    @storybook/addons: ^7.0.7 => 7.0.15 
    @storybook/core-server: ^7.0.7 => 7.0.15 
    @storybook/jest: ^0.1.0 => 0.1.0 
    @storybook/manager-api: ^7.0.11 => 7.0.15 
    @storybook/testing-library: ^0.1.0 => 0.1.0 
    @storybook/theming: ^7.0.11 => 7.0.15 
    @storybook/vue3: ^7.0.7 => 7.0.15 
    @storybook/vue3-vite: ^7.0.7 => 7.0.15

Additional context

No response

neptunus commented 1 year ago

I've noticed the same thing. Something interesting (and maybe helpful for finding a solution to this bug) is that clicking the Reset controls button actually re-renders the controls panel correctly—at least for me 😅

image

Edit: We're using React 7—more info below

Environment Info
  System:
    OS: macOS 12.4
    CPU: (10) arm64 Apple M1 Pro
  Binaries:
    Node: 16.17.0 - ~/.nvm/versions/node/v16.17.0/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v16.17.0/bin/yarn
    npm: 8.15.0 - ~/.nvm/versions/node/v16.17.0/bin/npm
  Browsers:
    Chrome: 113.0.5672.126
    Edge: 113.0.1774.57
    Firefox: 112.0.1
    Safari: 15.5
  npmPackages:
    @storybook/addon-a11y: 7.0.12 => 7.0.12 
    @storybook/addon-essentials: 7.0.12 => 7.0.12 
    @storybook/addon-interactions: ^7.0.12 => 7.0.12 
    @storybook/addon-links: ^7.0.12 => 7.0.12 
    @storybook/core-common: ^7.0.12 => 7.0.12 
    @storybook/core-server: 7.0.12 => 7.0.12 
    @storybook/nextjs: 7.0.12 => 7.0.12 
    @storybook/react: 7.0.12 => 7.0.12 
    @storybook/react-vite: 7.0.12 => 7.0.12 

Video https://github.com/storybookjs/storybook/assets/4912865/26156143-1a1f-4033-9eae-fce876233e42
Sidnioulz commented 1 year ago

@neptunus interesting! Here it does not. The control gets rendered without a selection once again. I am running SB 7.0.15 with the Vue 3 x Vite framework. It might help the maintainers if you also tell which framework you tested with!

shilman commented 1 year ago

@Sidnioulz @neptunus Can you please try the latest 7.0.18? We've recently shipped a bunch of arg-related fixes in Vue3 and it's possible that the problem has been fixed by those. If not, we'll need to add more tests and get to the bottom of it!

Sidnioulz commented 1 year ago

@shilman we've had other bugs fixed by the update, but not this one. I can still see that the control is not initially mapped, and not mapped on reset.

Sidnioulz commented 1 year ago

I think I now understand what happens. Let's assume we have a select based on this enum:

enum ExampleProp {
  Foo: 1
  Bar: 2
}

The story arg is passed exactly as is to ArgsTable, then ArgRow, ArgControl, etc.

If I pass the raw value (1) as a story arg

If I pass the ID instead (Foo) as a story arg

This appears to be working exactly as it's supposed to, judging by the code, but it was surprising.

I now believe the doc at https://storybook.js.org/docs/react/writing-stories/args#mapping-to-complex-arg-values could be improved with a sample StoryObj that has the expected format for the arg, making it clearer that args should use the mapping key and not the mapping value.

@shilman if you agree with this diagnosis, I'm happy to provide a PR.

csdoneil commented 5 months ago

So, while this works for JS, it is problematic for TS environments, where the value for that property is supposed to be an enum, for instance. If I put the Key of the enum, I get TS errors, but if I put that value of the enum, I don't end up having the intial value selected in the select list.

psychobolt commented 4 months ago

I am creating a add-on, which I need to custom args since values are could be non-JSON serializable objects:

// meta
storyAttr: {
  control: 'select',
    options: [
      'none',
      ...customStateAttrOptions // keys are strings
    ],
},
mapping: {
  'none': defaultStateAttributes, // values are objects
  ...customStateAttributes
}

// story
export const Primary: StoryObj<Args> = {
  args: {
    storyAttr: 'none'
  }
};

As you see here, I have no option but to use the IDs (e.g. none) as the initial arg value. Pointing out I have the same issue using mappings for custom args in my Lit + TS setup even with this setup: Story is not initially mapped to 'none'. I can verify that clicking on the reset button will re-render correctly as mentioned.

Just my 2 cents for TS devs. I believe there is a important distinction: Story (CSF) Args is not a 1 to 1 utility type mapping to your Component or Renderer args. Best to not pass these types as your Component or Renderer args/props. Although most do it by default or simplicity if the values are simple to serialize, which can be a mistake with setting up CSF files.

I have distinction for my types:

// only used in CSF
export interface StoryPseudoStateArgs<A = typeof DefaultStateAttrEnum> extends Args {
  storyAttr: 'none' | keyof A;
}

// only used in Component or Renderer
export interface StoryPseudoStateProps { 
  storyAttr?: Record<string, string | boolean>;
}