mui / material-ui

Materialย UI: Ready-to-use foundational React components, free forever. It includes Material UI, which implements Google's Material Design.
https://mui.com/material-ui/
MIT License
91.86k stars 31.57k forks source link

Custom theme not propagated within Storybook.js #24282

Closed robphoenix closed 8 months ago

robphoenix commented 3 years ago

Current Behavior ๐Ÿ˜ฏ

When using Material-UI v5 within Storybook, the sx prop is picking up the default theme rather than the custom theme.

// theme.ts
import { createMuiTheme } from '@material-ui/core';

const palette = {
  primary: {
    main: '#ff69b4',
  },
};

export default createMuiTheme({ palette })
// .storybook/preview.js
import React from 'react'
import {ThemeProvider} from '@material-ui/core'
import {StylesProvider} from "@material-ui/core";

import theme from '../src/theme'

export const decorators = [
  (Story) => (
    <StylesProvider injectFirst>
      <ThemeProvider theme={theme}>
        <Story />
      </ThemeProvider>
    </StylesProvider >
  ),
]
// stories/Thing.stories.tsx
const Template: Story<Props> = () => {
  const theme = useTheme();
  console.log({ theme });
  return (
    <>
      <Thing />
      <Box
        sx={{
          p: 8,
          backgroundColor: 'primary.main',
        }}
      />
    </>
  );
};

The theme logged to the console contains the custom theme colour (#ff69b4)

primary: {main: "#ff69b4", light: "rgb(255, 135, 195)", dark: "rgb(178, 73, 125)", contrastText: "rgba(0, 0, 0, 0.87)"}

However, the background colour of the Box component is #3f51b5 (indigo.500)

Expected Behavior ๐Ÿค”

Material UI components should pick up the custom theme when used within storybook.

Steps to Reproduce ๐Ÿ•น

Steps:

  1. git clone https://github.com/robphoenix/mui-storybook
  2. cd mui-storybook && yarn install
  3. yarn storybook
  4. View storybook in browser & check console output.

Context ๐Ÿ”ฆ

I'm building a UI component library. Everything is working fine with v5 when I use my custom theme/components within create-react-app, however I now want to develop the components outside of an app and am using storybook to do so. However this is not possible if the custom theme is not picked up. Apologies if I've overlooked something obvious, after a day of debugging I can't see the wood for the trees. :smile:

Your Environment ๐ŸŒŽ

`npx @material-ui/envinfo` ``` System: OS: macOS 11.0.1 Binaries: Node: 15.2.1 - /usr/local/bin/node Yarn: 1.22.10 - /usr/local/bin/yarn npm: 7.0.10 - /usr/local/bin/npm Browsers: Chrome: 87.0.4280.88 <-- used this browser Edge: Not Found Firefox: 75.0 Safari: 14.0.1 npmPackages: @emotion/react: ^11.1.2 => 11.1.4 @emotion/styled: ^11.0.0 => 11.0.0 @material-ui/core: ^5.0.0-alpha.22 => 5.0.0-alpha.22 @material-ui/icons: ^4.11.2 => 4.11.2 @material-ui/styled-engine: 5.0.0-alpha.22 @material-ui/styles: 5.0.0-alpha.22 @material-ui/system: 5.0.0-alpha.22 @material-ui/types: 5.1.4 @material-ui/unstyled: 5.0.0-alpha.22 @material-ui/utils: 5.0.0-alpha.22 @types/react: ^17.0.0 => 17.0.0 react: ^17.0.1 => 17.0.1 react-dom: ^17.0.1 => 17.0.1 typescript: ^4.1.3 => 4.1.3 ```
oliviertassinari commented 3 years ago

@robphoenix Thanks for the reproduction. As far as I can play with it, it's an issue with the version of emotion. Storybook uses emotion v10, Material UI v5 uses emotion v11. The context theme provider isn't resolved to the right version.

MUI handles emotion as a peer-dependency and not a direct dependency because of the singletons. We have seen the same problem in the past in: https://github.com/mui-org/material-ui/pull/23007/commits/00b33f9affcc0dd011df03da74fc7846d14aaaaa#r522777191. We had to force the resolution to a specific version.

I can solve the issue in the reproduction with this patch:

diff --git a/package.json b/package.json
index 544c087..bb8d89b 100644
--- a/package.json
+++ b/package.json
@@ -68,6 +68,9 @@
         "tslib": "^2.0.3",
         "typescript": "^4.1.3"
     },
