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

Clear decorators on module dispose #335

Merged
merged 1 commit into from
Jul 26, 2016
Merged

Conversation

thani-sh
Copy link
Contributor

The config.js file acts as the main entrypoint for users storybook
code. Stories, decorators etc. are added from here. When webpack HMR
replaces this module global decorators get applied repeatedly. This
happens because although the config.js file gets replaced the storybook
module remains unchanged along with all global decorators we may have
added before the replacement.

It is okay to remove all decorators when config.js module gets replaced
because we can be sure that they'll be added again when config.js file
gets executed.

The `config.js` file acts as the main entrypoint for users storybook
code. Stories, decorators etc. are added from here. When webpack HMR
replaces this module global decorators get applied repeatedly. This
happens because although the config.js file gets replaced the storybook
module remains unchanged along with all global decorators we may have
added before the replacement.

It is okay to remove all decorators when config.js module gets replaced
because we can be sure that they'll be added again when config.js file
gets executed.
@thani-sh
Copy link
Contributor Author

Fixes #284

@arunoda
Copy link
Member

arunoda commented Jul 26, 2016

Looks great to me.

@thani-sh thani-sh merged commit 0a9b595 into master Jul 26, 2016
@thani-sh thani-sh deleted the fix-hmr-duplicate-decorator branch July 26, 2016 08:08
@shilman shilman added the misc label May 27, 2017
@francoismajor
Copy link

francoismajor commented Nov 11, 2020

FYI after updating to latest version of Storybook, this bug seems to have been re-introduced. I use a global wrapper and every hot reload adds one (or many) more wrappers.

For now, I've imported and added clearDecorators(); in my config.js, which solves the issue but seems this should still be implemented like it was before (haven't had this issue in the past year, using the same setup, only since v 6.0.27)

Cheers

(my file now looks like this, but without clear I get the issue.)

import { configure, addDecorator, addParameters } from '@storybook/vue';
import { withA11y } from '@storybook/addon-a11y';
import Vue from 'vue';
import Vuex from 'vuex';
import UUID from 'vue-uuid';
import i18n from '../src/plugins/i18n';
import '../src/directives/click-outside';
import {VClosePopover, VPopover, VTooltip} from 'v-tooltip'
import '@/assets/styles/global.scss';
import Toast from '@/plugins/toast';
import {clearDecorators} from "@storybook/vue/dist/client/preview";

Vue.use(UUID);
Vue.use(Vuex);
Vue.use(Toast);
Vue.directive('tooltip', VTooltip);
Vue.directive('close-popover', VClosePopover);
Vue.component('v-popover', VPopover);

addParameters({
  backgrounds: [
    { name: 'white', value: '#fff',  default: true },
    { name: 'gray', value: '#f6f7f8' },
  ],
});

const backgroundWrappers = () => ({
  data: () => ({
    wrapper: {
      margin: "0 32px 32px",
      border: "thin solid transparent",
      boxShadow: "0 6px 8px 0 rgba(37, 55, 101, 0.2)",
      borderRadius: "3px",
      padding: "32px"
    },
    light: {
      backgroundColor: "#ffffff"
    },
    grey: {
      backgroundColor: "#f6f7f8",
    },
    heading: {
      fontSize: "14px",
      margin: "0",
      padding: "16px 32px",
      color: "#253765",
      textTransform: "uppercase"
    }
  }),
  template: `
    <div>
      <div :style="heading">Sur fond blanc</div>
      <div :style="[wrapper, light]"><story/></div>
      <div :style="heading">Sur fond gris</div>
      <div :style="[wrapper, grey]"><story/></div>
    </div>
    `
});

export { backgroundWrappers };

clearDecorators();

addDecorator(backgroundWrappers);

addDecorator(withA11y);

addDecorator(() => ({
  template: '<story/>',
  i18n,
}));

// automatically import all files ending in *.stories.js
configure(require.context('../stories', true, /\.stories\.js$/), module);

I use CSF

import ChartMiniBar from '@/components/chart/ChartMiniBar';

export default {
  component: ChartMiniBar,
  title: 'Components/Chart/Chart Mini Bar',
};

export const chartMiniBar = () => ({
  components: { ChartMiniBar },
  data() {
    return {
      series: [
        {
          data: [30, 40, 45, 50, 49, 60, 70, 81, 25, 65, 13, 65],
        },
      ],
    };
  },
  template: '<ChartMiniBar :series="series" />',
});

export const chartMiniBarBlue = () => ({
  components: { ChartMiniBar },
  data() {
    return {
      series: [
        {
          data: [30, 40, 45, 50, 49, 60, 70, 81, 25, 65, 13, 65],
        },
      ],
    };
  },
  template: '<ChartMiniBar :series="series" on-blue-background />',
});

@shilman
Copy link
Member

shilman commented Nov 12, 2020

cc @tmeasday

@tmeasday
Copy link
Member

What happened in 6.0.27 @shilman?

@francoismajor -- I think the issue would be solved if you used main.js to require your stories rather than the require.context() -- is there a reason you can't do that?

@francoismajor
Copy link

@tmeasday No reason, I followed some tutorial for setup and that's the way it was done ;)

I see the docs do it differently now. main.js, and preview.js for decorators. I figured something might have changed in the way to setup everything but couldn't spot what.

I'll give this a try and see if that solves it.

@tmeasday
Copy link
Member

@francoismajor it shouldn't be necessary but I think it'll workaround the issue anyway.

Do you have an exact version where the issue started to appear? i.e. did it work in 6.0.26 but not in 6.0.27?

@francoismajor
Copy link

@tmeasday no, we went from "@storybook/vue": "^5.3.9", to "@storybook/vue": "^6.0.27", using update latest, so it could be anything in between...

@tmeasday
Copy link
Member

Hmm, ok thanks.

@francoismajor
Copy link

francoismajor commented Nov 18, 2020

Okkkk. This is the guide I didn't read when I updated 🤦 There's lots of things in there that apply to my current setup...

https://medium.com/storybookjs/storybook-6-migration-guide-200346241bb5

So, just forget I mentioned anything 😅 , I'll go through and do a proper update and I'm pretty sure everything'll be great. Thanks for the quick replies.

@tmeasday
Copy link
Member

I guess so although I think there may still be a bug here. I guess we'll see if anyone else runs into it. Good luck!

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

Successfully merging this pull request may close these issues.

None yet

5 participants