storybookjs / storybook

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

Vue 3 app is reused, therefore plugins can't be re-initialized #18222

Open Akryum opened 2 years ago

Akryum commented 2 years ago

Describe the bug Writing stories involving Vue 3 plugins is made difficult by the reusing of the underlying app instance between stories. A typical usecase is to mock the vuex store and the router for each story. Using app from @storybook/vue3 works the first time a story is loaded, then an error occurs the next time:

can't redefine non-configurable property "$route"

image

Besides the error, the new plugins aren't being registered because they are not supposed to in Vue 3.

To Reproduce https://github.com/Akryum/storybook-issue-app-use/blob/main/src/stories/IssueStory.stories.js

System

Environment Info:

  System:
    OS: Linux 5.17 Fedora Linux 36 (Workstation Edition)
    CPU: (24) x64 AMD Ryzen 9 3900XT 12-Core Processor
  Binaries:
    Node: 16.14.0 - ~/.nvm/versions/node/v16.14.0/bin/node
    Yarn: 3.2.1 - ~/.nvm/versions/node/v16.14.0/bin/yarn
    npm: 8.3.1 - ~/.nvm/versions/node/v16.14.0/bin/npm
  Browsers:
    Chrome: 101.0.4951.64
    Firefox: 100.0
  npmPackages:
    @storybook/addon-actions: ^6.5.0-beta.8 => 6.5.0-beta.8 
    @storybook/addon-docs: ^6.5.0-beta.8 => 6.5.0-beta.8 
    @storybook/addon-essentials: ^6.5.0-beta.8 => 6.5.0-beta.8 
    @storybook/addon-interactions: ^6.5.0-beta.8 => 6.5.0-beta.8 
    @storybook/addon-links: ^6.5.0-beta.8 => 6.5.0-beta.8 
    @storybook/builder-webpack5: ^6.5.0-beta.8 => 6.5.0-beta.8 
    @storybook/manager-webpack5: ^6.5.0-beta.8 => 6.5.0-beta.8 
    @storybook/testing-library: ^0.0.11 => 0.0.11 
    @storybook/vue3: ^6.5.0-beta.8 => 6.5.0-beta.8 

Additional context

I believe there is a fundamental issue in the logic of @storybook/vue3: reusing the app instance.

Instead of doing this:

import { app } from '@storybook/vue3'
import { createStore } from 'vuex'
import { createRouter, createMemoryHistory } from 'vue-router'

import IssueStory from './IssueStory.vue'

export default {
  title: 'IssueStory',
  component: IssueStory,
}

const Template = (args) => {
  // Mock store
  app.use(createStore({
    state: () => ({
      foo: 'bar',
    }),
  }))
  // Mock router
  app.use(createRouter({
    history: createMemoryHistory(),
    routes: [
      { path: '/', name: 'mock', component: { render: () => null } },
    ],
  }))
  return {
    components: { IssueStory },
    setup: () => ({ args }),
    template: '<IssueStory v-bind="args"/>',
  }
}

export const Example = Template.bind({})

A better API to work around this issue would be:

import { createStore } from 'vuex'
import { createRouter, createMemoryHistory } from 'vue-router'

import IssueStory from './IssueStory.vue'

export default {
  title: 'IssueStory',
  component: IssueStory,
}

const Template = (args, { app }) => {
  // Mock store
  app.use(createStore({
    state: () => ({
      foo: 'bar',
    }),
  }))
  // Mock router
  app.use(createRouter({
    history: createMemoryHistory(),
    routes: [
      { path: '/', name: 'mock', component: { render: () => null } },
    ],
  }))
  return {
    components: { IssueStory },
    setup: () => ({ args }),
    template: '<IssueStory v-bind="args"/>',
  }
}

export const Example = Template.bind({})

Notice how app is specific to each story here.

There should also be a way to apply plugins and interact with the current app instance in preview.js:

import { onAppCreated } from '@storybook/vue3'

onAppCreated(app => {
  app.use(VueI18n)
  app.component('BaseButton', BaseButton)
})
yannbf commented 2 years ago

Hey @Akryum ! Thank you so much for writing this issue and even more for providing a repro repo!

I'll try my best to experiment with this and come up with a solution. Would you be able to assist if needed?