+    "resolutions": {
+        "**/@emotion/styled": "^11.0.0"
+    },
     "dependencies": {
         "@emotion/react": "^11.1.2",
         "@emotion/styled": "^11.0.0",
Capture dโ€™eฬcran 2021-01-05 aฬ€ 20 48 51

Relevant line of codes:

A couple of ideas for solving this issue properly:

  1. ~The solution proposed in the initial PR's comment could be interesting to push further. cc @Andarist~ Won't help Storybook forces a specific version.

Maybe @emotion/styled should reexport the ThemeContext.

  1. ~Maybe MUI should make @emotion/styled a direct dependency and get this issue fixed, not a peer one (only have @emotion/core as a peer dependency). cc @mnajdova.~ Tested, doesn't work

  2. Storybook should better isolate the dependencies it uses with the ones used by developers cc @shilman. Related issues: https://github.com/storybookjs/storybook/issues/13145, https://github.com/storybookjs/storybook/issues/12262, https://github.com/storybookjs/storybook/issues/10231

robphoenix commented 3 years ago

That addition to the package.json fixed it for me also, thanks! ๐Ÿš€ ๐Ÿ™

Thanks for the swift, er..., resolution to this issue @oliviertassinari ๐Ÿ˜† ๐Ÿ™„ It's very much appreciated.

Andarist commented 3 years ago

Well, I haven't checked this exact case but it looks awfully like something that has already been reported to me and which also has been reported already several times to Storybook.

The problem is that they enforce a resolution of @emotion/styled to their local version~ (or rather probably to what is found relative to their packages) with webpack aliases. They still depend on Emotion 10 and thus this is causing a conflict - context can't resolve properly.

They shouldn't apply any aliases like this at all. This just won't work - because even if they upgrade to Emotion 11 and force the imported @emotion/styled package to be resolved to that they will essentially break all existing Emotion 10+Storybook users.

I think you can fix that temporarily on your end by manipulating the Storybook's webpack config - but I'm not sure what exact changes are required. Maybe it's just a matter of removing that alias but that would have to be tested to see if Storybook works OK with such a config.

oliviertassinari commented 3 years ago

@Andarist Do you know how Storybook force the resolution of @emotion/styled to v10? I'm asking because I was hopping that Material-UI could have @emotion/styled as a dependency instead of a peer dependency. I might solve the issue. Is there a reason why @emotion/styled need to be a peer dependency?

Andarist commented 3 years ago

I think they are having it as regular dep and force all references to @emotion/styled to resolve to a single location in their webpack config (the webpack part im sure about)

I would advise against making it a dep for Material UI because package managers will just often install multiple copies of it and different parts of the module tree will resolve to different locations and thus to different created contexts etc and for things to work correctly you need to have a single Emotion context. Its pretty much the same reason why you need React itself as peer.

oliviertassinari commented 3 years ago

you need to have a single Emotion context

I likely miss something. The theme and cache contexts seems to be coming from @emotion/react which we would keep as a peer dependency.

Andarist commented 3 years ago

Ah, right - sorr, I've just generalized the problem, not realizing that you only ask about styled. Hm, it should be OK to put it as regular dep. You are still risking several copies being pulled into final bundles but thats just a bundlesize concern.

If I find any other reason for you not doing that I will report back - you might also ping me in the PR introducing that change (if you gonna put one together) for taking a final look at this.

robphoenix commented 3 years ago

@oliviertassinari @Andarist The issue isn't totally fixed by adding the resolutions field in the package.json; when adding components to the theme, any styleOverrides are still not picked up. I've updated the example repo with button overrides for the hover colour. The button picks up the theme colours but not the overrides, however these are present in the console output.

Andarist commented 3 years ago

Just FYI - I might not be able to take a look at the provided repo in the nearest future so if somebody could pick this up it would be highly appreciated. I would just try to locate from what disk location particular contexts are coming from - it really has to be an issue with multiple instances of @emotion/styled or smth being bundled and causing a mismatch.

oliviertassinari commented 3 years ago

The button picks up the theme colours but not the overrides, however these are present in the console output.

@robphoenix This is a different issue, looking at your code, it's very likely fixed by #24253.

robphoenix commented 3 years ago

@robphoenix This is a different issue, looking at your code, it's very likely fixed by #24253.

ah, ok great, thanks. Do you know when this will be released?

fixed with v5.0.0-alpha.23 thanks @oliviertassinari ๐Ÿ™

eps1lon commented 3 years ago

At which point was the repro faulty? I just cloned it and button and box have the same color.

oliviertassinari commented 3 years ago

@eps1lon The expected result is pink

Capture dโ€™eฬcran 2021-01-21 aฬ€ 14 45 49

the actual result is blue

Capture dโ€™eฬcran 2021-01-21 aฬ€ 14 45 27

Which color do you see rendered?

eps1lon commented 3 years ago

The expected result is pink, the actual result is blue. Which color do you see rendered?

Both pink. You're not using the latest version.

I guess the resolutions entry was added after the problem was reported. In the future, please don't fix reproductions so that we can reproduce them later on. Or update the reproduction steps.

I remove the resolutions entry from the repro and I don't know why you would expect that the theme.ts is picked up. I guess this is some magic from storybook that uses this theme in their ThemeProvider.

The problem is caused by two issues:

  1. we don't have an isolated theming context but use the one from emotion. I already described that for styled-components but we never attempted to resolve this issue
  2. Storybook is doing some magic with theme.ts @robphoenix Could you point to some documentation so that we can understand what's happening?
robphoenix commented 3 years ago

please don't fix reproductions so that we can reproduce them later on

sorry, I pushed the first fix as I thought the second issue I mentioned was connected.

I don't know why you would expect that the theme.ts is picked up

In .storybook/preview.js

import React from 'react'
import {ThemeProvider} from '@material-ui/core'
import {StylesProvider} from "@material-ui/core";

import theme from '../src/theme'

export const decorators = [
  (Story) => (
    <StylesProvider injectFirst>
      <ThemeProvider theme={theme}>
        <Story />
      </ThemeProvider>
    </StylesProvider >
  ),
]

Could you point to some documentation so that we can understand what's happening?

https://storybook.js.org/docs/react/configure/overview#configure-story-rendering

Pushplaybang commented 3 years ago

Also struggling with this. I'm seeing partial updates from the provided theme, such as primary and secondary colors, but typography is not applied.

Also, attempted to use the alpha release mentioned above, and received the following error:

TypeError: Cannot read property 'dark' of undefined at push../node_modules/@material-ui/core/Button/Button.js.Object.styleProps.styleProps 
// .storybook/main.js
module.exports = {
  stories: ['../src/**/*.stories.mdx', '../src/**/*.stories.@(js|jsx|ts|tsx)'],
  addons: ['@storybook/addon-links', '@storybook/addon-essentials', '@storybook/addon-a11y'],
}
// .storybook/preview.js
import { CssBaseline } from '@material-ui/core'
import React from 'react'
import { StylesProvider, ThemeProvider } from '@material-ui/core/styles'
import { ThemeProvider as SCThemeProvider } from 'styled-components'

import { useDarkMode } from 'storybook-dark-mode'
import { lightTheme, darkTheme } from '../src/app/theme'

export const decorators = [
  (Story, context) => {
    // FIXME:  no theme control set, use toolbars addon and retrieve state from context.globals once MUI issue resolved
    const isDark = false
    const theme = isDark ?  darkTheme : lightTheme

    return (
      <StylesProvider injectFirst>
        <CssBaseline />
        <ThemeProvider theme={theme}>
          <SCThemeProvider theme={theme}>
            <Story {...context} />
          </SCThemeProvider>
        </ThemeProvider>
      </StylesProvider>
    )
  },
]

export const parameters = {
  actions: { argTypesRegex: '^on[A-Z].*' },
}

I'm using this in nextJS, with styled-components, though I don't think this has any impact. All rendering with theme switching works perfectly in the app, styled-components work fine in storybook.

oliviertassinari commented 3 years ago

@Pushplaybang This issue is specific about emotion. If you are using styled-components you can configure Material-UI to use it too.

Pushplaybang commented 3 years ago

Hi @oliviertassinari

not sure if you misunderstood, adding the above in the hopes that it provides additional context. I'm experiencing the eact behaviour described by the OP.

Current Behavior ๐Ÿ˜ฏ When using Material-UI v5 within Storybook, the sx prop is picking up the default theme rather than the custom theme.

Further, this issue is specifically about the latest version of MUI and how it's themes don't work in storybook, which seems to be a symptom of some issues regarding how each library is depending on emotion, as per the original post, and all of the subsequent conversation above.

Screenshot 2021-02-28 at 21 15 45

The fact that I'm also using styled-components, has no bearing in this instance, I have styled-components and MUI working perfectly in my app, with or without styled-components, the issue remains:

MUI Custom themes don't currently seem to work in Storybook, for the latest stable release, and any subsequent release candidate.

oliviertassinari commented 3 years ago

@Pushplaybang I meant configure Material-UI so you don't bundle emotion and styled-components in your app, only one of the two. We share a workaround in the previous comments.

Pushplaybang commented 3 years ago

@oliviertassinari I've read this thread several times now, could you please quote the workaround you're speaking of?

segunadebayo commented 3 years ago

We experienced this issue as well and managed to walkaround it by updating storybook's main.js config.

// .storybook/main.js

const path = require("path")
const toPath = (filePath) => path.join(process.cwd(), filePath)

module.exports = {
  webpackFinal: async (config) => {
    return {
      ...config,
      resolve: {
        ...config.resolve,
        alias: {
          ...config.resolve.alias,
          "@emotion/core": toPath("node_modules/@emotion/react"),
          "emotion-theming": toPath("node_modules/@emotion/react"),
        },
      },
    }
  },
}

This forces storybook to use the most recent version of @emotion/react which ensures that the theme gets picked up correctly.

Andarist commented 3 years ago

Note that some things might break with this - so YMMV. It might work in 80-90% use cases though.

siriwatknp commented 3 years ago

We experienced this issue as well and managed to walkaround it by updating storybook's main.js config.

// .storybook/main.js

const path = require("path")
const toPath = (filePath) => path.join(process.cwd(), filePath)

module.exports = {
  webpackFinal: async (config) => {
    return {
      ...config,
      resolve: {
        ...config.resolve,
        alias: {
          ...config.resolve.alias,
          "@emotion/core": toPath("node_modules/@emotion/react"),
          "emotion-theming": toPath("node_modules/@emotion/react"),
        },
      },
    }
  },
}

This forces storybook to use the most recent version of @emotion/react which ensures that the theme gets picked up correctly.

This workaround works in one of my project, thanks!

package.json

{
  "devDependencies": {
    "@emotion/react": "11.1.5",
    "@emotion/styled": "11.1.5",
    "@material-ui/core": "5.0.0-alpha.28",
    "@storybook/addon-actions": "^6.1.18",
    "@storybook/addon-essentials": "^6.1.18",
    "@storybook/addon-links": "^6.1.18",
    "@storybook/react": "^6.1.18",
  }
}

.storybook/main.js is similar to above

.storybook/preview.js

import React from "react";
import { MuiThemeProvider, createMuiTheme } from "@material-ui/core/styles";
import StyledEngineProvider from "@material-ui/core/StyledEngineProvider";
import CssBaseline from "@material-ui/core/CssBaseline";

const withThemeProvider = (Story: any, context: any) => {
  return (
    <StyledEngineProvider injectFirst>
        <MuiThemeProvider theme={createMuiTheme()}>
          <CssBaseline />
          <Story {...context} />
        </MuiThemeProvider>
      </StyledEngineProvider>
  );
};

export const decorators = [withThemeProvider];

export const parameters = {
  actions: { argTypesRegex: "^on[A-Z].*" },
  controls: { expanded: true },
};

stories/demo.tsx

Result

the classes generated by makeStyles overrides emotion

image

andfk commented 3 years ago

We experienced this issue as well and managed to walkaround it by updating storybook's main.js config.

// .storybook/main.js

const path = require("path")
const toPath = (filePath) => path.join(process.cwd(), filePath)

module.exports = {
  webpackFinal: async (config) => {
    return {
      ...config,
      resolve: {
        ...config.resolve,
        alias: {
          ...config.resolve.alias,
          "@emotion/core": toPath("node_modules/@emotion/react"),
          "emotion-theming": toPath("node_modules/@emotion/react"),
        },
      },
    }
  },
}

This forces storybook to use the most recent version of @emotion/react which ensures that the theme gets picked up correctly.

I did a few changes but with your base as start and got it finally working! Thanks

// .storybook/main.js

module.exports = {
  webpackFinal: async (config) => {
      return {
        ...config,
        resolve: {
          ...config.resolve,
          alias: {
            ...config.resolve.alias,
            "@emotion/core": require.resolve('@emotion/react'),
            "emotion-theming": require.resolve('@emotion/react'),
            "@emotion/styled": require.resolve('@emotion/styled'),
          },
        },
      }
    }
}

Tested with packages:

"@material-ui/core": "^5.0.0-alpha.32",
"@emotion/styled": "^11.3.0",
"@emotion/react": "^11.1.5",
....

/// dev
"@storybook/react": "^6.2.9",
....
italodeandra commented 2 years ago

Hey team, not sure if it's related, but when trying to fix something I ended up here, so I'll leave the following "story" for people that might have the same problem as myself. But just to make it clear, nothing that was suggested here fixed my problem (like change the resolvers on the webpack which broke other things and didn't actually fix the problem), but the workaround I did might also work with the issue here (let me know if it doesn't so I can remove this part from my comment).

I created a new project with CRA, installed Storybook, added a decorator with the ThemeProvider, and created a simple story with a Button. It works normally on the "Canvas" tab, but on the "Docs" tab, when you interact with the button (clicking?) it will trigger the ripple which try to use the transition from the theme, and it can't, because for some reason inside the "Docs" tab, the theme from Material-UI is replaced by the theme from Storybook itself.

image

https://github.com/mui-org/material-ui/blob/07f638f1213e43b4ce7552dd39b6c04532d0d9e5/packages/material-ui/src/ButtonBase/TouchRipple.js#L80

How I discovered it's using the theme from Storybook itself:

image

This only happens on the "Docs" tab.

So a real good "fix" is, is to use the ThemeProvider from emotion-theming.

import CssBaseline from "@material-ui/core/CssBaseline"
import { defaultTheme } from "../lib/styles2"
import { ThemeProvider } from "@material-ui/core"

import { ThemeProvider as EmotionThemeProvider } from "emotion-theming"

export const parameters = {
  actions: { argTypesRegex: "^on[A-Z].*" },
  controls: {
    matchers: {
      color: /(background|color)$/i,
      date: /Date$/,
    },
  },
}

const withThemeProvider = (Story, context) => {
  return (
    <EmotionThemeProvider theme={defaultTheme}>
      <ThemeProvider theme={defaultTheme}>
        <CssBaseline />
        <Story {...context} />
      </ThemeProvider>
    </EmotionThemeProvider>
  )
}

export const decorators = [withThemeProvider]
oliviertassinari commented 2 years ago

@italodeandra emotion-theming seems to be an emotion v10 API, based on https://emotion.sh/docs/emotion-11#package-renaming. So I assume it's done in order to get the context provider of emotion from v10.

KASOGIT commented 2 years ago

Hey team, not sure if it's related, but when trying to fix something I ended up here, so I'll leave the following "story" for people that might have the same problem as myself. But just to make it clear, nothing that was suggested here fixed my problem (like change the resolvers on the webpack which broke other things and didn't actually fix the problem), but the workaround I did might also work with the issue here (let me know if it doesn't so I can remove this part from my comment).

I created a new project with CRA, installed Storybook, added a decorator with the ThemeProvider, and created a simple story with a Button. It works normally on the "Canvas" tab, but on the "Docs" tab, when you interact with the button (clicking?) it will trigger the ripple which try to use the transition from the theme, and it can't, because for some reason inside the "Docs" tab, the theme from Material-UI is replaced by the theme from Storybook itself.

image

https://github.com/mui-org/material-ui/blob/07f638f1213e43b4ce7552dd39b6c04532d0d9e5/packages/material-ui/src/ButtonBase/TouchRipple.js#L80

How I discovered it's using the theme from Storybook itself:

image

This only happens on the "Docs" tab.

So a real good "fix" is, is to use the ThemeProvider from emotion-theming.

import CssBaseline from "@material-ui/core/CssBaseline"
import { defaultTheme } from "../lib/styles2"
import { ThemeProvider } from "@material-ui/core"

import { ThemeProvider as EmotionThemeProvider } from "emotion-theming"

export const parameters = {
  actions: { argTypesRegex: "^on[A-Z].*" },
  controls: {
    matchers: {
      color: /(background|color)$/i,
      date: /Date$/,
    },
  },
}

const withThemeProvider = (Story, context) => {
  return (
    <EmotionThemeProvider theme={defaultTheme}>
      <ThemeProvider theme={defaultTheme}>
        <CssBaseline />
        <Story {...context} />
      </ThemeProvider>
    </EmotionThemeProvider>
  )
}

export const decorators = [withThemeProvider]

Thanks a lot solved it for me as well :) Using:

    "@emotion/react": "11.4.0",
    "@emotion/styled": "11.3.0",
    "@material-ui/core": "5.0.0-alpha.38",
    "@storybook/react": "6.3.1"
robphoenix commented 2 years ago

@italodeandra solution works for me as well ๐Ÿ™๐Ÿป

evyang commented 2 years ago

Adding the following to my package.json fixed it, thank you @oliviertassinari ! ๐Ÿ’ฏ ๐Ÿฅณ I was having the problem where experimentalStyled was not being recognized as an import from @material-ui/core

"resolutions": { "**/@emotion/styled": "^11.0.0" },

alexworker23 commented 2 years ago

@italodeandra Thank you! Your fix solved my problem. Issue was also only with the "Docs" tab.

italodeandra commented 2 years ago

@mstosio That's weird. Without having the code to take a look it's very difficult to reproduce.

I tested on my project but I don't have this problem and I'm also not on the latest beta release. I'm at 5.0.0-beta.2 so you can try rolling back to the previous one and keep testing until you reach the version I'm at.

mstosio commented 2 years ago

As you said, it was version issue. I resolved it :)

shilman commented 2 years ago

FWIW the difference between the Docs tab and the Canvas is that Docs uses Storybook theming (which is also based on emotion), whereas Canvas is a standalone iframe that only uses Emotion if your components/stories use it.

We are making sweeping architectural changes in SB6.4 that might address this issue, and I'll try to remember to post here when that's ready for testing. Furthermore, we will also aim to eliminate this problem completely in 7.x after the new architecture stabilizes.

dmastrorillo commented 2 years ago

Any updates on this? I was able to get the theme working in the canvas but I am getting TypeError: Cannot read properties of undefined (reading 'content') in the Docs tab and nothing is displayed.

Issue is arrising in DocsPage.js

Screen Shot 2021-10-23 at 5 04 56 pm (2)

italodeandra commented 2 years ago

@danyy1996 Did you try this? https://github.com/mui-org/material-ui/issues/24282#issuecomment-859393395

shilman commented 2 years ago

I mentioned above that we are improving our rendering architecture. I tried out the latest 6.4-beta version on MUI5 on the reproduction contributed in https://github.com/storybookjs/storybook/issues/16099, and it's working OK for me. However, the new renderer is still experimental and hidden behind a feature flag.

What I did:

  1. upgrade storybook to the latest 6.4-beta:
npx sb@next upgrade --prerelease
  1. enable the modernInlineRender feature flag in .storybook/main.js
module.exports = {
  features: { modernInlineRender: true }
}

Here's my simple reproduction:

https://github.com/shilman/sb-mui5-modern-inline-render

Andarist commented 2 years ago

Removing aliases in .storybook/main.js also works, like this:

module.exports = {
  stories: [
    '../src/**/**.stories.mdx',
    '../src/**/**.stories.@(js|jsx|ts|tsx)',
  ],
  addons: ['@storybook/addon-links', '@storybook/addon-essentials'],
  webpackFinal(config) {
    delete config.resolve.alias['emotion-theming'];
    delete config.resolve.alias['@emotion/styled'];
    delete config.resolve.alias['@emotion/core'];
    return config;
  },
};
hanihusam commented 2 years ago

