Storybook: Storybook 5.2 with CSF: __esModules story for every component

[chasemccoy]

Describe the bug
I installed the beta of Storybook 5.2 and ran the codmod to update our stories. Everything seemed to work just fine, except all of my components now have a story called __esModule. Clicking on this story displays an error message: Attempted to set a non-object key in a WeakMap (screenshots below)

Screenshots

Storybook config file:

import { configure, addDecorator, addParameters } from '@storybook/react';
import { jsxDecorator } from 'storybook-addon-jsx';
import { withA11y } from '@storybook/addon-a11y';
import { withKnobs } from '@storybook/addon-knobs';
import theme from './theme';

import icons from '!!raw-loader!../dist/icons.svg'

addDecorator(jsxDecorator);
addDecorator(withA11y);
addDecorator(withKnobs);

const req = require.context("../src", true, /.stories.js$/);

function loadStories() {
  const div = document.createElement('div');
  div.style.cssText = 'display:none;';
  div.innerHTML = icons;
  document.body.appendChild(div);
  return require.context('../src', true, /\.stories\.js$/)
}

addParameters({
  options: {
    theme: theme
  }
});

configure(loadStories(), module);

System:

  System:
    OS: macOS 10.14.5
    CPU: (8) x64 Intel(R) Core(TM) i7-6820HQ CPU @ 2.70GHz
  Binaries:
    Node: 10.15.1 - ~/.nvm/versions/node/v10.15.1/bin/node
    Yarn: 1.16.0 - ~/.yarn/bin/yarn
    npm: 6.9.0 - ~/.nvm/versions/node/v10.15.1/bin/npm
  Browsers:
    Chrome: 75.0.3770.142
    Firefox: 64.0
    Safari: 12.1.1
  npmPackages:
    @storybook/addon-a11y: ^5.2.0-beta.24 => 5.2.0-beta.24 
    @storybook/addon-knobs: ^5.2.0-beta.24 => 5.2.0-beta.24 
    @storybook/addon-viewport: ^5.2.0-beta.24 => 5.2.0-beta.24 
    @storybook/react: ^5.2.0-beta.24 => 5.2.0-beta.24 
    @storybook/theming: ^5.2.0-beta.24 => 5.2.0-beta.24 

[jnielson94]

Something that might help diagnose the issue further - do you have any custom babel/webpack config running? It sounds like you might have a custom babel config getting picked up which might be configured incorrectly for storybook?

[chasemccoy]

@jnielson94 I do have a custom Babel config:

const BABEL_ENV = process.env.BABEL_ENV;

module.exports = () => ({
  plugins: [
    "babel-plugin-styled-components",
    "polished",
    "flow-react-proptypes",

    // Stage 3
    "@babel/plugin-syntax-dynamic-import",
    ["@babel/plugin-proposal-class-properties", { loose: true }]
  ],
  presets: [
    [
      "@babel/preset-env",
      {
        loose: true,
        modules: BABEL_ENV === "es" ? false : "commonjs",
        targets: {
          browsers: [
            "last 2 Firefox versions",
            "last 2 Chrome versions",
            "Safari >= 8",
            "last 2 Edge versions",
            "ie > 10"
          ]
        }
      }
    ],
    "@babel/react",
    "@babel/flow"
  ]
});

[jnielson94]

To verify another thing, it looks like you're still using the storiesOf api, correct? Or did you switch to the new CSF?

[chasemccoy]

@jnielson94 Nope, I switched to CSF using the provided code-mod.

[jnielson94]

Ah, just noticed you mentioned that in the initial description. Are you running storybook with a BABEL_ENV defined? If not, it looks like it'll default to commonjs, which would have babel adding something like:

Object.defineProperty(exports, "__esModule", {
  value: true
});

To each file, which would explain why the story exists (since in CSF the export would get picked up as a story) :) Does it go away if you switch your BABEL_ENV to es (or adjust your config)?

[chasemccoy]

@jnielson94 That fixed it! Worked like a charm. This could be good to document somewhere maybe? At least in the migration guide. I definitely assumed it was a bug when I first upgraded.

[jnielson94]

I don't know the documentation well enough to know where it would make sense to note it - @shilman might have a better idea?

[chasemccoy]

@jnielson94 While I've got you here, I had another question about CSF... it seems as if this doesn't work:

Component.story = {
  name: "Category/Example"
};

I used to be able to nest stories into categories using this syntax, but it doesn't seem to work now. Is this a bug or expected?

[jnielson94]

