stories.add calls story function twice

[faceyspacey]

Is there any reason why func in stories.add('name', func) is called twice?

I'm working on furthering the capabilities of storybook-addon-specifications and I'd like to guarantee the tests contained in func are not called more than once.

Is this the normal behavior and is there any way to guarantee its called only once?

[faceyspacey]

UPDATE:

judging from the stack:

CALL 1:

at eval (webpack:///./storybook/facade.js?:141:7)
    at eval (webpack:///./~/@kadira/storybook/dist/client/preview/client_api.js?:96:22)
    at KnobManager.wrapStory (webpack:///./~/@kadira/storybook-addon-knobs/dist/KnobManager.js?:84:28)
    at withKnobs (webpack:///./~/@kadira/storybook-addon-knobs/dist/index.js?:93:18)
    at eval (webpack:///./~/@kadira/storybook/dist/client/preview/client_api.js?:95:20)
    at renderMain (webpack:///./~/@kadira/storybook/dist/client/preview/render.js?:108:17)
    at renderPreview (webpack:///./~/@kadira/storybook/dist/client/preview/render.js?:141:12)
    at Array.renderUI (webpack:///./~/@kadira/storybook/dist/client/preview/index.js?:89:26)
    at Object.dispatch (webpack:///./~/redux/lib/createStore.js?:186:19)
    at ConfigApi._renderMain (webpack:///./~/@kadira/storybook/dist/client/preview/config_api.js?:47:24)

CALL 2:

at eval (webpack:///./storybook/facade.js?:141:7)
    at eval (webpack:///./~/@kadira/storybook/dist/client/preview/client_api.js?:96:22)
    at KnobManager.wrapStory (webpack:///./~/@kadira/storybook-addon-knobs/dist/KnobManager.js?:84:28)
    at withKnobs (webpack:///./~/@kadira/storybook-addon-knobs/dist/index.js?:93:18)
    at eval (webpack:///./~/@kadira/storybook/dist/client/preview/client_api.js?:95:20)
    at renderMain (webpack:///./~/@kadira/storybook/dist/client/preview/render.js?:108:17)
    at renderPreview (webpack:///./~/@kadira/storybook/dist/client/preview/render.js?:141:12)
    at Array.renderUI (webpack:///./~/@kadira/storybook/dist/client/preview/index.js?:89:26)
    at Object.dispatch (webpack:///./~/redux/lib/createStore.js?:186:19)
    at ConfigApi._renderMain (webpack:///./~/@kadira/storybook/dist/client/preview/config_api.js?:48:24)

It's because the component is rendered twice due to multiple redux dispatches:

function _renderMain(loaders) {
      if (loaders) loaders();

      var stories = this._storyStore.dumpStoryBook();
      // send to the parent frame.
      this._channel.emit('setStories', { stories: stories });

      // clear the error if exists.
      this._reduxStore.dispatch((0, _actions.clearError)()); // CALL 1
      this._reduxStore.dispatch((0, _actions.setInitialStory)(stories)); // CALL 2
    }

But does the user's code really need to be re-rendered twice. Perhaps clearError() can be called before setStories is emitted or something like this. This can be problematic because imagine if on componentDidMount() the story is dispatching an action, and then in your tests you're counting the number of actions dispatched. You will now have 2 instead of 1. And depending on your reducers or other code, the state may be different than what you expect.

I know React StoryBook wasn't initially for redux usage, but it makes a damn good tool for working on your whole application--from tests to redux and more--if you know how to get the most use out of it. I don't see why it can't evolve into a tool that expects many users to be using Redux. In fact, it's a lot of throwaway work if this is just for designing components, and eventually you stop using the tool. Because the thing is as your app becomes real, half your components become connected redux containers. So if Storybook components aren't wrapped in a redux provider, you are left maintaining 2 codebases: your real app, and raw components, which expect a different set of props. It's way more efficient to be thinking in terms of the same set of connected container components. That way Storybook is more than just a designer's tool. Combine first-class redux support/usage with tests, and you have a way to bring up any focal point of your app and get down to business with ease. You basically can setup key areas you work on, and have a consistent state to expect once you get there--with passing/failing tests further informing you on what the "state" of the given area is.

Storybook lets you "bookmark" that state if you're using Redux. And not just the pretend raw component version of your app, but your real app. I don't see why anyone wouldn't want that.

So tests just make it even more important things are reliably rendering. But it's not just limited to tests. Of course a test should be able to re-run and not be dependent on how many times it ran, but because of the unique nature of Storybook and the Specifications plugin, it's useful to think of each describe block as one test where you setup your component once to be the return of stories.add() AND to be the basis for a bunch of consecutive tests, where in each of those tests the component is not setup again. That means, re-renderings not triggered by the user could have negative effects on the results of tests, e.g. multiple actions dispatched, incorrect state.

A redux dispatch batching package would easily solve this as well. It's generally more efficient too.

Another reason why to fix/enhance this: if you're putting any logging in your story, you're going to see 2 logs and wonder why. This sort of thing, as small as it is, does not breed confidence. It's a bad signal to developers that leaves them scratching their head.

[faceyspacey]

In addition to double rendering your component, this._reduxStore.dispatch((0, _actions.clearError)()) will also cause the following issue:

<Provider> does not support changing `store` on the fly. It is most likely that you see this error because you updated to Redux 2.x and React Redux 2.x which no longer hot reload reducers automatically.

and possibly this one:

Warning: Exception thrown by hook while handling onSetChildren: Invariant Violation: Expected onBeforeMountComponent() parent and onSetChildren() to be consistent (22 has parents 21 and 19).
Invariant Violation: Expected onBeforeMountComponent() parent and onSetChildren() to be consistent (22 has parents 21 and 19).

This is specifically when you wrap your components in a provider without using addDecorator.

The reason I'm doing it outside of addDecorator is because I've taken the storybook-addon-specifications and mocked it in such a way that you can use Storybook with regular describe/it tests if you simply return a story component from your it blocks. So I have it working very nicely for tests like this:

describe('AnimatedTransitionGroup 1', () => {
  it('blue', () => {
    console.log('RENDER')
    const { story, store } = setupStory()

    store.dispatch({ type: 'CHANGE', payload: 'blue' })
    store.dispatch({ type: 'BLUR', payload: 'blue' })

    const { color } = store.getState()
    expect(color).toEqual('blue')

    const component = renderer.create(story)
    expect(component).toMatchSnapshot()

    return story // when run in Storybook, this will become the return of `stories.add()`
  })

  it('red', () => {
    const { story, store } = setupStory()

    store.dispatch({ type: 'CHANGE', payload: 'red' })
    store.dispatch({ type: 'BLUR', payload: 'red' })

    const { color } = store.getState()
    expect(color).toEqual('red')

    const component = renderer.create(story)
    expect(component).toMatchSnapshot()

    return story // when run in Storybook, this will become the return of `stories.add()`
  })

  it('just tests, no story returned', () => {
    expect(1).toEqual(1)
    expect(2).toEqual(2)
    expect(3).toEqual(3)
    expect(1).toEqual(1)

    // notice no story is returned
  })

  it('more equal tests', () => {
    expect(66).toEqual(66)
    expect(55).toEqual(55)

    // notice no story is returned
  })
})

What I got going is extremely slick. Here's why: it lets you write tests as normal, and not worry that you're possibly messing it up by mixing in storybook calls will mess up your tests. Your tests are the same as before, with the very minor option of returning a story react component. I think this is a big win for the very important goal of mixing tests with Storybook.

It means your Storybook code is way more likely to stay maintained throughout the life of the project. For the importance of mixed snapshot testing, the real win is that you only need to setup components once and it is re-used between both Storybook and your tests. That is the win here. I got it working extremely nicely for me right now.

Give it a quick try to see what I'm talking about:

git clone git@github.com:faceyspacey/animated-transition-group.git
cd animated-transition-group
yarn
npm run test // just to see that the tests are working
npm run storybook 

If you're using Wallaby, the tests will run very nicely there, and your snapshots will appear there, diffed when failing. Wallaby + Storybook are perfect companions.

The files worth looking at are:

THE TESTS:
https://github.com/faceyspacey/animated-transition-group/blob/master/stories/index.js

THE FACADE FOR STORYBOOK:
https://github.com/faceyspacey/animated-transition-group/blob/master/storybook/facade.js

THE FACADE FOR JEST:
https://github.com/faceyspacey/animated-transition-group/blob/master/storybook/__mocks__/facade.js

So anyway, just look at the above test code. It's clearly an elegant interface to just be able to return your story component from your it tests. And perhaps more importantly, it's all really greared for redux. Each sub-storybook/test is geared toward corresponding to one state for the same component. To make this sort of stuff work, there can't be that double-rendering. Currently it doesn't seem to break anything (because you're not actually trying to replace the store) when you see the above error messages, but it's not a good signal. Possibly it does, or at the very least it's bad for developer confidence.

[faceyspacey]

...what it does break is hot reloading on the store. This is more related to the issue of using a redux Provider without using addDecorator. And that won't get fixed with the dispatching actions.clearError, as I've commented out the code. So that's another issue, and I assume one that is not easy to solve as you're obviously re-rendering the whole story. I'm hoping there is some real solution, as the only thing I can think of is hacking the standard describe/it interface to also allow you to add a decorator, but that non-standard Storybook-specific stuff is what I'm trying to get out of the tests.

The marketing idea here is big for Storybook. It's: Apply Storybook to your tests overnight. And boom Storybook is 10 times more useful. Any code that is already testing components, taking snapshots of them, etc, but not using Storybook can instantly make use of Storybook just by returning the story component from it calls. HOWEVER, any small amount of additional friction starts to greatly throw that opportunity out the window, as it scares people to mess with their tests, which are so crucial to their products.

[adamellsworth]

Voicing desire for the same: this is a really good use-case and am keen to work on it as well, though not only through the lens of Redux.

[ndelangen]

This sounds amazing 😍 ! I'm hoping someone will be willing to experiment further and possible create a PR?

[faceyspacey]

@adamellsworth @ndelangen here it is:

https://github.com/faceyspacey/jest-storybook-facade

Turn your tests into react storybooks in minutes by applying that and returning your components from tests. Let me know how it works out for you.

[ndelangen]

So I've also done some experimentation with jest & storybook & browsers and experienced it was just not feasible to run real jest in the browser or to rewrite jest for the browser. Jest has a pretty large API surface and is extendable. This will be used, and expected to work.

I love you're using storybook-addon-specifications! @mthuret which is what I've used as well, and things break when you're using more jest-feature other then describe, it etc.

But if we could find a reliable way, that'd be amazing!

[faceyspacey]

check the code, I mocked a lot more of Jest. The API surface isn't that big. We can totally get it up to par. Here are my notes from the readme on further improving the mocks:

https://github.com/faceyspacey/jest-storybook-facade#help-us-improve-storybook-facadejs

here's the jest mock:
https://github.com/faceyspacey/jest-storybook-facade/blob/master/src/storybook-facade.js#L169
the fn implementation.mock stuff needs to be taken all the way, but the starting idea is there and will work for basic use cases--perhaps this is what you're talking about and something you want to improve.

the most valuable thing is that expect looks like the Jest version:
https://github.com/faceyspacey/jest-storybook-facade/blob/master/src/storybook-facade.js#L51
expect.not works too.

[ndelangen]

Hey @faceyspacey think you could open a PR fixing this?

[faceyspacey]

I've been meaning to get to this and invest serious time. I have quite a serious agenda in this area. But I'm a month off Id say.

[AllenFang]

HI guys, I face same issue here, is there any progress? or any workaround? thanks a lots!!

[stale]

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 60 days. Thanks!

[stale]

Hey there, it's me again! I am going close this issue to help our maintainers focus on the current development roadmap instead. If the issue mentioned is still a concern, please open a new ticket and mention this old one. Cheers and thanks for using Storybook!

[ach206]

I had this error message too. Apparently, I was missing my import statement in .storybook/preview.js. When I added it in, it resolved this entirely.
import 'storybook-chromatic';
https://www.learnstorybook.com/intro-to-storybook/angular/en/test/
I'm running React not Angular but the link above helped.