Storybook: withOptions doesn't affect Storybook v5

[Rolandisimo]

Describe the bug
Adding addon-options decorator has no effect on the storybook

To Reproduce
Steps to reproduce the behavior:

1. Create a CRA2
2. Add v5 addons and storybook/react (5.0.0-beta.3 for addons and 5.0.0-rc.1 for storybook/react)
3. Add addDecorator(withOptions({ name: "FooBar", ... }))
4. Observe

Expected behavior
Options change as they do in Storybook v4

Code snippets
addons.js

import '@storybook/addons';
import '@storybook/addon-actions/register';
import '@storybook/addon-knobs/register';
import '@storybook/addon-links/register';
import '@storybook/addon-options/register';
import 'storybook-readme/register';

config.js

import React from 'react';
import { configure, addDecorator } from '@storybook/react';
import { withKnobs } from '@storybook/addon-knobs';
import { withInfo } from '@storybook/addon-info';
import { withOptions } from '@storybook/addon-options';

const styles = {
  display: 'flex',
  alignItems: 'center',
  justifyContent: 'center',
  border: '1px solid rgb(238, 238, 238)',
  padding: 100
};
const CenterDecorator = (storyFn) => (
  <div style={styles}>
    { storyFn() }
  </div>
);

// ❌ doesn't work in v5
addDecorator(
  withOptions({
    name: 'Ringa roll'
  })
);

// ✅ Work fine
addDecorator(CenterDecorator);
addDecorator(withKnobs);
addDecorator(withInfo(
  {
   inline: true,
   header: false,
  }
));

function requireAll(requireContext) {
  return requireContext.keys().map(requireContext);
}

function loadStories() {
  requireAll(require.context("../src", true, /\.story\.jsx?$/));
}

configure(loadStories, module);

[tmeasday]

@shilman - I think the new recommended API is:

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

But we should definitely keep back-compat, perhaps with a deprecation warning, and do we need to update the dos/migration guide too?

[shilman]

Thanks so much, this is a great bug! I'm able to reproduce and will fix.

Here are some notes:

The addon is receiving the event as expected. However the call to api.setOptions inside the addon is apparently failing in v5. Looking into this now and hopefully will be an easy fix.

Also, here are some side comments to you @Rolandisimo:

1. You should generally use the same version of Storybook packages, i.e. if you're using @storybook/[email protected] you should also use @storybook/[email protected]. That wasn't the problem in this case--this is a nice juicy bug, but I just wanted to say it again because this is the second issue you've filed with a version mismatch like that.
2. withOptions is deprecated in favor of addParameters. You'll see this if you look in the browser console. That said, it should still work.

[Rolandisimo]

@shilman Thanks a bunch for looking into this.

Regarding the comments:

1. You're absolutely right. Will note that. However, that felt like an easy inconsistency to run into. Thus, if this is important, I believe it should somehow be stressed more visibly. _(especially to a newcomer)_
2. Yes, a discrepancy I noticed several times. For example, here https://www.npmjs.com/package/@storybook/addon-options, the documentation uses withOptions. However, this didn't change the issue. addParameters still didn't work. @tmeasday I believe this answers your question regarding the doc in need of an update.

Looking forward to the fix! 👍🎉

[domyen]

@Rolandisimo

_Note: This is only about the theme, not the other configurable things in the option addon_

In 5.0, name is not configurable via the options addon. If you want to adjust the Storybook name, as it appears in the top left corner of the UI, take a look at using brand via a theme file. This allows you complete control to pass an svg(use template literals) or a string.

https://github.com/storybooks/storybook/blob/next/lib/theming/src/themes/light.ts#L38

[tmeasday]

Can we get back-compat on this or is it too hard @domyen?

[domyen]

@tmeasday We can, but I'm not sure passing name and url via params is an API we want to support in the future.

Building this into the UI logic results in at least 6 permutations of the brand area (top left) to test and maintain. The new brand is intended to be open ended to avoid this.

@Hypnosphi had some good feedback 🙏, but I don't think we came to a conclusion.

What do you think?

[tmeasday]

We can, but I'm not sure passing name and url via params is an API we want to support in the future.

Usually if we want to remove something we have a major release of deprecation (where we continue to support it but warn users about it). Not critical in this case but I would suggest better.

Building this into the UI logic results in at least 6 permutations of the brand area (top left) to test and maintain

So now the only option is either the default or a custom component? I feel like it's a pretty common use case to just want to change the title, seems a pity to make this hard for people. I'm not sure I really buy the permutations argument 🤷‍♂️

I think @Hypnosphi's main feedback was the brand option on the theme should be something renderedable rather than an element (something rendered). i.e. () => <x> rather than <x> and should get passed the title and url. I think that makes sense and is pretty uncontroversial.

[Rolandisimo]

I'm kinda on-board with @tmeasday regarding not liking over-complication of the simple action of changing a name of the storybook. My vote would be to be user-focused. It's not about a slick pipeline or 200ms faster compilation, but rather the ease of adoption of a tool.

Personally, withOptions seems like a ridiculously easy and encouraging way to add the configuration. Unlike, theming which felt hairy and scary.

Maybe I'm missing the big picture, but these are my 2 cents.

[domyen]

@Rolandisimo have a PR open for this from a theme POV. Will work with @tmeasday to hook it up to params. 👍

[shilman]

Ooh-la-la!! I just released https://github.com/storybooks/storybook/releases/tag/v5.0.0-rc.7 containing PR #5773 that references this issue. Upgrade today to try it out!

Because it's a pre-release you can find it on the @next NPM tag.

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