Thanks @Andarist! This way totally works for me.

siriwatknp commented 2 years ago

@Andarist I have an idea about forcing MUI theme via prop instead of ThemeProvider from emotion. Would like to have your opinion about this idea.

Even though this issue will be fixed from the storybook side but would it be better if MUI does not depend on emotion's ThemeProvider but using its own context so that it works with any lib that also use emotion. (We are building another design system by leveraging emotion styled also and it would be nice if both of them can live together without sharing emotion's ThemeProvider)

Here is the styled API that I think MUI can be updated:

// createStyled.js
import emotionStyled from '@emotion/styled';

const createStyled = ({ useTheme }) => {
  return (tag) => {
    const defaultStyledResolver = styledEngineStyled(tag);
    const muiStyledResolver = (styleArg) => {
      const Component = defaultStyledResolver(styleArg);
      const WrappedComponent = (props) => {
        const theme = useTheme() || defaultTheme;
        // I got it from https://github.com/emotion-js/emotion/blob/045f0aca3b8c746b6a0a2c10b935d7044c60f635/packages/styled/src/base.js#L80
        return <Component {...props} theme={theme} />; 
      };
    }
    return muiStyledResolver;
  }
}

Full version of MUI createStyled

Then MUI build its own React context + useTheme and create styled with it.

