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

I love this addon--it needs an argument to skip tree levels to determine component to grab propTypes from #949

Closed
shilman opened this issue Apr 22, 2017 · 9 comments

Comments

@shilman
Copy link
Member

shilman commented Apr 22, 2017

Issue by faceyspacey
Monday Feb 27, 2017 at 01:34 GMT
Originally opened as storybook-eol/react-storybook-addon-info#138


So if you're wrapping your component in a Provider such as the one from Redux, your top level component is now 1 level from the top, and you won't see its propTypes on the info page.

It would be very nice if it at the very least had automatic detection of such a Provider component. Ideally though, it also has an option to specify levels from top, perhaps in the last argument to addWithInfo.

That's all.

I know React Story book was originally intended for presentational work, but it makes a damn good "state and key area" browser for a real application. I.e. you can setup key areas you want to focus on, and return to it with no setup. The idea of just doing presentational work in Storybook greatly limits its usage, as eventually it becomes a maintenance burden to maintain 2 apps: one with raw components and one with connected containers. I only use it with connected containers and the Redux Provider.

So often my "key areas" are nested connected components, not just the whole app. And I'd like to see in the info page, not just the React Redux Provider prop types. What do you think?

@shilman
Copy link
Member Author

shilman commented Apr 22, 2017

Comment by UsulPro
Monday Feb 27, 2017 at 06:43 GMT


Hi!
I haven't personally used storybook with redux, but I think it's possible to put the provider in a storybbok decorator. Have you tried to do so? At least you'll get access to components of 1 level.
I'm not sure what can be done with the components wrapped in select, it seems there should be some kind of a special solution

@shilman
Copy link
Member Author

shilman commented Apr 22, 2017

Comment by faceyspacey
Monday Feb 27, 2017 at 06:58 GMT


I've used the decorator feature and it works. The only thing is I can't use that for what I'm trying to do. I've made it so the specifications add-on works with regular describe/it tests if you simply return a story component from your it calls. It's very nice and allows you to write your tests as normal, which is a big win in terms of confidence.

How exactly do you extract the propTypes? Are you not able to parse the tree in your code and you're saying it must be done in Storybook?

@shilman
Copy link
Member Author

shilman commented Apr 22, 2017

Comment by UsulPro
Monday Feb 27, 2017 at 08:34 GMT


Ok, if I understand you correctly, the problem with decorator is that you need to pass your story to storybook-addon-specifications via let output = mount(story);
Sorry if I'm wrong and maybe there should be a better solutions, but as a workaround you could try something like that:

const mountWithRedux = (store, story) => (
  <Provider store={store}>
    {story}
  </Provider>
)

and then you can pass the same store as in decorator:

import myStore from './myStore';

/* ... */

stories.addDecorator(reduxProvider(myStore))

/* ... */

stories.addWithInfo('Hello World', function () {
  const story = <MyHelloWorld />

  specs(() => describe('Hello World', function () {
    it('Should have the Hello World label', function () {
      let output = mountWithRedux(myStore, story);
      expect(output.text()).toContain('Hello World');
    });
  }));

  return story;
});

@shilman
Copy link
Member Author

shilman commented Apr 22, 2017

Comment by faceyspacey
Monday Feb 27, 2017 at 08:44 GMT


The decorator will work, but I have an interface that allows the following:

describe('AnimatedTransitionGroup 1', () => {
  it('blue', () => {
    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
  })

  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
  })

  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
  })
})

I made the mocks differently so that it allows me to keep my tests almost the same, minus the harmless task of returning a react component from your it calls.

So the decorator is a no go since each test will have a different store. There are other less optimal options such as creating the store before describe and passing it as a final parameter to describe, but then you end up with the same store across tests.

There is an even more challenging issue--hot module replacement doesn't work within the tests. It works within imported react components, but not the actual file containing the above tests/stories (again, the test doubles as a story in my setup).

On a side note, HMR works for for your Storybook spec tests though, just not what you see on the page.

I did some hacks with addDecorator and passing a store to describe and that didn't work. I actually think only the global addDecorator will allow for HMR to continue working with the Redux provider--i.e. stories.addDecorator() I don't think even works (even without my complicated scenario). The decorator at the stories level is called every time, and as React replaces the <Provider> element at the top of the tree, you get the issue where you can't hot module replace that way. It seems only the global decorator is setup once. I could be wrong though, but even when I tried to make my setup rely on the stories level decorator through hacks that ruined the standard describe/it block structure, it didn't work. Perhaps, if i tried outside of my setup, but then that defeats the goal of what i'm trying to do, which is essentially make turning your component tests into storybooks a one hour affair (i.e. just add the component returns, and proper mocks at the top of the file).

So forgetting HMR, I think the only way to make the propTypes works is to drilldown past the provider in your code. I can do the work if you can point me into the right direction. I just have to hope that the propTypes are accessible when parsing the tree. I assume they should be somewhere.

@shilman
Copy link
Member Author

shilman commented Apr 22, 2017

Comment by faceyspacey
Monday Feb 27, 2017 at 08:50 GMT


If you're interested in seeing what I'm up to, just paste these commands:

git clone [email protected]:faceyspacey/animated-transition-group.git
cd animated-transition-group
yarn
npm run test // just to see that the tests are working
npm run storybook 

Then check these files:

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

...I think i got an awesome thing in the works here. I mean it WORKS. I'm just optimizing minor aspects such as the propTypes info and HMR within test files that contain redux stores. HMR in test files not using redux does hot module replace in this setup (and again, HMR does work if you edit react components directly). So I'm being nit-picky at this point to finalize the concept.

...I'm using Wallaby by the way. Check it out if you haven't already (http://wallabyjs.com). So in addition to having my stories and tests combined with HMR for most of it, I have HMR for all my tests with inline logging, logging within the wallaby web panel, beautiful snapshot diffing in the web panel, and one-click individual snapshot updating. I'm on the verge of the most beautiful workflow I've ever had.

@shilman
Copy link
Member Author

shilman commented Apr 22, 2017

Comment by faceyspacey
Monday Feb 27, 2017 at 08:55 GMT


check this:

https://twitter.com/wallabyjs/status/827059959728795648

notice the camera icon in the top right of the diff.

Wallaby runs your tests every character you type and provides all sorts of indicators of how the tests are performing and anything you log in realtime.

@ndelangen
Copy link
Member

I'll consider adding some api to define the actual component that the story is about.
#1209 Please pitch in how! 👍

I'll leave this issue open for that.

@stale
Copy link

stale bot commented Oct 31, 2017

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. We do try to do some housekeeping every once in a while so inactive issues will get closed after 90 days. Thanks!

@stale stale bot added the inactive label Oct 31, 2017
@stale
Copy link

stale bot commented Nov 2, 2017

Hey there, it's me again! I am going to help our maintainers close this issue so they can focus on development efforts 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!

@stale stale bot closed this as completed Nov 2, 2017
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

2 participants