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

Addon API: proposal #1212

Closed
wcastand opened this issue Jun 7, 2017 · 6 comments
Closed

Addon API: proposal #1212

wcastand opened this issue Jun 7, 2017 · 6 comments

Comments

@wcastand
Copy link
Contributor

wcastand commented Jun 7, 2017

Hey everyone, i'll try to make a resume(we talked a lot) of the conversation in the #addon channel, do not hesitate to add your thoughts and comments about the idea talked here or new one :)

@tmeasday made a quite overview about addons that resume the thing quite well i think, so i think it's a good start :

There are basically three different kinds of "decorators" that you might want.
Depending on where in the "composition" of the story they live:

  • Decorators that alter the story in a real way (think knobs, or the prop-generator one).
    Really, these are classic HOCs

  • Decorators that want to inspect the "real" story and report the result somewhere
    (typically to the channel but it could be somewhere else i guess) (info, jsx, etc).
    These have been called "middleware" although I think "inspectors" or "connectors" or "probes" would be more descriptive.

  • Decorators that provide some kind of rendering context to the story, be it a container class, fonts/bg colors, or react context (i.e. providers).
    Some of that is visual, some of it is needed for the story to even execute, but it is not "part" of the story

Some goals of the new/reworked API are :

  • Make a clear and simple API to create addon
  • Make addon clear about their use (type of decorators describe above)
  • Avoid the current problem with the overwrite of .add() by addons like infos on jsx (the main is that they can't be use together)
  • Avoid the current pattern where addons "pollute" the stories
  • Create "best practices" and "guides" about creating addons for the community
  • Respond to the needs from the developer (addons and storybooks evolves and we face new challenges like a playground for example)

You can find some gist/branches of proposed API below (some are more ideas rather than concept, but i think it's worth listing them here anyway) :

@wcastand
Copy link
Contributor Author

wcastand commented Jun 7, 2017

For my proposals (1 and 2)
the idea behind is this one (comes from slack channel conversation too):

An other reason to separate decorators and "middleware"(addons like infos, jsx, ...)
decorators modify or provide context to your component and allow them to render.
Your want them applied in the right order.

Middleware are side effect for storybook, they do not need to alter your story, only get informations. They should receive the story rendered with the decorators and that's it. Each middleware should receive the same story.

This what i mean by pollute the story (it return the story with new JSX wrapping your story) :

export default story => {
    return (
        <Provider renderer={getRenderer()} mountNode={getMountNode()}>
            <ThemeProvider theme={theme}>
                <Container>
                    {story()}
                </Container>
            </ThemeProvider>
        </Provider>
    );
};

// OR storybook addon

import React from 'react';

import { storiesOf } from '@storybook/react';
import { WithNotes } from '@storybook/addon-notes';

import Component from './Component';

storiesOf('Component', module)
  .add('with some emoji', () => (
    <WithNotes notes={'A very simple component'}>
      <Component></Component>
    </WithNotes>
  ));

the problem with this is the fact that addons like storyshots will render the decorator in the snapshot and not just the story.

If we use decorators/connectors we can avoid that by sending the final story to the connectors without the extra code from other connectors.

@wcastand
Copy link
Contributor Author

wcastand commented Jun 7, 2017

One of the text from the conversation that's worth mentioning and keep a trace of from @ndelangen


So.. I want to move away from using context at some point.. by making connectors responsible for modifying the props-object, and decorators for just wrapping things for decorative purposes, I think this will become quite easy to do.

Both decorators and connectors can have connections to panels.
Decorators should be renderable things (class extending from components, or just a function)
Connectors should be pure functions (possibly async)

Decorators cannot set props on a story. (this is actually current behaviour!) But this is circumvented by using context.

So the use case for connectors is to provide a clean and robust way to provide props (over time) to a story.

As a bonus we get less components rendered in the preview, possibly making debugging easier for users.

This compose is just a reduce over all functions… applying them in order… Which is exactly what we currently do. Am I missing something or is this just a alternative-method for setting which connectors/decorators a user wants on a story?
I think an array will be a whole lot LESS confusing to people. It’s very similar to how webpack is configured.

Here's a possible api:

storybook.add({
  decorators: ['centered', Sparkles],
  connectors: ['redux', props => ({...props, isAmazing: true, })],
  ...
});

stories = storiesBy('name', {
  decorators: ['centered', Sparkles],
  connectors: ['redux', props => ({...props, isAmazing: true, })],
  ...
});

stories.add('name', props => <MyComponent {...props} /> , {
  decorators: ['centered', Sparkles],
  connectors: ['redux', props => ({...props, isAmazing: true, })],
  ...
});

if item === string, we’ll do a lookup into registered decorators/connectors


@tmeasday
Copy link
Member

tmeasday commented Jun 8, 2017

Here's a pretty simple proposal that moves the onus of categorizing and ordering decorators onto the addon author and avoids the user having to understand what the different types are and even think to much about the order they apply:

https://gist.github.com/tmeasday/775c5572271121bb2a71153ac5f2cd7a

@Stupidism
Copy link
Contributor

Stupidism commented Jun 19, 2017

Hi, I'd like to present my proposal. It is compatible with current api. As I can see, it meets all the requirements listed above and has more capabilities.
https://gist.github.com/Stupidism/4f40f3a9688cb09b3a7cb995831d3be9

@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!

@danielduan
Copy link
Member

Are we still debating this proposal? If not, let's close this and open a new issue for new API proposals.

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