Skip to content

Make stories exportable#6538

Closed
jantimon wants to merge 1 commit intostorybookjs:nextfrom
jantimon:feature/exportable-stories
Closed

Make stories exportable#6538
jantimon wants to merge 1 commit intostorybookjs:nextfrom
jantimon:feature/exportable-stories

Conversation

@jantimon
Copy link

This is NOT a feature complete pull request but more a start point for a proposal or discussion.

I would like to to reuse a story from one place at another place:

file1.stories.js

export const stories = storiesOf(..).add('default', ..).add(..)

file2:

import {stories} from './file1.stories.js'
const FancyStory = stories.default // Matches the storyFn from file1.stories.js .add('default', storyFn)
storiesOf(..).add('default', 
  () => (
      <div>  
          some wrapping content
          <FancyStory />
          some more wrapping content
       </div>)
   )

This would allow me to combine multiple stories for example a Logo(with a demo url) and a BreadCrumb (with some demo links) and a MainNavigation (with a lot of demo links) and show a story how all those components would look liked if they are sitting next to each other.

What I did

I created a small helper object to store all storyFn functions once they are added for a given kind.

Would love to know what you think

@vercel
Copy link

vercel bot commented Apr 16, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://monorepo-git-fork-jantimon-feature-exporta.storybook.now.sh

@github-actions
Copy link
Contributor

Fails
🚫

PR is not labeled with one of: ["cleanup","doc-dependencies:update","BREAKING CHANGE","feature request","bug","documentation","maintenance","dependencies:update","dependencies","other"]

Generated by 🚫 dangerJS against aedf7d1

@ndelangen
Copy link
Member

Hey @jantimon thanks for opening this,

We've been working and experimenting with new story formats behind the scenes. Notable work has been done by @tmeasday on a story format based on ES modules, where all named exports are stories.

Also with upcoming docs-mode (@shilman) MDX will become a first-class citizen, and it will likely make it possible for users to define 'stories' directly within the MDX, and also reference existing stories by ID.

Being able to re-use in for example unit-tests and composing stories so you don't have to repeat yourself IS happening.

Also notable is @expe-lbenychou's work to be able to export stories & related files to codesandbox.

@ndelangen
Copy link
Member

@jantimon
Copy link
Author

jantimon commented Apr 16, 2019

Wow 💯 👍

I always thought this will be bad idea but you managed to come up with a really great api!

I have one idea how you could even add better IDE support and readability - it would be an optional layer and could use exactly the same api you proposed behind the scenes.

So instead of writing the following

export default {
  title: 'Core|Scroll',
}
export const story1 = () => (
  <div>🚀</div>
);
story1.title = 'Fancy Rocket';

You would write:

import {storyDefinition, createStory} from 'storybook';
export const default = storyDefinition({ title: 'Core|Scroll' })
export const story1 = createStory('Fancy Rocket', <div>🚀</div>)

The result should be exact the same like without storyDefinition, createStory so people don't have to use it. It would just be a minimal helper to place the title in front of a component and add IDE / typescript support.

Would love to know what you think

So the code for those helpers would be e.g.:

function storyDefinition({title}) { return {title} }
function createStory(title, storyFn) { return Object.assign(storyFn, {title}) }

@codecov
Copy link

codecov bot commented Apr 16, 2019

Codecov Report

Merging #6538 into next will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #6538      +/-   ##
==========================================
+ Coverage   40.77%   40.78%   +<.01%     
==========================================
  Files         616      616              
  Lines        8529     8530       +1     
  Branches      595      595              
==========================================
+ Hits         3478     3479       +1     
  Misses       4959     4959              
  Partials       92       92
Impacted Files Coverage Δ
lib/client-api/src/client_api.js 78.84% <100%> (+0.2%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2887028...aedf7d1. Read the comment docs.

@tmeasday
Copy link
Member

tmeasday commented Apr 17, 2019

@jantimon yes I think @ndelangen and I had a similar idea. You can read this document here which is a little older but has a few other ideas inside: https://gist.github.com/tmeasday/4de20eab47226a870ab1025642ba848c

The biggest thing I am still trying to resolve is how to reuse data between stories (which sound similar to what you have been thinking about). I'm not sure it is totally resolved yet.

Right now in the Storybook codebase I've done so via a slightly awkward .storyData prop:

// in https://github.com/storybooks/storybook/blob/next/lib/ui/src/components/sidebar/SidebarHeading.stories.js

const menuItems = [
  { title: 'Menu Item 1', onClick: action('onActivateMenuItem') },
  { title: 'Menu Item 2', onClick: action('onActivateMenuItem') },
  { title: 'Menu Item 3', onClick: action('onActivateMenuItem') },
];

export const standard = () => (
  <ThemeProvider /* snip */>
    <SidebarHeading menu={menuItems} />
  </ThemeProvider>
);
standard.storyData = { menu: menuItems };

// in https://github.com/storybooks/storybook/blob/next/lib/ui/src/components/sidebar/Sidebar.stories.js

const { menu } = SidebarHeadingStories.standard.storyData;
const { stories, storyId } = SidebarStoriesStories.withRoot.storyData;

export const simple = () => <Sidebar menu={menu} stories={stories} storyId={storyId} />;
simple.storyData = { menu, stories, storyId };

It's awkward because there is duplication of the key menu and mistakes could easily be made (ie. the actual menu prop ends up being slightly different to the exported storyData). I would rather make it automatic somehow. One PR I am planning on sending @ndelangen (sorry!) looks something like:

// in SidebarHeading.stories.js

const menuItems = [
  { title: 'Menu Item 1', onClick: action('onActivateMenuItem') },
  { title: 'Menu Item 2', onClick: action('onActivateMenuItem') },
  { title: 'Menu Item 3', onClick: action('onActivateMenuItem') },
];

export const standard = () => (
  <ThemeProvider /* snip */>
    <SidebarHeading menu={menuItems} />
  </ThemeProvider>
);

// in Sidebar.stories.js

// Actually evaluate the story and read the props off the React element!
// (Note it's already a bit awkward because of a wrapper)
const { menu } = SidebarHeadingStories.standard().children.props;
// etc
export const simple = () => <Sidebar menu={menu} stories={stories} storyId={storyId} />;

@jantimon
Copy link
Author

jantimon commented Apr 17, 2019

Hey I added feedback to your gist and created a small proof of concept which is very similar to the current api. (it only removes the chainability)

https://codesandbox.io/s/6owq9351w

export const story = storiesOf('Demo');
// Exported components:
export const Component = story.add('Component', () => <div>Component</div>);
export const Component2 = story.add('Component2', () => <div>Component</div>);
// Not exported components:
story.add('Component2', () => <div>Component</div>)
// Exported non storybook related data 
export const data = { foo: 'bar' }

Please let me know what you think :)

@tmeasday
Copy link
Member

Hey @jantimon I commented on the gist

@ndelangen
Copy link
Member

@tmeasday what's the next step for this PR?

@tmeasday
Copy link
Member

I think we can close this PR and continue discussion on the gist. I don't seem to get notification of gist changes however so perhaps @jantimon you could comment here if you make another comment (I am about to comment now)

@tmeasday tmeasday closed this Apr 29, 2019
@jantimon
Copy link
Author

Sorry for the delay - I finally found some time to update the gist

@jantimon
Copy link
Author

@tmeasday
Copy link
Member

HI @jantimon -- replied

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants