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

JSX syntax addon #1006

Closed
theinterned opened this issue May 9, 2017 · 13 comments
Closed

JSX syntax addon #1006

theinterned opened this issue May 9, 2017 · 13 comments

Comments

@theinterned
Copy link
Member

A more natural api for storybooks might be to adopt a set of react components and JSX syntax and abandon the storiesOf and add functions in place of a react component-oriented design.

For a story about a component called MyComponent the old story would look like:

storiesOf('My Component')
  .add(
    'story title', 
    () => <MyComponentToTest />,
    { myOtherOption: {foo: bar,  ... } }
  );

I would propose that could be expressed like so:

<StoriesOf title="My Component">
  <Story title="story title">
    <MyComponentToTest />
  </Story>
</StoriesOf>

... I would maybe refine the language at the same time and call the <StoriesOf /> component a <Chapter /> instead. That would look like this:

<Chapter title="My Component">
  <Story title="story title" myOtherOption={{foo: bar,  ... }} >
    <MyComponentToTest />
  </Story>
</Chapter>
@theinterned
Copy link
Member Author

Some related issues that could be solved with a react-component-based api:

#993 Allow passing options to storiesOf().add() as hook to customizing story behaviour in plugins

(This is the issue I created that lead me to consider this JSX approach)

<Chapter title="My Component">
  <Story 
    title="My story" 
    props="are options" 
    so="anything can go here" 
    myOtherOption={{foo: bar,  ... }} 
  >
    <MyComponentToTest />
  </Story>
</Chapter>

#881 Allow passing options to react-test-renderer .create() method

<Chapter title="My Component">
 <Story title="My story" createNodeMock={node => { ... }} > ... </Story>
</Chapter>

#151 Substories/Hierarchy

<Chapter title="Grandparent category">
  <Chapter title="Parent category">
    <Chapter title="Child category">
      <Story title="Story" createNodeMock={node => { ... }} > ... </Story>
    </Chapter>
  </Chapter>
</Chapter>

#58 Toggle global settings

<Chapter title="Chapter">
 <Story title="Story" createNodeMock={node => { ... }} >
    <ThemeToggle> ... </ThemeToggle>
  </Story>
</Chapter>

And probably a whole host of other problems

@theinterned
Copy link
Member Author

theinterned commented May 9, 2017

This is sort of how I imagine this would work. Given the following JSX in two sepeate files:

// fileOne.story.js
<Chapter title="Parent Category">
  <Chapter title="Child Category">
    <Story title="Story title">
      <MyComponentToTest>Story One content</<MyComponentToTest>>
    </Story>
  </Chapter>
</Chapter>

//fileTwo.story.js
<Chapter title="Parent Category">
  <Chapter title="Child Category">
    <Story title="Another story">
      <MyComponentToTest someState={true}>Story Two content</<MyComponentToTest>>
    </Story>
  </Chapter>
</Chapter>

You would get this output.

screen shot 2017-05-09 at 9 58 10 am

Any Chapter rendered would become a tab in the left-side panel. and TheStory would render to the main panel.

So the Chapter would act sort of like Route component from react-router. And all the title props would be merged to create the left-panel menu hierarchy.

@shilman
Copy link
Member

shilman commented May 9, 2017

I really love this collection of ideas. Some questions or things to consider:

  • How would decorators work under this scheme? Global addons?
  • In current storybooks, each story is implemented as a function () => <SomeComponent />, so if you have a lot of stories it only executes the currently-selected story (as I understand it). Is it possible to retain that behavior in this proposed setup?

@theinterned
Copy link
Member Author

@shilman hmm ... @tmeasday brought up the "story is implemented as a function" issue. I'm not sure of all the values that paradigm provides vs. the "story implemented as props.children" paradigm in this proposal.

I'd love to hear some of the benefits.

Would something like the "Function as Children" pattern address this?

@theinterned
Copy link
Member Author

There was some great discussion about the idea of JSX syntax on #993. I don't know how to move that discussion over here so I will link to it in this comment for posterity.

The Discussion starts with @tmeasday comment #993 (comment)

@tmeasday
Copy link
Member

tmeasday commented May 10, 2017

@theinterned: I'm not sure what the reason for the requirement that a story is a function as implemented, but I would guess you'd pretty quickly run into the reason if you tried to implement this (is it because React doesn't like it if you re-use elements? On the surface it doesn't complain). Certainly it's easily solved with the "function as children" pattern, although I suspect that takes away from some of the elegance of the JSX approach.

only executes the currently-selected story

@shilman: What do you mean by "executes" here? I don't think creating an element (i.e. writing <Foo bar="baz"/>) does much beyond boxing Foo with its props; it's only when you actually render the element that your code starts to run. I'm not an expert though.


@shilman re: decorators; I think you can get a nice syntax for them like so:

<Chapter for={component}>
  <div className="this-is-a-decorator-for-all-stories-in-the-chapter">
     <Story>...</Story>
     <Story>...</Story>
  </div>
