Addon-docs: Exclude decorators in dynamic source snippets

[lauracarballo]

Issues: #12022 #11542

What I did

The dynamic code snippet is generated by a decorator called jsxDecorator which is included in kindMetadata.decorators.

This decorator runs after the story decorators provided by the developer, which normally include i18n providers, data fetching or visual styling that shouldn't be shown in the code snippet.

The order of the decorators can be seen here:
https://github.com/storybookjs/storybook/blob/next/lib/client-api/src/story_store.ts#L379

Re-ordering the decorators would be the easiest solution, but that seems like it would be breaking change to library due to some addons possibly expecting the output of story decorators.

So, to bypass the story decorators, the solution myself and @jamesb3ll came up with is to use the component from the story context. If the developer has provided a component we can construct the JSX element using React.createElement together with the args as the props. That component is then converted to readable code string using the react-element-to-jsx-string library.

We've updated the tests to reflect this, any feedback on this solution would be great.

[shilman]

Hi @lauracarballo, thanks for the fix! However, I'm a little unclear on how it will work when the component needs contexts provided by the decorators, such as i18n or theme.

[shilman]

One more wrinkle. A story function can contain arbitrary JSX, but this only shows the default.component. If this solution works for components that depend on contexts from decorators, we can probably come up a similar solution that can show the story's JSX.

[lauracarballo]

Hi @shilman, thanks for reviewing the PR. I've updated the cra-kitchen-sink with an example including decorators to showcase what's being solved in this PR in a better way.

Currently decorators are being included inside the code snippet. To my understanding decorators should not be appearing inside the source code snippet since they are something "external" to the original component.

For example:

With the fix provided, the jsxDecorator still returns the original story including decorators but before doing so, we grab the component from the context and use that to generate the code snippet.

This way we end up rendering a story that includes decorators in the Canvas and Doc Blocks but does not include them in the source code snippet.

[ndelangen]

There a difference between these 2 stories? 1 shows a function returning JSX, the other shows just the JSX.

I don't have a strong preference which we do, But I'd prefer it if it was consistent.

[lauracarballo]

Hi @ndelangen, thanks for your reply and for having a look at my PR.

The 2 stories added in decorators.stories.js are to demonstrate how source code snippets render for 2 different styles of stories.
Basic story:

export const Basic = () => <Button onClick={action('clicked', { depth: 1 })}>Basic</Button>;

Args story:

export const WithArgs = (args) => <Button {...args} />;
WithArgs.args = { onClick: action('clicked', { depth: 1 }), children: 'With args' };

According to DocsPage documentation when docs.source.type is the default value of auto the code snippet output will:

Use dynamic snippets if the story is written using Args and the framework supports it.

And otherwise, if not an Args story it will render using code which is the raw story code parsed at build time.

This is why the decorators.stories.js file shows two different outputs: JSX (dynamic) for Args Story and a function returning JSX (code) for a Basic story.

What I'm attempting to fix in this PR is that currently the Args stories code snippets are broken when adding decorators as mentioned in this issue (#12022).

Thanks again for reviewing

[shilman]

Following up on my previous comment, I did a little testing on this branch. This is a fantastic start, but the solution will be a little more involved.

From what I can tell, if a StoryFn relies some context, such as themes provided by a decorator:

• ReactDOM.render(<StoryFn />) needs decorators/context
• <StoryFn /> doesn't need them

This means that we can render JSX for the component without decorators even when we can't render the component, which is great news!

However, this PR as it stands is only accurate when the user's story function is:

const Template = (args) => <Component {...args} />

This is the most common case, but there are plenty of times when this is not the case. For example, it's also common to show compositions of components in a story:

export const Empty = (args) => <List {...args} />;

export const ManyItems = (args) => (
  <List {...args}>
    <ListItem />
    <ListItem />
    <ListItem />
  </List>
);

The first story would render correctly with the changes in this PR but the second story wouldn't.

I think the solution is to render the undecorated story function, which we happen to keep around in the story store. Unfortunately, it's currently not available in the StoryContext at render time.

So I'd like to see a few changes to this:

• Make the undecorated story function available in the story context
• Render JSX from the undecorated story function instead of from the synthetic <Component {...args /> function
• Make this feature either opt-in or opt-out using a story parameter, because I think there are cases where people WANT to see the decorated story. For example, if my story needs a React.Context to work, I might want to show that in the code sample.

I'm happy to help make these changes. What do you think?

[lauracarballo]

Thanks a lot for reviewing. Your solution seems great, I was hesitant to add extra data to the context but having your feedback it seems like the best place to access the original story.

I'm looking into this, I'll get back to you if I need any extra help.

Thanks for the detailed write up and tasks.

[shilman]

Awesome @lauracarballo 💯 -- please ask any q's here or feel free to grab me or any of the maintainers (@ndelangen @tmeasday etc.) on Discord https://discord.gg/storybook. This is going to be really great.

[gabiseabra]

Re-ordering the decorators would be the easiest solution, but that seems like it would be breaking change to library due to some addons possibly expecting the output of story decorators.

Hey, I’m also looking into issue #12022 and from what I gather jsxDecorator is the first entry in the global decorators list (not kind decorators), so it runs immediately after user-defined decorators (kind & story) and before library-defined decorators.

So I don’t understand why re-ordering jsxDecorator would be an issue as long as it keeps running before all other global decorators, only moving "down" the stack by skipping user-defined ones. I may be missing something, can you share any more insights? Which addon would be affected by this?

[nx-cloud]

Nx Cloud Report

CI ran the following commands for commit a5c667f. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status	Command
	nx run-many --target=prepare --all --parallel --max-parallel=15

---

Sent with 💌 from NxCloud.

[lauracarballo]

Hi @shilman, sorry for the delay in the response. I've made updated on the PR based on your previous feedback :)

I have added the originalStoryFn to the context so we can access the original story in the jsxDecorator. Following your advice I have added an extra parameter docs.source.includeDecorators to opt-in or opt-out of showing decorators in the source code snippet.
Note that I have excluded the decorators by default and this could be a breaking change, consequently a lot of the tests are breaking as the context object has been changed. I would like to receive some feedback on this matter before attempting to fix the tests.

Thanks for your patience and guidance

[shilman]

Hi @lauracarballo thanks so much for following up, this is looking great!

How about making it opt-out just to be on the safe side, with excludeDecorators instead of includeDecorators? That way:

1. It won't be a breaking change
2. It won't crash when the stories require decorators to render properly

Since parameters cascade, you could opt-in to this feature globally with one line in .storybook/preview.js. I think after that, it's just a matter of cleaning up in various places to make TS types line up.

What do you think? Storybook 6.3 is going into beta tomorrow, so it would be great if we could wrap this up this week. Happy to do whatever it takes to help get this over the line!

[lauracarballo]

Hi @shilman, thanks for the quick response :)

I have updated the PR to make it opt-in and opt-out using excludeDecorators and I have also set this in .storybook/preview.js in the cra-kitchen-sink example. The test has also been updated for this change.

[shilman]

Awesome!!! 💯 x 💯

[jonniebigodes]

@lauracarballo one additional thing if you're ok with it. If you could jump into our Discord server and message me (same username), I wanted to follow up with you on this pull request.

[lauracarballo]

Hi @jonniebigodes, I messaged you on Discord already. Thanks :)

[raduliviu]

I have updated the PR to make it opt-in and opt-out using excludeDecorators and I have also set this in .storybook/preview.js in the cra-kitchen-sink example. The test has also been updated for this change.

Hi @shilman sorry to revive this thread, but I was curious if this parameters.docs.source.excludeDecorators parameter can be found anywhere in the official Storybook docs?