[Bug]: Args applied also to html/decorators, not only to the component itself

[Radouch]

Describe the bug

I am using Storyboook 7 beta for my Vue 3 component library. The story is written in CSF 3 format. My component is CgsNumLocaleInput and my story looks like:

export const Test = {
  render: (args) => ({
    components: { CgsNumLocaleInput },
    data() {
      return { firstValue: 13};
    },
    setup() {
      return { args };
    },
    template: `
<CgsNumLocaleInput v-bind="args" v-model="firstValue"></CgsNumLocaleInput>
    `,
  }),
  decorators: [
    () => ({
      template: `
      <div class="input-group">
      <span class="input-group-text">firstValue</span>
      <story />
      </div>
    `,
    }),
  ],
  args: {
    min: 1,
    max: 20,
    step: 1,
    class: "form-control"
  }
};

I need to apply args to the component CgsNumLocaleInput only. Unfortunatelly, generated code looks like:

<div id="storybook-root" data-v-app="">
<!-- HERE IS THE PROBLEM: args are also applied on the following div -->
  <div class="input-group form-control" min="1" max="20" step="1">
<!--
Should be: <div class="input-group">
 -->
    <span class="input-group-text">firstValue</span
    >
  <!-- Begin of code of the component CgsNumLocaleInput. Args are applied here, too, as intended -->
   <input
      type="text"
      min="1"
      max="20"
      step="1"
      class="form-control bg-danger"
      readonly=""
    /><input
      type="number"
      min="1"
      max="20"
      step="1"
      class="form-control bg-danger"
      style="display: none"
    />
  <!-- End of code of the component CgsNumLocaleInput -->
  </div>
</div>

How could I prevent applying args to html code in decorators? The same problem occurs if I use html directly in the story template (without decorators).

To Reproduce

No response

System

System:
    OS: Windows 10 10.0.19044
    CPU: (8) x64 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz
  Binaries:
    Node: 16.17.0 - C:\Program Files\nodejs\node.EXE
    npm: 8.19.2 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.19041.1266.0), Chromium (110.0.1587.57)
  npmPackages:
    @storybook/addon-essentials: ^7.0.0-beta.4 => 7.0.0-beta.60
    @storybook/addon-interactions: ^7.0.0-beta.4 => 7.0.0-beta.60
    @storybook/addon-links: ^7.0.0-beta.4 => 7.0.0-beta.60
    @storybook/blocks: ^7.0.0-beta.4 => 7.0.0-beta.60
    @storybook/testing-library: ^0.0.14-next.0 => 0.0.14-next.1
    @storybook/vue3: ^7.0.0-beta.4 => 7.0.0-beta.60
    @storybook/vue3-vite: ^7.0.0-beta.4 => 7.0.0-beta.60

Additional context

No response

[Radouch]

It seems there is workaround: args are applied only to the single wrapping element for the whole story.