I'm not sure if that's expected or a bug, I know that I ran into that when I tried CSF on our codebase. Using nesting like that definitely works in the default export name (storiesOf equivalent), but I don't know if that was a decision that was made to not allow stories to nest themselves...

[chasemccoy]

@jnielson94 Good to know 👍 I know it can be done with the default export, but only one of those is allowed per-file, so we couldn't have multiple nested categories without making multiple component.stories.js files for each component.

[jnielson94]

Yep, that's where my experimentation landed me as well. We decided that it'd probably be useful to break them up anyway to have smaller files to deal with, so we went ahead and did that (on 5.1 actually). I'd be curious to know the official stance/reason on stories nesting themselves though :)

[chasemccoy]

@jnielson94 Cool. I will open a separate issue to discuss that. Thanks for your help! Gonna go ahead and close this out.

[jnielson94]

I'd be tempted to leave this one open to make sure the docs question gets addressed? Otherwise, glad to help!

[shilman]

@chasemccoy @jnielson94 Looks like you guys sorted this out while I was sleeping.

Let's add this to the docs if it comes up again. CSF picks up all named exports. Perhaps it would be useful to ignore exports whose names start with underscore? That would be a breaking change, but I'd consider it in 6.0. What do you guys think.

Related feature is you can add includeStories/excludeStories rules to your default export. But your solution here is much better, since you'd need to add that to every file, which would suck.

Re: Story nesting, it works exactly as in storiesOf. So:

export default { title: 'Foo|Bar/baz' }
export const apple = () => ...
zoo.story = { name: 'apple/banana' }

Gets translated to:

storiesOf('Foo|Bar/baz').add('apple/banana', () => ...)

I don't expect this to change in future versions of CSF.

[chasemccoy]

@shilman In that example above, apple/banana won't get turned into a nested accordion called "apple" with a story called "banana" in it, will it? That's what I am trying to achieve. Having that happen in the default export means I can't have multiple of those in one file. I'd like to avoid having multiple story files per component.

[shilman]

@chasemccoy I see, that's interesting. Won't change in 5.2, but we could consider something like that for a future release. No promises! cc @tmeasday

[chasemccoy]

@shilman Thanks so much! 🙏 Definitely not a deal breaker for me. My team and I are super excited about CSF.

[tmeasday]

I didn't even know we supported that in the .add() API 😬

I think it (a) probably breaks the assumptions made in CSF and (b) is rarely used enough that we shouldn't support it.

But correct me if I am wrong on (a) or (b).

[shilman]

@tmeasday We don't support that in the .add API. The story will just get the name 'apple/banana' which doesn't help @chasemccoy! 😁

[shilman]

Closing this for now, reply if there's more to resolve!

[chasemccoy]

Sorry, I think I explained this poorly. Here's the code we had before:

storiesOf("Checkbox", module)
  .add("Checked", () => <Checkbox checked={boolean("checked", true)} />)
  .add("Unchecked", () => <Checkbox checked={boolean("checked", false)} />);

storiesOf("Checkbox/Disabled", module)
  .add("Checked", () => (
    <Checkbox
      checked={boolean("checked", true)}
      disabled={boolean("disabled", true)}
    />
  ))
  .add("Unchecked", () => (
    <Checkbox
      checked={boolean("checked", false)}
      disabled={boolean("disabled", true)}
    />
  ));

and it gave us this:

As far as I can tell, with CSF there is no way to replicate this without creating multiple files (so that you can have more default exports).

[chasemccoy]

What I suppose I expected when switching to CSF if that the default export title would be Checkbox, and then naming a story Disabled/Checked would allow me to get the same nesting I had before (but in a single file).

[chasemccoy]

So... I guess what I am saying is that the nesting separators seem to make more sense to me at the story level with CSF, instead of at the module level. If a module == a component in the mental model (at least this is our mental model), then it seems reasonable that you would have stories within that component that would have different nesting. But at the moment it seems nesting is only possible at the module level.

[shilman]

@chasemccoy I understood your request. The current structure is by design. Our intent is that each component will have its own file and will be placed in the user-specified position in the hierarchy. It's flexible, you can achieve what you're looking for by creating more files. But for now we're pretty sticking to the storiesOf structure, and we don't have any immediate plans to deviate from that.

That said, we're also receptive to feedback from users (like yourself) and partners (integrating with other tools), so the format will surely evolve. My point is that you shouldn't hold your breath on being able to nest in the way you requested. It complicates the format and I'm not sure whether we'll add it. And even if we did, it wouldn't make it in until 6.0, which will be awhile.

[chasemccoy]

@shilman Good to know! Thanks for walking me through it. Like I said, definitely not a deal breaker for us 🙂