[Change request] Disable reactDocgen task in storybook build, increasing SB build performance

[Philzen]

This makes storybook rebuild components considerably faster:

// `web/config/storybook.config.js`
module.exports = {
  addons: [ /* ... */ ],
  features: { /* ... */ },
  /** @see {@link https://storybook.js.org/docs/react/configure/typescript} */
  typescript: {
    reactDocgen: false,
  }
}

Although the "docs" tab is not used in the RedwoodJS storybook setup at all, it seems to be included in the storybook dependency stack and executed by default. This causes re-rendering to pause / stop / meditate / waste our precious time at 70% of the rebuilt:

Above snippet disables this build step. An additional comment could be added so RedwoodJS users know where to look should they ever decide they need the DogGenPlugin in their storybook.

Would have contributed this as PR straightaway, but somehow could not find the origin of web/config/storybook.config.js in this repo.

[jtoar]

Thanks @Philzen! I'll try to have a look sometime this week. And most of the storybook config files are here: https://github.com/redwoodjs/redwood/tree/main/packages/testing/config/storybook. This one specifically is probably what you're looking for: https://github.com/redwoodjs/redwood/blob/main/packages/testing/config/storybook/main.js.

[Philzen]

@jtoar Thanks, now i slowly get it after checking the RedwoodJS Storybook reference.

As much as i don't want to live without this optimization in my projects and thus can only highly recommend it, i feel this requires additional steps to make this explicit (so users don't fall into a pit of failure when they try to activate the DocGen-Plugin):

Either

1. Make it very explicit in the RedwoodJS Storybook reference that user need to add reactDocgen: true, to storybook.config.js should they want to activate the DocGenPlugin.

   or

2. Add storybook.config.js to the project by default with that explicit setting and a comment as shown in my code snippet.

[jtoar]

@virtuoushub thoughts on this one? @thedavidprice do we have any plans for the docs feature of storybook?

[virtuoushub]

@virtuoushub thoughts on this one? @thedavidprice do we have any plans for the docs feature of storybook?

Not a strong one, but I think I lean towards option 2:

2. Add storybook.config.js to the project by default with that explicit setting and a comment as shown in my code snippet.

Also curious how much Storybook 6.5 will help performance so maybe this tweak won't be necissary? ( see: https://storybook.js.org/blog/storybook-lazy-compilation-for-webpack/ )

[thedavidprice]

Although the "docs" tab is not used in the RedwoodJS storybook setup at all, it seems to be included in the storybook dependency stack and executed by default.

My guess is that this is a miss on our part. @virtuoushub a good next step with the Storybook team would be to set up an audit where they critique our default setup and let us know what might be missing and what might be uneccesary. Also, this work was originally done by Peter P. — we can loop him in when the time is right.

Near-term

As of 1.0, removing anything is technically a breaking change unless we can demonstrate otherwise. I think our next steps to get there should be:

1. Documenting things like this in the Redwood Storybook Doc; e.g. a new section for performance that suggests config changes like this
2. Scheduling a meeting with the SB team to walk them through our defaults and audit what we've done
3. Wait until 6.5 hits for final evaluation and decisions. If it really improves build time by 50%, then that could possibly affect what defaults we consider to be baseline "good enough" performance.

How does all that sound?

[shilman]

In my experience, performance is all over the map with react-docgen-typescript. If you remove it, storybook controls won’t get auto-generated, but Storybook should fail gracefully with a “no controls found” note. Your call whether you consider that to be a breaking change. Some of our users disable this option for that reason.

Some notes on the status of this issue:

• This won’t change before 6.5
• Moving forward, we will re-evaluate react-docgen as the default, which is dramatically faster than react-docgen-typescript but didn’t work well for typescript in the past (the major limitation has since been fixed). This could happen in SB7.0 and should take care of the main issue (possibly at the cost of type accuracy for some corner cases).
• In 7.x we will change our architecture for how controls are inferred to improve performance and also compatibility with non-webpack builders.

[virtuoushub]

I think for now we just do a better job in our docs showing how to disable unused addons/features of those addons (like @Philzen's above suggestion ) rather than taking an opinion on defaults.

see also: https://storybook.js.org/addons/tag/essentials

---

related: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#docs-framework-refactor-for-react

cc @thedavidprice / @jtoar

[Philzen]

@virtuoushub

... rather than taking an opinion on defaults.

From RedwoodJS' README.md:

Just saying 😉

---

If you remove it, storybook controls won’t get auto-generated, but Storybook should fail gracefully with a “no controls found” note.

@shilman can you elaborate? Our dev team has been using this option for ages in our projects now, and all storybook controls work fine. Our assumption back then was that docgen is only required if the storybook includes docs, but from looking at the docs i'm not that sure anymore.

All i know is that when i disable it on my comparably slow machine, the rebuild time is cut down by ~25 – 30 % (also in Storybook 6.5.x), thus enhancing the development flow quite perceptible.

[virtuoushub]

@virtuoushub

... rather than taking an opinion on defaults.

From RedwoodJS' README.md:

Just saying 😉

@Philzen - I hear you, however the amount of Storybook configuration permutations makes it so having a default decision for every possibility is not in scope.

What we have is a set of opinionated defaults, with the option for user overrides.

---

opinion ⌚

storybook controls won’t get auto-generated

imho this loss of a feature is not what we want for our default out of the box experience; so, I am of the opinion to leave the current defaults as is, and document our findings to aid others in the performance gains you have found.

In terms of the "docs" tab, this is a known issue when args are missing from a Story; and work has already been started to close the gap on this.

---

If the above opinion on defaults for typescript.reactDocgen can be improved upon, please use #5269 to let us know.

[Philzen]

Hey @virtuoushub thanks for your patience with me until here – there actually was a lack of understanding from my side for quite a while. I have tested with re-enabled reactDocgen: 'react-docgen' | 'react-docgen-typescript' and now finally understand what you mean when you say

storybook controls won’t get auto-generated

… i sincerely didn't get it, although now as i see it, it kinda obvious 🤦 😆

When we set react-docgen: false in our project back then, we were simply happy with the enormous performance gain and only now i realize why i haven't missed any "auto-generated" controls for a second: b/c they simply never worked for us – much to the contrary, they were a real nuisance.

Our project is all Chakra UI, and that means our custom components either get a listing of ~63 inferred controls of which we normally want to touch three in average, with basically all the others requiring an object configuration making the whole component break as soon as the Set object-Button is clicked – or we'll not get a single one (for any component using forwardRef that is).

BTW, i can confirm that react-docgen (as opposed to undefined or react-docgen-typescript) is really fast, but in my tests it did not render a single control, so again it's pointless to our project.

Now as i have retested it i can only affirm that i'd never go back to the default setting – configuring a few controls via the args-argument with exactly the options i want is very simple and actually preferable for me, while i get usability and performance gains on top for free :)

But absolutely hear you when you say that people with other project setups have very good experiences with it and that you want to keep an "all batteries included"-philosophy for freshly created RedwoodJS projects.

Hence my suggestion would be to keep the default setting, but give developers a hint about their options in the config-file, something like:

// `web/config/storybook.config.js`
module.exports = {
  addons: [ /* ... */ ],
  features: { /* ... */ },
  typescript: {
    /** Default setting infers properties from your components and generates controls in storybook from them.
      Set to `reactDocgen: false` if you want faster rebuild times and prefer to configure them yourself anyway
      via `args` and `argTypes` (@see https://storybook.js.org/docs/react/writing-stories/args) */
    reactDocgen: 'react-docgen-typescript'
  }
}

What do you think?