Akryum commented 2 years ago

Sure!

yannbf commented 2 years ago

Alright @Akryum I did my best to analyze the issue and come up with a solution. I opened a PR which is linked to this issue, and there I added a video where I explain what I did. It's just a conversation starter (that's why it's a draft PR), I'm sure there are better ways to do this and therefore I'd love your assistance.

Unfortunately that is a breaking change, reason being that the app coming from @storybook/vue3 will have to be mutated now, given that there will be always a cleanup and re-instantiation of the app when rendering a new story. So the way I managed to always give the up to date reference of the vue app was to turn that into a function. I wonder if there's a way to fix that without having a breaking change!

Tagging @prashantpalikhe in case you have good ideas also!

Akryum commented 2 years ago

It looks good. I guess using a Proxy could make a non-breaking version.

yannbf commented 2 years ago

It looks good. I guess using a Proxy could make a non-breaking version.

Thanks! That's a good idea. Still we have a challenge with docs mode, given that every story renders in the same page. I added some examples as comments in the PR. Do you have any idea on how to circumvent the issues?

Akryum commented 2 years ago

I'm not familiar with how storybook works, but couldn't each story have its own Vue 3 app?

Akryum commented 2 years ago

There is also a challenge with the usage of app in preview.js since plugins need to be re-instanciated, I don't see a non-breaking way for this.

thedamon commented 1 year ago

Running into this.. I have added router to certain routes in a decorator and anytime HMR runs I get "Cannot redefine property: $route" error. It also makes me wonder how often the code in preview.js reruns on the same instance of app from what @Akryum is saying it's any time i go to a new Story without reloading or anytime I make a change to the story and it hot reloads?

I think this will also mean that what I see in storybook is in certain instances unlikely to not match the real world functioning of the components/template code within the story? 😕

nskazki commented 1 year ago

Running into the same issue with Vue 3, but I feel confused about it. Having an individual Vuex store per story is such a typical task. No way there's no obvious and simple solution that I just fail to see.

nskazki commented 1 year ago

To define a Vuex store per story, I ended up writing this. Pay attention I'm still on Storybook v5. For the newer versions, the idea is the same, but the decorator's signature may be different.

Written a decorator that wraps a story into a component and adds a $storybookStore to the wrapper

export const WithStore = (state) => {
  return (_story) => {
    return {
      computed: {
        $storybookStore() {
          // storeConfig is a normal config with mutations, actions, and getters.
          // state is a plain object with overrides specific to the story. 
          // _.merge is lodash.merge.
          return createStore({
            ...storeConfig,
            state() {
              return _.merge(storeConfig.state(), state)
            }
          })
        }
      },
      provide() {
        // for a reason, I couldn't make it work with composition API's provide.
        // probably because the app is being migrated to Vue 3. 
        return {
          store: this.$storybookStore
        }
      },
      template: '<story />'
    }
  }
}

And patched the globalProperties to traverse the parent chain looking for the $storybookStore defined in the wrapper whenever $store is accessed in the story.

Object.defineProperty(app.config.globalProperties, '$store', {
  configurable: true,
  enumerable: true,
  get() {
    let vm = this
    while (vm) {
      if (Object.hasOwnProperty.call(vm, '$storybookStore')) {
        return vm.$storybookStore
      }

      vm = vm.$parent
    }

    return null
  }
})
freakzlike commented 1 year ago

I'm currently using this workaround to avoid calling setup twice on 7.0.0-beta.48

preview.js

import { setup } from '@storybook/vue3'

setup(app => {
  // Storybook calling setup twice per app
  if (!app.__STORYBOOK_SETUP_CALLED__) {
    // init vue plugins here
  }
  app.__STORYBOOK_SETUP_CALLED__ = true
})
Horlamedhey commented 1 year ago

I'm currently using this workaround to avoid calling setup twice on 7.0.0-beta.48

preview.js

import { setup } from '@storybook/vue3'

setup(app => {
  // Storybook calling setup twice per app
  if (!app.__STORYBOOK_SETUP_CALLED__) {
    // init vue plugins here
  }
  app.__STORYBOOK_SETUP_CALLED__ = true
})

I get this error though

Screenshot 2023-05-18 at 09 25 09