Storybook: Addon-docs: Support non-story exports

[yarastqt]

Describe the bug
When im export my component with any HOC's, then in sidebar i'm getting unknown story with export name.

Screenshots

Code snippets

import { Preview, Story, Meta } from '@storybook/addon-docs/blocks';

export const Spin = enhance(SpinPresenter);

<Meta title="Spin/@desktop" />

# Spin

## Examples

### _size

<Preview>
    <Story name="_size">
        <Spin size="l" />
    </Story>
</Preview>

System:

• Framework: react
• Addons: @storybook/addon-docs
• Version: 5.2.0-alpha.21

[shilman]

@yarastqt Yeah.. that needs to be documented. Is there a reason you need to export that?

[yarastqt]

For examples needed create complex component with compose, if not export const then this expression print as plain text, maybe have any ideas workarounds for this?

[shilman]

Gotcha. What's happening is that the MDX is being compiled to a new, platform-independent ES6 modules format where "exports" are treated as stories.

There are a few ways we could deal with this:
1) Change it so that MDX is loaded directly into the underlying Storybook storiesOf API, rather than using the modules format.
2) Provide a way to blacklist certain exports from being treated as stories.
3) Tell users to define reusable components in separate files and not in their MDX. So in this case you'd add import { Spin } from './EnhancedSpin' to the top of your MDX.
4) Somehow be smart about distinguishing story exports from other exports. Since we're generating these story exports in a compiler, we could decorate them without the user having to do any extra work. Though we'll also be promoting the modules format as the "new" way to write stories starting in 5.2, so that's a little tricky.

My strawman solution would probably be option 2, which would look like:

<Meta title="Spin/@desktop" ignoreExports={['Spin']} />

@tmeasday I'm sure you have opinions here

[yarastqt]

@shilman Maybe im trying send pull requests with ignoreExports?

[shilman]

@yarastqt That would be great. I'm also happy to implement it, tho I'd like to hear back from @tmeasday before we settle on a solution.

[tmeasday]

@yarastqt -- What does the enhance do?

My instinct is that if people want to do complicated JS-style things with their components in their story files then they should just do it in the module format and either define the markdown inside there (like story.docs = '#some markdown') or else create a second file MDX file that imports from the story file.

It feels like doing complex JS stuff inside MDX is an anti-pattern. Of course this example is about as simple as "complex JS stuff" could be, but this feels like the first step on a long road that we don't want to walk down.

I guess a simpler solution would be: is there a way to write a const declaration in MDX without it outputting the result as a string?

[shilman]

@tmeasday I'm not sure I understand your last sentence there. AFAIK the issue is that Spin is being exported and that export is getting emitted in the module format directly. If the spin was just declared as a const but not exported, this wouldn't be an issue.

[tmeasday]

@shilman because @yarastqt said that was the reason for the export:

if not export const then this expression print as plain text

Or did I read that wrong?

[yarastqt]

Yep, if write:

const Spin = ...

