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

Global decorators in config.js are removed after hot module reload #1637

Closed
clementoriol opened this issue Aug 10, 2017 · 25 comments
Closed
Assignees

Comments

@clementoriol
Copy link

clementoriol commented Aug 10, 2017

Using the following package versions :

"@storybook/addon-actions": "^3.2.0",
"@storybook/addon-info": "^3.2.0",
"@storybook/addon-knobs": "^3.2.0",
"@storybook/addon-options": "^3.2.3",
"@storybook/react": "3.2.3",

After adding global decorators in the config.js (before the configure function), the decorators work as expected (addon-info is here, addon-knobs works, etc.), but as soon as I save one of my file (a story for exemple), Storybook hot-reloads and the decorators are gone. Knobs doesn't work anymore, my custom decorator for wrapping the Preview component are gone, etc.

I tried removing decorators one by one with no success. The issues looks fairly consistent on any kind of Decorator.

I manage to find a workaround for now, downgrading my packages to these versions :

"@storybook/addon-actions": "^3.1.6",
"@storybook/addon-info": "^3.2.0",
"@storybook/addon-knobs": "^3.1.6",
"@storybook/addon-options": "^3.1.6",
"@storybook/react": "^3.1.6",

And it now works as expected.

I can't provide a simple repo reproducing the error right now, but I believe it must be easily reproducible (I encountered the same bug on two projets with very different configurations).

Let me now if you can't reproduce it, and I'll make one.
Thank you !

@clementoriol clementoriol changed the title Global decorators are removed after hot module reload Global decorators in config.js are removed after hot module reload Aug 10, 2017
@ntucker
Copy link

ntucker commented Aug 11, 2017

There are two other reports of this in the original issue whose PR merge created this bug: #877 (comment)

@deehuey
Copy link

deehuey commented Aug 11, 2017

Any updates?

@ndelangen ndelangen added the bug label Aug 13, 2017
@ndelangen
Copy link
Member

@clementoriol it would be super useful for anyone picking up this issue if there's a ready to go reproduction repo to clone. So it would help out tremendously if you could set that up? 🙇

Switching from 3.2 to 3.1 solved the issue for you? damn, so we introduced this bug recently. Sorry about that.

@ntucker
Copy link

ntucker commented Aug 13, 2017

@ndelangen FYI the bug is added here: 8a48f80

@mrtnpro
Copy link
Contributor

mrtnpro commented Aug 18, 2017

We're having the same issue with applying a global decorator which disappears after hmr:

// Custom decorator providing IntlProvider
const CustomIntlProvider = (storyFn) => (
  <IntlProvider locale="en">
    { storyFn() }
  </IntlProvider>
);

addDecorator(CustomIntlProvider);