const ThemeContext = React.createContext();

const useTheme = () => React.useContext(ThemeContext);

export const styled = createStyled({ useTheme });

export const ThemeProvider = ({ theme }) => (
  <ThemeContext.Provider value={theme}>...</ThemeProvider>
)

With this approach, MUI can use theme to communicate with components without the need from emotion's ThemeProvider.

Do you see any downsides to this approach? Let me know if this is unclear to you.

Andarist commented 2 years ago

@siriwatknp yes, this is one of the downsides of a library using a singleton context provided by Emotion. I think that often it's better to create your own context for a library because it allows the library to be better isolated from the host application. This is often less convenient for the author though. We were thinking in the past about removing theming from Emotion altogether (the reasoning can be found here) but we didn't go with it since theming was so in-demand feature. I believe though that creating your own context is so easy nowadays that using such a strategy is a good choice to make.

Overkloker commented 2 years ago

@robphoenix I have a similar problem I get a theme in ThemeProvider but my components apply the default theme image

It looks like Storybook wraps things using its own ThemeProvider that overwrites the MUI theme image

It seems to be a problem with emotion

Try a solution that helped me preview.js

import { ThemeProvider as MUIThemeProvider, createTheme } from '@mui/material/styles';
import { ThemeProvider } from 'emotion-theming';

const theme = createTheme({
    palette: {
        primary: {
            main: '#000000'
        },
        secondary: {
            main: '#ff00ff'
        }
    }
});

export const decorators = [
    (Story) => (
        <MUIThemeProvider theme={theme}>
            <ThemeProvider theme={theme}>
                {Story()}
            </ThemeProvider>
        </MUIThemeProvider>
    )
];
roughpandaz commented 2 years ago

@Overkloker YES! YOU BEAUTIFUL HUMAN! I hope karma comes around one day and brings you peace and joy. ๐Ÿ™

oliviertassinari commented 2 years ago

I have an idea about forcing MUI theme via prop instead of ThemeProvider from emotion. Would like to have your opinion about this idea.

@siriwatknp I recall that we explored this option with @mnajdova in the past. One downside is that it creates yet another React element in the tree. I think that we should explore how much it harms the DX in the React dev tools and how much the extra element slowdowns the rendering.

Otherwise, if we assume that Joy and Material are not meant to be rendered in the same tree, then we could force to use the ThemeProvider. It makes me think about this issue, incompatibility with Chakra: https://github.com/mui-org/material-ui/issues/25852

mnajdova commented 2 years ago

@oliviertassinari I remembered the exact same downside, can't remember if there was another reason. Obviously, if people want to use styled() from emotion/styled-components they wound need to add additional ThemeProvider. @siriwatknp the discussion is already happening on three places, here, https://github.com/mui-org/material-ui/pull/29150#discussion_r735441092 and spreaded on different issues. Could we create a dedicated RFC with the proposed changes and consilidate all information we have and start from there?

