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

Adding stories from data structure throws error #112

Closed
shilman opened this issue Apr 14, 2016 · 8 comments
Closed

Adding stories from data structure throws error #112

shilman opened this issue Apr 14, 2016 · 8 comments

Comments

@shilman
Copy link
Member

shilman commented Apr 14, 2016

I have a working set of stories from the sample code. However, when I try to populate stories from a simple data structure, storybook gives me an error. Loading stories from data would keep code DRY when you are sharing fixture data between stories and unit tests.

When I modify the storybook example components/stories/TodoItem.js with the following trivial change, it gives me the following error preview.js?a230:61 Uncaught TypeError: story is not a function. Is there a better way to do this?

const data = {
  'not completed': { id: 'the-id', text: 'Hello Todo', completed: false },
  'completed': { id: 'the-id', text: 'Hello Todo', completed: true }
}

const todoStories = storiesOf('TodoItem', module)
for (const key in data) {
  todoStories.add(key, getItem(data[key]))
}

// storiesOf('TodoItem', module)
//   .add('not completed', () => {
//     const todo = {
//       id: 'the-id',
//       text: 'Hello Todo',
//       completed: false
//     };
//
//     return getItem(todo);
//   })
//   .add('completed', () => {
//     const todo = {
//       id: 'the-id',
//       text: 'Hello Todo',
//       completed: true
//     };
//
//     return getItem(todo);
//   });
@shilman shilman changed the title Adding stories from data structure? Adding stories from data structure throws error Apr 14, 2016
@thani-sh
Copy link
Contributor

Hi @shilman,
I believe the code given above will work fine if we use an arrow function.

const todoStories = storiesOf('TodoItem', module)
for (const key in data) {
  todoStories.add(key, () => getItem(data[key]))
}

React storybook can be modified to support data structures by changing these 2 lines.

But using a function to create the component can be useful. Let's keep the issue open for now.

@shilman
Copy link
Member Author

shilman commented Apr 14, 2016

@mnmtanish You're correct, it's just a bug in my code, not a problem with storybook. Thanks for the quick response and solution! I think we can close the issue.

@thani-sh
Copy link
Contributor

You're welcome 😃

@shilman
Copy link
Member Author

shilman commented Apr 14, 2016

@mnmtanish Could be a convenience function perhaps, something like todoStories.addAll(data)? Not sure if it's a good idea, but it's a thought 😄

@jeef3
Copy link
Contributor

jeef3 commented Apr 14, 2016

Could be a convenience function perhaps, something like todoStories.addAll(data)?

I was thinking this as well. The only problem though is that currently the add() story requires you to pass a function that does the rendering. addAll could only work like you've described if a Story was set-up knowing what component it would render, and then only allowed you to pass props.

Perhaps something like:

todoStories.addAll(data, (props) => <TodoItem {...props} />);

Where the render function of addAll passes the props each time. Could actually work quite well with functional components:

// functional component
const TodoItem = ({ id, text, completed }) => <div>{id}: {text} {completed ? 'Y' : 'N'}</div>;

// ... other set-up

// Since it would be called, given the props.
todoStories.addAll(data, TodoItem);

@thani-sh thani-sh reopened this Apr 14, 2016
@jeef3
Copy link
Contributor

jeef3 commented Apr 14, 2016

This looks great. Mind if I submit a PR, to practice my "open-sourcery"?

I don't think anyone will stop you 😄

One thing I notice though is that add(kind, fn) requires you to give it a name (kind) and a render function that does all the work. Where this proposed addAll(data, fn) requires a kind/props pair, with a render function that accepts props. While it might work well, add and addAll work quite differently, so it might be weird that they are named so similarly.

@shilman
Copy link
Member Author

shilman commented Apr 14, 2016

Looking into the code it's a non-trivial change. I think it's better implemented as a convenience function external to the library. This works for me for now:

function addAll(stories, storyData, componentFn) {
  Object.keys(storyData).forEach((storyName) => {
    stories.add(storyName, () => componentFn(storyData[storyName]))
  })
}
addAll(todoStories, todoData, TodoItem)

Though I guess this only works for functional components. Anybody know how to write a generic function to render a class-based component?

ndelangen pushed a commit that referenced this issue Apr 5, 2017
Extend React Native versions range to 0.37
@ndelangen
Copy link
Member

ndelangen commented Jul 7, 2017

todoStories.addAll(data, TodoItem);

#1209 Will be considered for api-v2

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

5 participants