So you can use e.g.:

 decorators: [
    () => ({
      template: `
      <h3>This heading is a workaround: no args are applied to the following div</h3>
      <div class="input-group">
      <span class="input-group-text">firstValue</span>
      <story />
      </div>
    `,
    }),

and args are applied to the component only, as is desired. Anyway, this behaviour still looks like a bug.

[bodograumann]

We are having the same problem. This causes breaking tests, because we add e.g. placeholder text and select the DOM Node by it in our play function with getByPlaceholderText. This fails, because now there are two nodes with this exact text.

Will now just add another sibling node to the top-level to workaround the problem, but it is definitely a bug.

[dryton]

I'm also running into this issue with testing scenarios, it can create duplicate IDs, duplicate attributes, invalid HTML.

[bodograumann]

Encountered the bug again, this time with a class attribute. So the problem only became apparent in our chromatic tests.
For the existing stories we did catch it, but new stories might show the components incorrectly.
Not to mention that colleagues who are not aware of the issue and possible workaround will probably get confused and waste time.

Any chance of a fix, @shilman ?
I could try to find some time and look into it myself otherwise.

[Sidnioulz]

Simple solution to this:

function withRootClass(story, context) {
  return {
    components: { story },
    inheritAttrs: false,
    template: `<span class="theme-whatever"><story v-bind="$attrs" /></span>`,
  }
}

I don't think it would be fixable under the hood, as there's hardly a way to know where the original root of the component is located in the template being given to the story renderer. The documentation could be updated to include inheritAttrs by default. Thoughts?

[bodograumann]

I don't understand why inheritAttrs: false should be necessary now, when it was working fine before.

[bodograumann]

Looks like there might be a fix in 7.0.18. Will have to check.
Cf.

• Nice write-up: Vue3: Rollback v7 breaking change and keep reactive v6-compatible API #22229
• Backport: Vue3: Revert v7 breaking change, restore reactive v6-compat API #22692
• Release: https://github.com/storybookjs/storybook/releases/tag/v7.0.18

[chakAs3]

Simple solution to this:

function withRootClass(story, context) {
  return {
    components: { story },
    inheritAttrs: false,
    template: `<span class="theme-whatever"><story v-bind="$attrs" /></span>`,
  }
}

I don't think it would be fixable under the hood, as there's hardly a way to know where the original root of the component is located in the template being given to the story renderer. The documentation could be updated to include inheritAttrs by default. Thoughts?

that was my direct simple solution if you really want to make things right, Vue has his great API and behaviour that should. be respected, I felt no one was really there from the community. Please check this #22241. we could avoid all these bugs because only by doing the right things will have a solid maintainable codebase.

Storybook has an awesome API if we really understand this, the custom render function should return a Vue component ( whether composition api, options API, or functional component which is responsible for the rendering so this should behave.
like the real vue component.

I always believe that Storybook should empower Vue Developers and let them use their unique and smart framework and not limit them and subtract and change behaviour, But for that, they need Feedback from Vue users
so @Sidnioulz can vote for my proposition if you would like to have vue API which is natural

[chakAs3]

Encountered the bug again, this time with a class attribute. So the problem only became apparent in our chromatic tests. For the existing stories we did catch it, but new stories might show the components incorrectly. Not to mention that colleagues who are not aware of the issue and possible workaround will probably get confused and waste time.

Any chance of a fix, @shilman ? I could try to find some time and look into it myself otherwise.

I don't understand why inheritAttrs: false should be necessary now, when it was working fine before.

Hi @bodograumann we understand that you used the same behaviour, But in the first place, we have to match the Vue API and behaviour, we should start by understanding how Vue works as it can be very confusing, our reference should be Vue, end of the day you are developing UI using Vue renderer. it is true that was not well documented in v6 as even the implementation lacks reactivity and was very limited, Here there is a PR #22241 that can solve the problem easily by enabling/disabling inheritAttrs globally, in story or component, most important is not to block the developer he would like to use this API, especially that fallthrough attributes is one of the great and unique feature that distinguishs Vue as the smart framework.

[chakAs3]

Encountered the bug again, this time with a class attribute. So the problem only became apparent in our chromatic tests. For the existing stories we did catch it, but new stories might show the components incorrectly. Not to mention that colleagues who are not aware of the issue and possible workaround will probably get confused and waste time.
Any chance of a fix, @shilman ? I could try to find some time and look into it myself otherwise.

I don't understand why inheritAttrs: false should be necessary now when it was working fine before.

Hi @bodograumann we understand that you used this behaviour, But in the first place, we have to match the Vue API and behaviour, we should start by understanding how Vue works as it can be very confusing, our reference should be Vue, end of the day you are developing UI using Vue renderer. it is true that was not well documented in v6 as even the implementation lacks reactivity and was very limited which creates an illusion that Storybook dictates how to write your custom render using a setup function, but the reality and with v7 rendering is delegated to UI framework Vue or react so you can use any component, the behaviour guaranteed by the renderer and not storybook.., Here there is a PR #22241 that can solve the problem easily by enabling/disabling inheritAttrs globally, in story or component, most important is not to block the developer he would like to use this API, especially that fallthrough attributes is one of the great and unique features that distinguishes Vue as the smart framework.
with this PR we could

• To satisfy your need and the need of devs that have a large codebase that can be buggy after migration and need complex refractory, I have set by default inherritAttrs to false so you don't need to change anything.
• satisfy the need of Vue developers that they would like to benefit from the power of Vue features.
• adopt Vue reactivity, rendering, as a natural API to render your story.

so what do you think ?

[bodograumann]

First of all, I agree with what kasperpeulen wrote in the fix:

[the users] don't expect the template that they give to Storybook to be invoked as a component with their args as props. The user cannot see how SB will invoke the component, and args are already available in the closure. It is not intuitive that args are also available via a second route.

I guess there might be an argument to be made for having a default renderer (is that the correct term?), which just renders the component from the story metadata and applies all args as vue props.
Apart from that this seems like a complicated topic.
My main concern would be, that there needs to a very good documentation for using args.
I'll try to have a look at your PR and continue the discussion there, @chakAs3. That seems to be a more suitable place for it.

[chakAs3]

My main concern would be, that there needs to a very good documentation for using args.

documentation is important to guide users and avoid confusion. We still lack complete documentation.

[chakAs3]

First of all, I agree with what kasperpeulen wrote in the fix:

[the users] don't expect the template that they give to Storybook to be invoked as a component with their args as props. The user cannot see how SB will invoke the component, and args are already available in the closure. It is not intuitive that args are also available via a second route.

Yes i agree that some of them especially old users because we did not teach them that. untill now we say template but actually it is not template that users set in the render function it is a component

{ render:( args )=> VueRenderComponent }
// so it can be composition api
{ render: ( args ) => { components :{} , setup() {  return ()=> h('div',args,'Hello Vue C')   } } 
// or options api with render option or template 
{ render: ( args ) => { components :{} , render:()=> h('div',args,'Hello Vue C')   } } 
// or function component simply 
{ render: ( args ) => ()=> h('div',args,'Hello Vue C')   } 

[Sidnioulz]

that was my direct simple solution if you really want to make things right, Vue has his great API and behaviour that should. be respected, I felt no one was really there from the community. Please check this #22241. we could avoid all these bugs because only by doing the right things will have a solid maintainable codebase.

I don't think there is a right and wrong here. The problem with applying inheritAttrs manually is it increases the barrier to entry. Addon decorators would need to do this right. Folks who write their first decorator in their codebase too. As my org works on Vue, we notice that all incoming hires are familiar with React, etc., but almost none with Vue. Being able to hide complexity is beneficial to us.

My priority is to have a clear mapping between props+listeners+slots and args, one that lets us write new components and documentation use cases fast, at scale. The "natural Vue way" is a massive hindrance right now, because it results in lots of code duplication across stories, which complicates maintenance. As soon as args don't get perfectly mapped to both props+listeners+slots, then people need to start destructuring the args and manually inject each slot in a template. In our Vue 2 codebase, I automated all that behind an abstraction layer specifically so people wouldn't have to figure out args -> props + listeners + slots + actual HTML attrs -> v-bind + v-on + <template>.

I've seen large swathes of our codebase with the natural Vue way of writing stories, using event listeners and slot templates, and with an abstraction layer. Here's my feedback:

• Everyone who's tried both preferred the abstraction layer
• Stories that attempted to map args manually ended up being less DRY and requiring more updating
• If abstraction layers are well built directly into SB rather than our codebase, then we have better support across the board for prop reactivity, and we have fewer addons bugging out because they didn't set Vue-specific parameters correctly

[bodograumann]

So I finally managed to try the update and can confirm that the bug has been fixed in storybook 7.0.18.
Will you close this issue, @Radouch ?

As a follow-up, there is a proposal by @shilman about making vue stories easier to write, if anybody is interested: https://www.notion.so/chromatic-ui/RFC-Vue-story-components-9dc085671df34e12901b48fcba4b491d

[shilman]

Thanks @bodograumann 🙏 Closing this for now. Please let me know if there's more to be done here!