im getting plain text in result :(

[tmeasday]

@shilman having said everything I said above, (I guess reacting to us trying to support non-story exports in order to work around a work around which seems a bit mad), some feedback I did get on the module format was that people wanted to be able to have non-story exports (e.g. for data or other story-related exports that you might want to use in other story files).

One idea I kicked around in that gist was to mark the stories, something like:

export const name = () => <Story/>;
name.isStory = true;

Obviously it would be a pain to do that, so we'd make a simple boxing function, something along the lines of

import { story } from '@storybook/story';

export const name = story(() => Story);

[shilman]

@tmeasday @yarastqt OK I missed that bit about const x treated as plain text.

That's part of the MDX spec. MDX understands:

• markdown
• JSX
• imports & exports

The boxing proposal fits nicely with our MDX compiler. JSX <Story name=xxx> elements would get mapped to boxed stories and all other exports would be treated as normal exports. I could imagine other benefits to this, e.g. it might make other kinds of story annotation easier in the future.

I want to ponder this for a few days before building it into the technical preview.

[tmeasday]

I want to ponder this for a few days before building it into the technical preview.

Yeah we should think about this and loop @ndelangen in as he had opinions about the export being as simple as possible

[ndelangen]

When we load in the mdx, can we get a list of exports?

Then mark those exports as non-stories automatically?

[shilman]

@ndelangen We can do whatever we want from MDX. The question is how we need to designate that in the modules format.

[ndelangen]

This would be a manual ignore-list (proposed by @shilman)

<Meta title="Spin/@desktop" ignoreExports={['Spin']} />

I think asking users to add this list manually is not optimal.

I'd prefer an allow-list instead:

<Meta title="Spin/@desktop" storyExports={['_size']} />

But honestly I think this list should just not be manual at all.

The storybook mdx loader should just differentiate between an export it received and one it generates.

The ones it generates it would also add to a list in the default export object.

I fear that this (suggestion by @tmeasday):

export const name = () => <Story/>;
name.isStory = true;

...might in some edge-cases cause issues, since it's mutating the storyFn.

[tmeasday]

I personally don't think we need to worry about allowing non-story exports from MDX. I am not convinced this is a problem that we need to solve this way. For instance, is there is probably a better way to solve the original issue on this ticket rather than hackily supporting a hack (exporting something that is not intended to be exported).

However, I do think there is a legitimate use case for non story exports in the module format. If we accidently end up allowing it in MDX, I wouldn't say that's bad. But I wouldn't be doing any magic to make it happen.

---

Should we take this discussion elsewhere?

I am interested in this @ndelangen:

I fear that this might in some edge-cases cause issues, since it's mutating the storyFn

Was the module format going to mutate the story function as a matter of course (for instance to change the title or add decorators or params?).

We could definitely do something like:

export default {
  title: 'My Kind',
  storyExports: ['story'], // defaults to all named exports
}

export story = ...;
export nonStory = ...;

Although I suspect that'll be pretty annoying to use.

[ndelangen]

I personally don't think we need to worry about allowing non-story exports from MDX. I am not convinced this is a problem that we need to solve this way. For instance, is there is probably a better way to solve the original issue on this ticket rather than hackily supporting a hack (exporting something that is not intended to be exported).

You mean in the storybook mdx loader we just drop the extra exports and fix the issue that way?

However, I do think there is a legitimate use case for non story exports in the module format. If we accidentally end up allowing it in MDX, I wouldn't say that's bad. But I wouldn't be doing any magic to make it happen.

I am interested in this @ndelangen:

I fear that this might in some edge-cases cause issues, since it's mutating the storyFn

Was the module format going to mutate the story function as a matter of course (for instance to change the title or add decorators or params?).

You're right, I forgot about that.

Still, considering we talked about stories as data, -where the story is the pure mockdata-, do you still believe this would be a good idea?

We could definitely do something like:

export default {
  title: 'My Kind',
  storyExports: ['story'], // defaults to all named exports
}

export story = ...;
export nonStory = ...;

Although I suspect that'll be pretty annoying to use.

The idea is that you'd only need to use this IF exporting non-stories from this module format. Minimizing the annoyance-factor perhaps.

[tmeasday]

I personally don't think we need to worry about allowing non-story exports from MDX. I am not convinced this is a problem that we need to solve this way. For instance, is there is probably a better way to solve the original issue on this ticket rather than hackily supporting a hack (exporting something that is not intended to be exported).

You mean in the storybook mdx loader we just drop the extra exports and fix the issue that way?

What I mean is that I don't currently think there are legitimate use cases for exporting from an MDX story file directly (willing to be convinced though!). In this case @yarastqt is simply export-ing in order to work around a limitation of MDX, they are not actually trying to export anything. I would suggest we explore the use case a little more before worrying about supporting MDX exports just in order to let people hackily export things as a workaround.

Still, considering we talked about stories as data, -where the story is the pure mockdata-, do you still believe this would be a good idea?

I think you convinced me there are enough legitimate use cases where a story needs to be a function. If we moved to stories-as-data it would probably need to be an either-or (i.e. you can make a story a function OR a set of props). I'm not sure the extra complexity that would result would be worth it, but I would definitely be up for exploring it further.

The idea is that you'd only need to use this IF exporting non-stories from this module format. Minimizing the annoyance-factor perhaps.

I know, but I suspect that users that are exporting non-stories from files are probably going to have some pattern (such as exporting the story data 😉 ) that means they end up exporting non-stories from most story files. So I'm not sure. We could definitely prototype it in any case!

[shilman]

I've opened a separate issue to discuss the module format & have summarized our conversation there https://github.com/storybookjs/storybook/issues/7071

I'll implement whatever gets decided for the module format as part of the MDX compiler so that this won't require any extra syntax on the MDX side.

[shilman]

Shiver me timbers!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.2.0-alpha.30 containing PR #7188 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.