</Chapter>

Although I'm not sure that's compatible with @theinterned's idea to render the Chapter in the menu as well as the preview pane. I also suspect that might not end up working anyway though: I'd be interested to know a little more about how it could work @theinterned.

Another question: if we are going to make this a layer on the existing API, we'll need to programmatically get a list of stories for a chapter. How do we do that? Collect the children of the <Chapter> element and turn each <Story> into a .add() call?

@tmeasday
Copy link
Member

@theinterned on this subject:

I think the value of composition is well expressed by @usulpro in slack: it makes it easy to solve problems like this: #766 because the add-on could be a component rather than a plugin.

Maybe I'm missing the insight here, but I'm not seeing how the JSX syntax really helps here.

I don't think that in general you can just combine two addons that have custom add behaviour and expect it to make sense. If you think about that example, it only makes sense in one direction: add "info" to the special "combination" story. Trying to add combinations to the "info"-ed story wouldn't work.

To me this just highlights that we should make it easier for addons to be less "destructive". If the JSX mode allows that then great, but I'm not sure it does in either case here.

  1. The info addon should just use metadata, which we should add via your Allow passing options to storiesOf().add() as hook to customizing story behaviour in plugins #993
  2. The combinations addon should just export a MakePropCombinations component (bad name!), IMO:
storiesOf(...)
  .add('foo', () => (
    <MakePropCombinations options={existingOptions}>
      {(props) => <MyComponent {...props} />}
     </MakePropCombinations>
  ));

(Or it could use a HoC ;) )

Of course this would be nicer in the JSX syntax! But it doesn't require it. [Sorry if it sounds like I am being difficult, I just want to be clear about what the advantages of JSX are].

@tmeasday
Copy link
Member

tmeasday commented May 10, 2017

It would be a different story if that addon was calling .add() more than once (which I initially thought it was doing). In that case I think maybe the JSX syntax can be less destructive:

<MakeStoryCombinations options={existingOptions}>
  {(props) => <MyComponent {...props} />}
</MakeStoryCombinations >

// could be equivalent to
<Story name="foo-1"><MyComponent {...props1}/></Story>
<Story name="foo-2"><MyComponent {...props2}/></Story>

Of course it wouldn't be hard for the addon to have a functional API that's equivalent, but it's sort of ugly:

const chapter = storiesOf('MyComponent');

addStoryCombinations(chapter, options, (props) => <MyComponent {...props} />);

@theinterned
Copy link
Member Author

theinterned commented May 10, 2017

@tmeasday great thoughts here! One thing that I thought of reading through this: to decorate a bunch of stories in a chapter as per your example #1006 (comment)

You could decorate a "container" for the chapter as follows:

const DecoratedChapter = ({ for, Decorator, children }) => (
  <Chapter for={for}>
    { children.map( child =>
      <Decorator>
        { child }
      </Decorator>
    ) }
  <Chapter>
);

And then use it in a story file like:

<DecoratedChapter for={Component} Decorator={SomeDecorator}>
     <Story>...</Story>
     <Story>...</Story>
</Chapter>

I'm not sure if I'm putting the Decorator inthe right place in that implementation, but I'm sure you get the idea

@usulpro
Copy link
Member

usulpro commented May 14, 2017

@theinterned thanks for the idea of such addon!

it turns out that other addons (internal and third-party) will also have to support this syntax. So it could look like this:

import StoryWithInfo from '@storybook/addon-info';
import Chapter from `storybook-chapters`;

<StoriesOf title="My App">
  <Chapter title="My Component">
    <StoryWithInfo title="story title">
      <MyComponentToTest />
    </Story>
  </Chapter>
</StoriesOf>

@ndelangen
Copy link
Member

Sorry, but I don't think is going to make it into storybook mate, supporting 2 very different methods of configuring would be really bad long term.

I like the idea in concept, but it ties Storybook to much to react.

@theinterned
Copy link
Member Author

Thanks @ndelangen. It was an idea ahead of its time 👨‍🚀

@oyeanuj
Copy link

oyeanuj commented Oct 7, 2017

@ndelangen @usulpro @shilman @tmeasday I know this issue is closed but incase the team would be open to it in v4, just wanted to add one more perspective.

As someone new to the library, as I was looking at Storybook, understanding its syntax and API methods actually took a while. I feel like being able to compose stories and chapters like normal JSX components would really be a huge win for readability and scannability.

<Stories title = "Page title">
  <Chapter .. >
    <Story >
      <MyComponent />
    </Story>
  </Chapter>
</Stories>

FWIW, something I've really liked about catalog(https://github.com/interactivethings/catalog) is that it gives you the option to either use their non-React syntax or use React components - https://docs.catalog.style/guides/react, in which case, React components are just sugar for the other methods.

It'd be super cool if Storybook could consider doing that for 'Storybook for React' in v4, so that using storybook feels more natural within React.

Thank you for reading this, thankful for your work but just wanted to put out my $0.02!

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

6 participants