Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Storybook unmounts Vue components when args change #18313

Closed
Juice10 opened this issue May 24, 2022 · 9 comments
Closed

[Bug] Storybook unmounts Vue components when args change #18313

Juice10 opened this issue May 24, 2022 · 9 comments

Comments

@Juice10
Copy link

Juice10 commented May 24, 2022

Describe the bug
Storybook unmounts Vue components when args change, wiping internal data and stopping css transitions from working.

To Reproduce
Github Repo: https://github.com/Juice10/sb-vue3-unmount-repro

Toggle primary true/false and see the number in the button change.
Chromatic: https://www.chromatic.com/component?appId=628cdebba97684003a26c695&csfId=example-button--primary&buildNumber=1&k=628cdf3b450e3a004a11f9c1-1200-docs-true&h=2&b=24

System

Environment Info:

  System:
    OS: macOS 12.3.1
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
  Binaries:
    Node: 12.22.8 - ~/.nvm/versions/node/v12.22.8/bin/node
    Yarn: 3.2.1 - /usr/local/bin/yarn
    npm: 6.14.15 - ~/.nvm/versions/node/v12.22.8/bin/npm
  Browsers:
    Chrome: 101.0.4951.64
    Firefox: 80.0
    Safari: 15.4
  npmPackages:
    @storybook/addon-actions: ^6.5.4 => 6.5.4 
    @storybook/addon-docs: ^6.5.4 => 6.5.4 
    @storybook/addon-essentials: ^6.5.4 => 6.5.4 
    @storybook/addon-interactions: ^6.5.4 => 6.5.4 
    @storybook/addon-links: ^6.5.4 => 6.5.4 
    @storybook/builder-vite: ^0.1.35 => 0.1.35 
    @storybook/builder-webpack4: ^6.5.4 => 6.5.4 
    @storybook/manager-webpack4: ^6.5.4 => 6.5.4 
    @storybook/testing-library: ^0.0.11 => 0.0.11 
    @storybook/vue3: ^6.5.4 => 6.5.4 

(also happens on M1 Mac with yarn 1, specific environment info for that setup will follow)

Additional context
Happens both in Canvas and Docs.
I think it might have to do with #12255. I'm not using decorators directly, but I noticed internally @storybook/vue3 is using decorators (see @storybook/vue3/preview/decorateStory.js:36), and they are triggering Vue's re-render (see trace below).
image

@thomasaull
Copy link

thomasaull commented Jul 2, 2022

@Juice10 I have a solution for this, it actually need very little changes on the vue3 framework integration, check this commit: thomasaull@252862a

However it would cause some breaking changes, the official way of writing vue 3 components:

const Template = (args) => ({
  components: { Button },
  setup() {
    //👇 The args will now be passed down to the template
    return { args };
  },
  template: '<Button v-bind="args" />',
});

would still work, but only on the first render. On subsequent arg updates the component would not reflect the new state. For this to work properly, you'd need to incorporate the changes from my commit and write stories like it's recommended for vue 2:

const Template = (args, { argTypes ) => ({
  props: Object.keys(argTypes),
  components: { Button },
  template: '<Button v-bind="$props" />',
});

For all I know about vue, it's not possible to have this working without using props.

However @shilman has indicated in the past, they do not want to go this route, so unless we can figure out a way to make both the new and the old way work, it'll probably not get merged.

I have used a forked package hosted in a private gitlab registry in the past and I would be willing to publish this if people are interested (can't promise it to be always up to date though).

@thedamon
Copy link

Do we know why the vue2 way doesn't work in vue3 out of the box? there's nothing about it that seems incompatible with vue 3. Also curious to see the comments about this from @shilman as creating a new component with different props is very different from updating props in an existing components and honestly both are an important test case.

@thedamon
Copy link

We're currently really confused about what is happening with storybook behind the scenes as we are upgrading to vue-3 in a feature branch and can find very little guidance on what to expect (and a lot of what's happening isn't good). we were hoping that stories would not need to be versioned specifically for the version of vue.

@shilman
Copy link
Member

shilman commented Sep 28, 2022

@thedamon I just spent a bit looking through the vue2/vue3 rendering functions with @prashantpalikhe. We're trying to reconcile the two different approaches & hopefully provide a fix & recommendations as part of 7.0.

Here's the context:

  • The Vue2 renderer was written ages ago, and it's args handling / remount behavior was developed in 6.0
  • The Vue3 renderer was written much later, by a different author, and took a different approach which has a bunch of nice properties. Most recent one I encountered if you update args in a decorator it does the right thing, whereas in Vue2 it ignores that. There were other issues that came up as well, but I'd need to search.

My crude understanding is that the Vue3 renderer fixes a bunch of issues with the Vue2 renderer, but possibly at the expense of this remount behavior. Ideally we "get it all", but at this point I'd be happy with two renderers that both work with Storybook's desired semantics: "args changes re-render, HMR remounts" cc @tmeasday

@tmeasday
Copy link
Member

tmeasday commented Sep 28, 2022

One thing we are doing as part of reworking our examples=>sandboxes and generic stories is creating test cases that explicitly test this behaviour across all the frameworks. That's making it a lot easier to understand the (unintentional) differences between SB's behaviour in different renderers and in the future it'll help up ensure that we both stay consistent and don't regress.

It sounds like the issue in this case is covered by this example, so we should ensure that in fixing @shilman that both the story starts broken and ends up fixed in the vue3 sandbox.

@tmeasday
Copy link
Member

PS reading that story, it currently links to #13913 -- is that a dupe of this issue?

@shilman
Copy link
Member

shilman commented Sep 29, 2022

Thanks @tmeasday. Closing this as a dupe to #13913

@thedamon
Copy link

thedamon commented Oct 1, 2022

Here's the context:

This is really helpful, thank you!

It is unfortunate though as it drastically increases the complexity of upgrading from vue2 to vue3 within storybook if it's not possible to do very quickly within a commit or two (which it isn't because it's already totally confusing and hard due to the absence of any resources and the intensely internal stuff that storybook does at a framework level). Still trying, but feeling like it may have been foolhardy. We almost need to recreate all our stories outside of storybook.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants