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

Proposed update to add() #114

Closed
wants to merge 1 commit into from

Conversation

jeef3
Copy link
Contributor

@jeef3 jeef3 commented Apr 14, 2016

This PR is more for discussion than immediate merge as it proposes a change to the main API. This was born out of discussion on #112.

Instead of rendering the component by just executing the render function of the story, we can wrap it in a React.createElement and everything should still work as it currently does:

// Before
return ReactDOM.render(story(), rootEl);

// After
return ReactDOM.render(React.createElement(story), rootEl);

This allows you to pass just a component to the add() function:

// Before:
stories.add('default view', () => <MyComponent />);

// After:
stories.add('default view', MyComponent);

Of course, now we are without props. The API change in this PR adds a third optional argument to the add method, props. These get passed to the createElement allowing you to:

stories.add('default view', MyComponent, { label: 'Component label' });

This paves the way for a matching `addAll`` style API:

stories.addAll(MyComponent, {
  'default view': { label: 'Component label' },
  'another view': { label: 'Another label' }
});

Anyway, as I mentioned, this is more for discussion than immediate merge.

@arunoda
Copy link
Member

arunoda commented Apr 16, 2016

I don't this is a replacement API for add in anyway. Which gives us freedom to do anything inside the function pass to add.

I know this makes the API a bit simpler for many cases. I believe in APIs with providing only one way to do things. (No idea, that's a thing yet)

stories.add('default view', () => <MyComponent />);

Above example will work for most of the case. With arrow functions, I don't think it's not ugly too.


Anyway, I think the proposed API are good too in some cases. So, why don't we create a new NPM module with these.

@jeef3
Copy link
Contributor Author

jeef3 commented Apr 17, 2016

Anyway, I think the proposed API are good too in some cases. So, why don't we create a new NPM module with these.

Good idea. Fits nicely with the addition of #115 as well. I can see a series of storybook-* plugins coming 😄

@jeef3 jeef3 closed this Apr 17, 2016
@jeef3 jeef3 deleted the add-accept-components branch April 17, 2016 22:06
@arunoda
Copy link
Member

arunoda commented Apr 18, 2016

@jeef3 sweet.

Copy link

nx-cloud bot commented Apr 24, 2024

View your CI Pipeline Execution ↗ for commit 15dfcc6.

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 1m 4s View ↗

☁️ Nx Cloud last updated this comment at 2025-01-06 22:06:20 UTC

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.

3 participants