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

React: Add renderStory export #11478

Closed
wants to merge 5 commits into from
Closed

React: Add renderStory export #11478

wants to merge 5 commits into from

Conversation

tmeasday
Copy link
Member

@tmeasday tmeasday commented Jul 9, 2020

Issue: we want to make it easy to use CSF exports in testing frameworks. This adds it for React.

What I did

Added @storybook/react/render with a renderStory export. You can use as per the test.

How to test

  • Is this testable with Jest or Chromatic screenshots?

Yes, there is a test.

@tmeasday tmeasday requested a review from shilman July 9, 2020 07:48
@shilman shilman changed the title Added a renderStory export to the react framework React: Add renderStory export Jul 9, 2020
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

const React = require('react');

module.exports = {
renderStory(Story) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if I want to add extra args? Consider the following example:

import { render, fireEvent } from '@testing-library/react';
import { Text } from './Button.stories';
it('should respond to click events', () => {
  const handleClick = jest.fn();
  const instance = render(
    <Text {...Text.args} onClick={handleClick} />
  );
  fireEvent.click(instance.container.firstChild);
  expect(handleClick).toHaveBeenCalled();
});

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you think it should be renderStory(Story, args?)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that makes more sense to me

app/react/src/__tests__/render.test.ts Outdated Show resolved Hide resolved
tmeasday and others added 3 commits July 10, 2020 11:06
Co-authored-by: Michael Shilman <shilman@users.noreply.github.com>
@tmeasday
Copy link
Member Author

@shilman I've realized some things about this that make it not quite bulletproof:

  1. If the story has component-level args they will not be set.
  2. If a story has defaults in argTypes they will not be set.

The second problem could be fix I suppose. The first is trickier. WDYT?

@shilman
Copy link
Member

shilman commented Jul 10, 2020

@tmeasday Oof. Both good points.

We went over something similar with decorators here: #10145

Maybe the API could be:

renderStory(story, options);

Where options can optionally contain:

  • args
  • argTypes
  • decorators

@tmeasday
Copy link
Member Author

I’m not sure what we do with decorators TBH.

Would one of those options be component (ie the default export?)

@tmeasday
Copy link
Member Author

tmeasday commented Jul 11, 2020

Hmm, I think we over-simplified this and should have remembered the earlier conversation with @lifeiscontent.

There's a lot to think about here in terms of reliably rendering stories in a external context:

  • Args + ArgTypes: gathering args from the story + component, pulling defaults from argTypes (from both)
  • Decorators: gathering from global, component + story; applying them to to the story; excluding ones that you don't want for tests; how to deal with preset-added decorators?
  • Parameters: similarly these likely are needed for some decorators to function and more or less the same challenges apply as for decorators.

I guess this PR is a first step that only deals with simple args (at this stage). Here's my strawman:

  1. Lets update this PR to also
    a) deal with argType defaults
    b) optionally take component

  2. Let's also document (in the testing section where we use this) the pattern to create a custom "decorate" story function for your project. We can cover off all the complicated stuff above with an example.

  3. In the future we can make utils to do 2. easier and consider adding that Jest transform (as well as versions of this for other frameworks).

WDYT @shilman?

@yannbf
Copy link
Member

yannbf commented Jul 11, 2020

This might be a silly question but is renderStory the best name for this? It doesn't actually render the story, it returns the story element, right? So when I'm using in my tests:
render(renderStory...) couldn't this be a bit confusing?

@yannbf
Copy link
Member

yannbf commented Jul 11, 2020

Also one great benefit of using a component in JSX form with typescript is that you can do something like:

import { Button } from './stories/Button'
const { getByText } = render(<Button>learn react</Button>) // e.g. error here, missing onClick

Then both you get autocompletion for props but you get error for missing mandatory props.
Testing the component that way is possible but of course there's more effort into writing a wrapper function to add the decorators.

In the renderStory scenario, it might be possible to make it easier to pass all kinds of configuration and to give the same TS benefits for component props, although I'm trying hard and I can't seem to make it work 😂

Just trying to bring this to the table, although this will be something for the future.

@tmeasday
Copy link
Member Author

Maybe what we want is a function to turn a story into something renderable? Ideally that’d be the export in the first place but all the complexities of args, decorators + parameters makes that sort of impossible.

So something like:

import storyToComponent from @storybook/react/render’;
import { WithText } from ./button.stories’;

const WithTextComponent = storyToComponent(WithText);

render(<WithTextComponent prop={foo}>other text</WithTextComponent>);

@yannbf
Copy link
Member

yannbf commented Jul 12, 2020

Maybe what we want is a function to turn a story into something renderable? Ideally that’d be the export in the first place but all the complexities of args, decorators + parameters makes that sort of impossible.

So something like:

import storyToComponent from @storybook/react/render’;
import { WithText } from ./button.stories’;

const WithTextComponent = storyToComponent(WithText);

render(<WithTextComponent prop={foo}>other text</WithTextComponent>);

Although that'd be nice, it's not really possible, right? If the component returned is a result of bunch of decorators wrapping the story, you wouldn't be able to easily pass the props and children like <WithTextComponent prop={foo}>other text</WithTextComponent>. Or am I missing something?

@stale
Copy link

stale bot commented Aug 2, 2020

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. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Aug 2, 2020
@stale
Copy link

stale bot commented Sep 2, 2020

Hey there, it's me again! I am going close this issue to help our maintainers focus on the current development roadmap instead. If the issue mentioned is still a concern, please open a new ticket and mention this old one. Cheers and thanks for using Storybook!

@stale stale bot closed this Sep 2, 2020
@stof stof deleted the export-render-from-react branch May 25, 2022 09:30
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.

None yet

3 participants