react-theming / storybook-addon

Develop themable components with Emotion/Styled Components/Material-UI with help of Storybook & React Theming
https://react-theming.github.io/storybook-addon
MIT License
206 stars 36 forks source link

not working with storybook 6.4 #39

Closed Develliot closed 2 years ago

Develliot commented 2 years ago

Storybook 6.4 is deprecating things like addDecorator functions.

This is such a good plug in and I have no idea how to fix it :(

it looks like this now in the preview.js file:

export const decorators = [
  (Story) => (
    <ThemeProvider theme="default">
      <Story />
    </ThemeProvider>
  ),
];

The tab for this add on with the theme inspector would still be broken :

Screenshot 2021-12-01 at 19 27 06

I ended up making my own theme switcher that lives on every story:

import { addons } from "@storybook/addons";
import { UPDATE_GLOBALS } from "@storybook/core-events";

export const decorators = [
  (Story, { globals }) => {
    const channel = addons.getChannel();

    const themeFile = (theme) => {
      switch (theme) {
        case "su2c":
          return su2cTheme;
        default:
          return crukTheme;
      }
    };

    const setTheme = (theme) => {
      channel.emit(UPDATE_GLOBALS, {
        globals: { theme },
      });
    };

    return (
      <ThemeProvider theme={themeFile(globals.theme)}>
        <Box marginBottom="m">
          <Select
            value={globals.theme}
            label="Theme"
            name="themeSelector"
            onChange={(e) => {
              setTheme(e.target.value);
            }}
          >
            <option value="cruk">CRUK theme</option>
            <option value="su2c">SU2C theme</option>
          </Select>
        </Box>
        <GlobalStyle />
        <Story />
      </ThemeProvider>
    );
  },
];
dptoot commented 2 years ago

I can definitely confirm this is not working for any version after @storybook/react@6.4.0.

@Develliot can you point me to where you saw this?
Storybook 6.4 is deprecating things like addDecorator functions.

I still see many references in the docs to its usage

dptoot commented 2 years ago

My apologies I see it here in a 6.0.0 release

https://github.com/storybookjs/storybook/pull/11417

dptoot commented 2 years ago

Strangely I have forked and upgraded the demo to 6.4.9 and it works fine. Even after updating the demo to use the addon as is described in the docs.

chrisrhymes commented 2 years ago

I have also tried to upgrade to storybook 6.4 recently, using v1.1.3 of this package. The project builds successfully (now using the storybook default webpack 5) but when I try and view a component, I just get a blank screen, then eventually this error appears.

RangeError: Maximum call stack size exceeded
    at String.replace (<anonymous>)
    at prettyPrint (http://localhost:6006/vendors-node_modules_react-theming_storybook-addon_dist_index_js-node_modules_storybook_addon-d66c03.iframe.bundle.js:160:27)
    at http://localhost:6006/vendors-node_modules_react-theming_storybook-addon_dist_index_js-node_modules_storybook_addon-d66c03.iframe.bundle.js:151:25
    at Array.map (<anonymous>)
    at prettyPrint (http://localhost:6006/vendors-node_modules_react-theming_storybook-addon_dist_index_js-node_modules_storybook_addon-d66c03.iframe.bundle.js:146:52)
    at http://localhost:6006/vendors-node_modules_react-theming_storybook-addon_dist_index_js-node_modules_storybook_addon-d66c03.iframe.bundle.js:151:25
    at Array.map (<anonymous>)
    at prettyPrint (http://localhost:6006/vendors-node_modules_react-theming_storybook-addon_dist_index_js-node_modules_storybook_addon-d66c03.iframe.bundle.js:146:52)
    at http://localhost:6006/vendors-node_modules_react-theming_storybook-addon_dist_index_js-node_modules_storybook_addon-d66c03.iframe.bundle.js:151:25
    at Array.map (<anonymous>)

I also get the "Waiting for data" message in the Theming window.

dptoot commented 2 years ago

@chrisrhymes That is exactly what I am seeing and what I assume everyone else is as well.

I forked this awesome package in an attempt to see if I could address the issue and maybe even open a PR to update the addon to conform to their future addon standards.

https://github.com/react-theming/storybook-addon/compare/master...dptoot:demo-upgrade

This branch has storybook bumped to the latest (6.4.9) and webpack 5 along with the demo refactored to no longer use the deprecated storiesOf api and bring the initialization of the addon to what the README actually specifies

But for the life of me I can't get it to recreate the issue we are all seeing. It's quite confounding and I can understand any reticence on the maintainers side to addressing an issue they can't recreate.

dptoot commented 2 years ago

@UsulPro sorry to pester but this seems like a real issue and being that I can't reproduce it in the actual repo I am not sure how to assist or proceed.

I have created a separate repo that demonstrates the issue. This example repo was create with create-react-app and sb init. I then added a button example using the same code as used in the addons demo code.

https://github.com/dptoot/react-theming-storybook-addon-test

npm run storybook will show the error occurring with storybook@6.4.9 when the button story is selected

RangeError
Maximum call stack size exceeded

Pinning the all storybook dependencies to v6.3.12 will show the addon to work as expected.

Hopefully this is helpful

usulpro commented 2 years ago

Hi @dptoot Thanks for reproducing this. I'll look at it this weekend!

chrisrhymes commented 2 years ago

As a workaround, I followed this approach to set a theme switcher using globals in storybook. It does not have the features of this package, but it allows you to swap between different themes in storybook 6.4.

https://storybook.js.org/docs/react/essentials/toolbars-and-globals#create-a-decorator

usulpro commented 2 years ago

Hey guys! Thanks for reporting this!

Switching to the new syntax is pretty straightforward and in a general case it should look like this

// preview.js

export const decorators = [withThemes(ThemeProvider, [theme1, theme2, theme3])];

We will update the package and readme later but for now, it should work fine with the current package.

@dptoot Thanks again for creating a repo. There is an issue indeed. But looks like it's another kind of issue related to using EmotionJS with Storybook itself. This is reproduced even without react-theming addon. I've created a separate issue to track this https://github.com/react-theming/storybook-addon/issues/45

I'm closing this as the main question is solved. Please feel free to reopen this if you're still facing issues with adding decorators

hkar19 commented 2 years ago

Hey guys! Thanks for reporting this!

Switching to the new syntax is pretty straightforward and in a general case it should look like this

// preview.js

export const decorators = [withThemes(ThemeProvider, [theme1, theme2, theme3])];

We will update the package and readme later but for now, it should work fine with the current package.

@dptoot Thanks again for creating a repo. There is an issue indeed. But looks like it's another kind of issue related to using EmotionJS with Storybook itself. This is reproduced even without react-theming addon. I've created a separate issue to track this #45

I'm closing this as the main question is solved. Please feel free to reopen this if you're still facing issues with adding decorators

sorry, this is not working for my case (i am using JSS btw):


const providerFn = ({ theme, children }) => {
  return (
    <ThemeProvider theme={theme}>{children}</ThemeProvider>
  );
};

const themingDecorator = withThemes(null, [themeA, themeB], { providerFn });

export const decorators = [
  themingDecorator,
  ];

i still got

gambar
sdaconceicao commented 2 years ago

@UsulPro I can also confirm the provided solution does not work with react-jss. I still see the Max call stack exceeded error, specifically whenever I have a story using controls where I spread the args coming into the story. If I remove the args spreading, the story will load.

Working Story `

{(args) => }

`

Failing Story `

{(args) => }

`

PoomSmart commented 2 years ago

Based on @chrisrhymes solution, you will add something like this to preview.js:

import { defaultTheme } from 'some-default-theme';
import { anotherTheme } from 'some-another-theme';
import { ThemeProvider } from 'some-theme-provider';

export const globalTypes = {
    theme: {
        name: 'Theme',
        description: 'Global theme for components',
        defaultValue: 'Default',
        toolbar: {
            icon: 'switchalt',
            items: ['Default', 'Another'],
            showName: true,
        },
    },
};

const getTheme = (themeName) => {
    switch (themeName) {
        case 'Another':
            return anotherTheme;
        default:
            return defaultTheme;
    }
};

const withThemeProvider = (Story, context) => {
    const theme = getTheme(context.globals.theme);
    return (
        <ThemeProvider theme={theme}>
            <Story {...context} />
        </ThemeProvider>
    );
};

export const decorators = [ withThemeProvider ];
usulpro commented 2 years ago

@PoomSmart