Dropped support for addon-actions addDecorators conflict with Chromatic and Dark Mode diffing

[stevensacks]

Regarding this breaking change: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#dropped-support-for-addon-actions-adddecorators

"In SB6.5, decorators are applied always. "

The official way in Chromatic to diff both dark and light mode is to conditionally render a decorator that will render both dark and light mode only in Chromatic using isChromatic()

Example:

import isChromatic from 'chromatic/isChromatic';
import {initialize, mswDecorator} from 'msw-storybook-addon';
import chromatic from './decorators/chromatic';

initialize();

export const decorators = [mswDecorator];

if (isChromatic()) {
    addDecorator(chromatic);
}

When I rewrite preview.js like this below with a conditional concat, I don't get an error from Chromatic anymore. However, t's fundamentally the same thing, it's just written in a more complicated way.

export const decorators = [mswDecorator].concat(
    isChromatic() ? chromatic : []
);

Using conditional concats like this is somewhat inelegant. It was cleaner to use addDecorator().

If you have a suggestion to solve this in a different way, please let me know.

Note: I have simplified my code above - the actual code has more decorators and an else statement to add a DARK_MODE channel listener locally, which I have rewritten like this:

if (!isChromatic()) {
    const channel = addons.getChannel();
    channel.on('DARK_MODE', (isDark) => {
        const docsStory = document.querySelector('.docs-story');

        if (isDark) {
            document.documentElement.classList.add('dark');
            docsStory?.classList.add('bg-grey-900');
            docsStory?.classList.add('text-white');
        } else {
            document.documentElement.classList.remove('dark');
            docsStory?.classList.remove('bg-grey-900');
            docsStory?.classList.remove('text-white');
        }
    });
}

[stevensacks]

I suppose I could always apply the chromatic decorator and inside of it do the check for isChromatic()

import React from 'react';
import isChromatic from 'chromatic/isChromatic';

const Chromatic = (story) => isChromatic() ? (
    <>
        <div
            className="relative bg-white p-4 text-grey-800"
            style={{minHeight: '50vh'}}
        >
            {story()}
        </div>
        <div
            className="dark relative bg-grey-900 p-4 text-white"
            style={{minHeight: '50vh'}}
        >
            {story()}
        </div>
    </>
) : story();

export default Chromatic;

I don't know if this is better or worse, but it feels worse since this decorator should only be included when we're on Chromatic.

Also, I don't actually just use isChromatic() because it is not sufficient to test dark/light mode on Chromatic. I was just keeping it simple for the use-case.

This is the actual code that's required for dark/light mode to work correctly on Chromatic:

const showBothModes =
    isChromatic() ||
    !![...(window?.location.ancestorOrigins || {length: 0})].some((origin) =>
        origin.includes('www.chromatic.com')
    );

if (showBothModes) {
  addDecorator(chromatic);
} else {
  // the DARK_MODE listener up above
}

This means I would have to make this logic available in both preview.js (for the channel listener) and in the chromatic decorator. It's not hard, it just makes the code more complicated.

I can think of other scenarios where I might want to conditionally add a decorator based on an environment variable, as well.

[shilman]

cc @tmeasday

[tmeasday]

@stevensacks That comment in the MIGRATION is purely referrring to the addDecorator option that used to exist for the actions addon (you'd set it in main.js I believe`): https://github.com/storybookjs/storybook/pull/17755/files#diff-ee93d78468925fce77dd4fdbc569b373527961d478b712d27c074195dde99c4f

addDecorator in preview.js is still supported in 6.5, although it will be removed in 7.0.

I think some kind of array spreading is the best API to use here, probably:

export const decorators = [
  a,
  b,
  ...(isChromatic() ? [c] : []),
];

[ndelangen]

This issue is removed in 7.0