(We know there's storybook-addon-intl however it seems to introduce much more errors in our storybook so we decided to provide react-intl manually)

Yet, adding the same decorator manually inside a story works just fine and does not break on hmr.

import { IntlProvider } from 'react-intl';
import { storiesOf } from '@storybook/react';

import Component from '../component';

const CustomIntlProvider = (storyFn) => (
  <IntlProvider locale="en">
    { storyFn() }
  </IntlProvider>
);

storiesOf('Component', module)
.addDecorator(CustomIntlProvider)
.add('Component', () => <Component />);

/cc @diablourbano

@ntucker
Copy link

ntucker commented Aug 18, 2017

@ndelangen I don't know why this 8a48f80 was added. All it does is break HMR. HMR will run modules again while maintaining state so it just throws and exception. PR is to revert this buggy commit.

@igor-dv
Copy link
Member

igor-dv commented Aug 23, 2017

I think we are facing the same issue. I'll try to look into it ASAP.

@ndelangen
Copy link
Member

Thank you @igor-dv 🙇

@igor-dv
Copy link
Member

igor-dv commented Aug 23, 2017

Let's make some order.

  1. Improved error checking in global addDecorator #1481 was about to fix some wrong behavior in Storyshots regarding the global decorators (Global decorators don't work in storyshots when applied in config file #877, .addDecorator is probably not working #879) I still can't relate what was the problem, can someone elaborate it ? (cc @tmeasday)
  2. This PR was about not to update global decorators after stories were loaded.
  3. Since in a regular work HMR causes global decorators to be cleaned and re-added again (IMO it's OK, because we do want to add global decorators on the fly) - it introduced a bug in which these decorators are cleaned but NOT re-added.

IMO we need to revert this change and fix the storyshots problem in a proper way.

@tmeasday
Copy link
Member

tmeasday commented Aug 23, 2017

The idea of #1481 was to help people who put their global decorators after their story modules are required, like: https://github.com/ktj/storybook-rp/blob/master/.storybook/config.js#L6-L20

Looking at it closer, the reason the above code breaks in SS is that storyshots does this totally zany thing to emulate require.context (a webpack thing, SS runs in Jest): https://github.com/storybooks/storybook/blob/master/addons/storyshots/src/require_context.js

Honestly, I think it's just mis-implemented, maybe there is something I am missing.

We should revert the PR and fix the require context thing.

@igor-dv
Copy link
Member

igor-dv commented Aug 23, 2017

Maybe it's far from being discussed (and a wrong place), but what if configure, will take loadDecorators function, like it takes loadStories.. And in general, may be it's better to limit the API to some life-cycle

@theKashey
Copy link
Contributor

Still an issue :(

@wintercounter
Copy link

Why is this closed?

@shilman
Copy link
Member

shilman commented Jul 23, 2019

@wintercounter are you still seeing this? If so, what version?

@wintercounter
Copy link

Latest. I'm having a custom ApolloProvider as:

addDecorator(storyFn => <ApolloProvider client={client}>{storyFn()}</ApolloProvider>)

@shilman
Copy link
Member

shilman commented Jul 23, 2019

@wintercounter 5.1.9? Just verifying since I've been touching that code in 5.2.0-beta.x and want to make sure it wasn't those changes that caused a regression. Thanks.

@wintercounter
Copy link

Yes, it 5.1.9.

@wintercounter
Copy link

We still have this issue btw on 5.2.6. SInce we develop in Storybook in the first place, it makes the process really painful.

@heleg
Copy link

heleg commented Nov 22, 2019

@wintercounter Do you require your decorators from another file?

I had this problem until I removed require and started to use addDecorator directly in my config.js

@wintercounter
Copy link

No, i have it in my setup:

// Storybook setup file
import { addDecorator } from '@storybook/react'
import { ApolloProvider } from 'react-apollo'
import { ApolloClient } from 'apollo-client'
import { ApolloLink } from 'apollo-link'
import { InMemoryCache } from 'apollo-cache-inmemory'

const cache = new InMemoryCache()

const client = new ApolloClient({
    link,
    cache
})

addDecorator(storyFn => <ApolloProvider client={client}>{storyFn()}</ApolloProvider>)

@shilman
Copy link
Member

shilman commented Nov 25, 2019

@wintercounter do you have a repro repo I can look at?

@raphaelboukara
Copy link

raphaelboukara commented Dec 30, 2019

To reproduce:

Open a new shell and:

nvm use 10
git clone https://github.com/raphaelboukara/storybug.git
cd storybug
npm i
npm run storybook

image

go to src/Button/index.js.
add some changes. (for instance add "hi!" before text)
image

The global decorator disappear.

Some info about my environment:

storybug master node --version
v10.15.3
storybug master npm --version
6.13.4
storybug master nvm --version
0.34.0

@shilman
Copy link
Member

shilman commented Dec 30, 2019

@raphaelboukara global decorators should be applied in config.js. if you move the global decorator in your example, HMR works as expected.

@peterp
Copy link

peterp commented Jun 28, 2020

config.js doesn't seem to be documented.

@tmeasday
Copy link
Member

@peterp it is now .storybook/preview.js

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