Storybook: v5 applies decorators to all the kind, no matter where it's placed

Created on 27 Feb 2019  Â·  11Comments  Â·  Source: storybookjs/storybook

import React from 'react';
import { storiesOf } from '@storybook/react';
import centered from '@storybook/addon-centered/react';

storiesOf('Stories', module)
  .add('noncentered', () => 'Hello')
  .addDecorator(centered)
  .add('centered', () => 'Hello');

V4: only second story is centered
V5: both stories are centered

This should be either fixed or documented in MIGRATION.md

bug decorators high priority

Most helpful comment

I had an idea -- we could make .addDecorator log a warning if it is called on a kind after a story has already been added.

I'll do the above and add the migration notes above if everyone is cool with it?

All 11 comments

I'm fairly sure this was an intentional change (right @ndelangen).

Is it possible to add per-story decorators? It doesn't actually look like it is, but it would be easy to add via changing this line:

https://github.com/storybooks/storybook/blob/9f87b711b5aa9f487225c48407c2a23b7cd5df17/lib/client-api/src/client_api.js#L206

To

getDecorators: () => [...allParam.decorators, ...localDecorators, ..._globalDecorators, withSubscriptionTracking],

(Arguably we should make localDecorators and globalDecorators just be set via addParameters behind the scenes, maybe something for v5.1).

If everyone is OK with this, my MIGRATION.md notes would go something like:

Decorator changes

Previously, adding decorators to a component/kind was "stateful". If you added a decorator after a story, the decorator would only apply to future stories. For example:

storiesOf('Stories', module)
  .add('noncentered', () => 'Hello')
  .addDecorator(centered)
  .add('centered', () => 'Hello');

Moving forward, we have a new API planned that will not support this. As it is clearer, if you want to apply a decorator to subset of a kind's stories, you can use the decorator parameter;

storiesOf('Stories', module)
  .add('noncentered', () => 'Hello')
  .add('centered', () => 'Hello', { decorators: [centered] });

What does everyone think? We could also work to make the existing behaviour deprecated, but still work. I'm not sure how easy that is to do @ndelangen

@tmeasday I think we should deprecate the old behavior as part of the 5.0 release. Feels "tricky" to me...

Deprecate or remove entirely?
On 1 Mar 2019, 4:57 PM +1100, Michael Shilman notifications@github.com, wrote:

@tmeasday I think we should deprecate the old behavior as part of the 5.0 release. Feels "tricky" to me...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

Sorry, remove entirely and document rationale in MIGRATION.md

storiesOf('Stories', module)
  .add('noncentered', () => 'Hello')
  .addDecorator(centered)
  .add('centered', () => 'Hello');

I think we should deprecate the old behavior as part of the 5.0 release. Feels "tricky" to me...

I tend to agree.

Yo-ho-ho!! I just released https://github.com/storybooks/storybook/releases/tag/v5.0.0-rc.8 containing PR #5806 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.

I had an idea -- we could make .addDecorator log a warning if it is called on a kind after a story has already been added.

I'll do the above and add the migration notes above if everyone is cool with it?

Yes perfect!

w00t!! I just released https://github.com/storybooks/storybook/releases/tag/v5.0.0-rc.9 containing PR #5819 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.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

rpersaud picture rpersaud  Â·  3Comments

purplecones picture purplecones  Â·  3Comments

wahengchang picture wahengchang  Â·  3Comments

arunoda picture arunoda  Â·  3Comments

miljan-aleksic picture miljan-aleksic  Â·  3Comments