siriwatknp commented 2 years ago

Could we create a dedicated RFC with the proposed changes and consilidate all information we have and start from there?

Sounds good.

luannoe commented 2 years ago

@Overkloker I've tried at least 6 different solutions and yours is the only one that works. Thank you so much :D

robphoenix commented 2 years ago

This suggestion to use the modernInlineRenderer feature didn't work for me unfortunately, however deleting the alias did. Thanks @Andarist ๐Ÿ™‚

jrskerritt commented 2 years ago

Heads up, @Overkloker's workaround no longer works. Seems the emotion-theming package was deleted and merged into @emotion/react, but simply switching out one for the other caused this problem to return.

Use @Andarist's workaround instead -- it works great

venanciorodrigo commented 2 years ago

I mentioned above that we are improving our rendering architecture. I tried out the latest 6.4-beta version on MUI5 on the reproduction contributed in storybookjs/storybook#16099, and it's working OK for me. However, the new renderer is still experimental and hidden behind a feature flag.

What I did:

  1. upgrade storybook to the latest 6.4-beta:
npx sb@next upgrade --prerelease
  1. enable the modernInlineRender feature flag in .storybook/main.js
module.exports = {
  features: { modernInlineRender: true }
}

Here's my simple reproduction:

https://github.com/shilman/sb-mui5-modern-inline-render

Thanks @shilman. Just one minor change here:

Instead of

module.exports = {
  features: { modernInlineRender: true },
}

use

module.exports = {
  features: { emotionAlias: false },
}

Then this should work as expected using the latest pre-release ๐Ÿ‘

cgarrovillo commented 2 years ago

~~Latest as of 11.12.21 @storybook/...: 6.4.0-rc.1 emotionAlias: false causes crashes~~