Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[theme] Fails when combining @chakra-ui/react and @material-ui/core #25852

Closed
2 tasks done
fel1xw opened this issue Apr 20, 2021 · 27 comments
Closed
2 tasks done

[theme] Fails when combining @chakra-ui/react and @material-ui/core #25852

fel1xw opened this issue Apr 20, 2021 · 27 comments
Labels
discussion package: styled-engine Specific to @mui/styled-engine

Comments

@fel1xw
Copy link

fel1xw commented Apr 20, 2021

TypeError: undefined is not a function

It fails here: https://github.com/mui-org/material-ui/blob/next/packages/material-ui/src/Autocomplete/Autocomplete.js#L340

  • The issue is present in the latest release aka alpha
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

<Autocomplete /> component can't be opened.

Expected Behavior 🤔

<Autocomplete /> should work with chakra and material ui :)

Steps to Reproduce 🕹

https://codesandbox.io/s/frosty-browser-694e6?file=/src/App.js

Steps:

  1. Try to open Autocomplete component

Context 🔦

I am using @material-ui/core and @chakra-ui/react in the same application (which might not be the best but that is a different topic). Looks like with the release of version 5 and some changes to the theme/styling the Autocomplete component broke.

Your Environment 🌎

`npx @material-ui/envinfo`
System:
    OS: macOS 10.15.7
  Binaries:
    Node: 15.12.0 - ~/.nvm/versions/node/v15.12.0/bin/node
    Yarn: 1.12.3 - /usr/local/bin/yarn
    npm: 7.6.3 - ~/.nvm/versions/node/v15.12.0/bin/npm
  Browsers:
    Chrome: 89.0.4389.128
    Edge: Not Found
    Firefox: 86.0
    Safari: 14.0.2
@fel1xw fel1xw added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Apr 20, 2021
@mnajdova
Copy link
Member

I've switched the providers, and added second the ThemeProvider coming from Material-UI and it works - https://codesandbox.io/s/focused-meninsky-uj9js?file=/src/App.js:663-664 Both libraries are using emotion's ThemeProvider, but expect and create different theme structure. If you would like to use both libraries, I'd suppose you need to take care of making sure that the theme that you use in the end is compatible with both libraries.

@mnajdova mnajdova added package: styled-engine Specific to @mui/styled-engine out of scope The problem looks valid but we won't fix it (maybe we will revisit it in the future) and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Apr 20, 2021
@mnajdova
Copy link
Member

I am closing this as I don't think it's worth exploring any further.

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 20, 2021

Interesting issue. It's a case we discussed a long time ago when we talked about the migration to emotion/styled components. Basically, it's about how our theme structure might conflict with the one existing apps have. So I don't think that it's specific to Chakra-UI.

As @mnajdova said, it's unlikely we can easily solve this. The only alternative I can think of is to remove the styled engine theme provider from our ThemeProvider and forward the theme with a prop. I guess this would bring back to the question @eps1lon asked not too long ago on https://github.com/mui-org/material-ui/pull/25776/files

@fel1xw I would love to learn more about the incentive for using both libraries. Is there something that Material-UI could do, in its long-term roadmap to remove the requirement?

@fel1xw
Copy link
Author

fel1xw commented Apr 22, 2021

Thank you for your fast responses, but looks like the solution from @mnajdova only works in my skeleton example, as soon as you integrate an component from chakra-ui it also fails probably for the reasons that @oliviertassinari pointed out.

@oliviertassinari Regarding your question why I need to use both libraries I can easily tell you that the project I'm working on started using chakra-ui but at some point we needed the look and feel of the material ui form elements so I hooked that one in. But when I browse through the codebase I can see that we mostly use the <Box /> and <Flex /> components for layout things. I guess I'll just replicate those and will kick out chakra for now. :)

@oliviertassinari oliviertassinari removed the out of scope The problem looks valid but we won't fix it (maybe we will revisit it in the future) label Apr 22, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 22, 2021

@fel1xw Very interesting, so design (look and feel) was the primary factor in the decision? If it's true, then it resonates with #22485.

But your last sentence would suggest that it's otherwise, that what your team was looking for is the CSS component helpers. @mnajdova has spent the last few months focusing almost exclusively on bringing them to Material-UI v5 (we had to change the styled engine). If there is something that's missing, please let us know.

Regarding the theme structure conflict, did you look into how they could be "adapted"? It's possible that with a few hacks, they could work together.

@oliviertassinari oliviertassinari changed the title [Autocomplete] Fails when combining @chakra-ui/react and @material-ui/core [theme] Fails when combining @chakra-ui/react and @material-ui/core Apr 22, 2021
@eps1lon
Copy link
Member

eps1lon commented Apr 22, 2021

I'm pretty sure we originally talked about having a separate theme context. That was one of the arguments against styled-components. Material-UI interferring with existing emotion themes is a problem which we ideally solve before v5 release. In the meantime documentation should be sufficient.

@eps1lon eps1lon reopened this Apr 22, 2021
@eps1lon eps1lon added this to the v5 milestone Apr 22, 2021
@fel1xw
Copy link
Author

fel1xw commented Apr 22, 2021

@oliviertassinari I can clearly say that the design (of the form components) was more or less the reason to introduce material ui at that point (customer requirement). In the beginning I thought I can make the chakra inputs look like material but decided against it because I did not wanted to implement the floating label for all different kind of input elements, like select, datepicker etc on my own. But I can clearly say that the api for chakra components for responsive styles is amazing e.g.

<Box width={{ base: '100%', lg: '50%' }} />

or also the useBreakpointValue is really beneficial like

const variant = useBreakpointValue({ base: "outline", md: "solid" })

to choose a different variant dependent on the media query.

Regarding the theme structure conflict, did you look into how they could be "adapted"? It's possible that with a few hacks, they could work together.
Until now I tried to "merge" the two themes into one ({ ...muiTheme, ...theme }) (which should not have been worked for like 99% -> which did not). Do you have another approach/idea that I could try out? Otherwise as I stated in my last comment that I wanted to remove chakra-ui completely (which I really can't do at the stage of the project) but have already faced today several issues because I would have needed to recreate many helpers (two of them I shared here with the responsive syntax and the hook).

So for now and thanks to @eps1lon this issue became a milestone. Looking forward to see a solution to this. Do you have any idea about the timeframe and when the first v5 stable version could be released including that change?

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 22, 2021

I can clearly say that the design

@fel1xw Nice. It seems that design has been more than half of why MUI is successful so far. I feel like it's 2/3 to be honest (assuming React doesn't count).

For the Box API is what we document in https://next.material-ui.com/system/basics/. There is the sx prop and the flatten props for Box, Grid, Stack, and Typography.

For the useBreakpointValue API, please upvote #23885. I have been personally pushing for adding it.

Do you have any idea about the timeframe and when the first v5 stable version could be released including that change?

As far as I understand the issue, it's unlikely that it will ever land. We need to use the styled engine's ThemeProvider in the ThemeProvider of Material-UI so the styled API work. We also need to keep the current theme structure for minimizing breaking changes. But at least, we can explore the tradeoff deeper 👍.

@fel1xw
Copy link
Author

fel1xw commented Apr 26, 2021

Oh, I have not found the <Box /> api, it's lovely that you also have that. i've for sure upvoted the useBreakpointValue issue for you as it is soooo beneficial for the user. :)

Damn, I had the hope that it can be solved until the v5 stable release. :P
Can you think of any "hack" that I can try or use for now? :)

@oliviertassinari
Copy link
Member

I had a look at the theme conflicts, I couldn't see any, so a deep merge between the two seems enough. https://codesandbox.io/s/musing-nightingale-7hnd9?file=/src/App.js

import { deepmerge } from "@material-ui/utils";
import { createTheme } from "@material-ui/core/styles";
import { ChakraProvider, extendTheme } from "@chakra-ui/react";

const muiTheme = createTheme();
const chakraTheme = extendTheme();

const theme = deepmerge(chakraTheme, muiTheme);

@eps1lon
Copy link
Member

eps1lon commented May 3, 2021

To be clear: We need to investigate how we can use a separate context. Affecting existing context is quite problematic.

@eps1lon eps1lon reopened this May 3, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented May 3, 2021

@eps1lon As far as I know, we can't. We use the same context so the styled() API can use the theme of Material-UI and so we don't have to provide the theme prop everywhere.

@eps1lon
Copy link
Member

eps1lon commented May 4, 2021

As far as I know, we can't. We use the same context so the styled() API can use the theme of Material-UI and so we don't have to provide the theme prop everywhere.

These are all descriptive facts about the current state. This isn't helpful unless you think the current implementation is perfect.

What we're interested in is better behavior. And the current behavior is not ok and goes against one of the initial goals that we were explicitly concerned with styled-components.

@oliviertassinari
Copy link
Member

oliviertassinari commented May 4, 2021

Correct. What could help is to detail the current tradeoff.

In which way does the current tradeoff yields a better outcome than the implementation of the proposed alternative (isolate the Material-UI theme context and the style engine theme context)?

Pros:

  • allows developers to use a single theme provider, reduce boilerplate and source for mistakes as soon as they need to use our styled helper.
  • allows implementation to not have to apply the theme as a prop systematically, avoid source of mistakes.

Cons:

  • create conflict with existing applications using a custom theme

From my perspective, I think it would be best to wait to see developers complaining more about the theme conflict. Is this a real problem (a pain strong enough) or one that we think its, but is not in practice?
If we can have isolation, without the two cons exposed above, then it's perfect, but can we?

I understand why developers can have two different conflicting theme structure in their application, when dealing with legacy, not great.
I don't understand how this can be a long term target for developers using the styled MD components.

@mrkrlli
Copy link

mrkrlli commented Dec 9, 2021

@oliviertassinari @eps1lon So I ran into this issue, where our current emotion theme is clashing with the MUI 5 theme. It also wasn't clear from the migration documentation that this would be an issue, which took a lot of digging to uncover.

The only place I see this mentioned is here: https://mui.com/guides/interoperability/#theme

However I think the warning is also misleading. Rendering the MUI ThemeProvider first does not actually isolate the 2 themes right? They still use the same theme context, so naming conflicts are still possible.

We're using MUI 5 components as needed (many components are not from MUI). It feels odd that we need to conform our separate theme to fit (or at least not conflict) with the MUI 5 theme. Preferably we would keep our theme framework agnostic, and not require knowledge about MUI 5 theme to continuously avoid naming conflicts.

It's doable, but doesn't feel quite right IMO.

  1. Is there a github issue that is working on having separate theming contexts? I've only found bits and pieces of discussion here and there.
  2. If not, can we properly document this issue? Theme conflicts should be mentioned more explicitly. Especially coming from MUI 4, where having a custom emotion theme works fine.

@mnajdova
Copy link
Member

If not, can we properly document this issue? Theme conflicts should be mentioned more explicitly. Especially coming from MUI 4, where having a custom emotion theme works fine.

We should definitely update the docs on this. It works differently than how it did in v4. I will create a PR for the docs next week.

@caocuong2404
Copy link

This is like cake. Move inside the chakra user interface. The problem will be solved. 🐧

import "./styles.css";
import AutoComplete from "@material-ui/core/Autocomplete";
import TextField from "@material-ui/core/TextField";
import { ThemeProvider } from "@material-ui/core/styles";
import { createMuiTheme } from "@material-ui/core/styles";
import { ChakraProvider, extendTheme } from "@chakra-ui/react";

const muiTheme = createMuiTheme();
const theme = extendTheme();

export default function App() {
  return (
    <ChakraProvider theme={theme} resetCSS>
      <ThemeProvider theme={muiTheme}>
        <div className="App">
          <h1>Hello CodeSandbox</h1>
          <h2>Start editing to see some magic happen!</h2>
          <AutoComplete
            id="combo-box-demo"
            options={[{ title: "a" }, { title: "b" }, { title: "c" }]}
            getOptionLabel={(option) => option.title}
            renderInput={(params) => (
              <TextField {...params} label="Combo box" variant="outlined" />
            )}
          />
        </div>
      </ThemeProvider>
    </ChakraProvider>
  );
}

Sandbox link: https://codesandbox.io/s/eloquent-hopper-e2upeh?file=/src/App.js:0-1026

@bring-shrubbery
Copy link

@caocuong2404 If you try using Chakra's components though, they are not styled at all. I think the styles are getting overwritten by MaterialUI ones. I specifically was trying to use <Button> component.

@vegemite4me
Copy link

@caocuong2404 Thank you for the suggestion, but unfortunately this hasn't helped me. It looks like Chakra deletes MUI's theme.breakpoints.keys value and then Material falls over when trying to call reduce() on something that is undefined (breakpoint.keys) inside generateGrid().

@rimiti
Copy link

rimiti commented Jul 7, 2022

This issue is still present for latest mui and Chakra-ui version.
If someone has found a solution, I'm interested.

@gbrocha
Copy link

gbrocha commented Aug 15, 2022

The issue still occurs. Some solution?

@bring-shrubbery
Copy link

I've come up with a solution, and it's pretty simple:

  1. Wrap your app in both theme providers at the same time.
  2. Join your ChakraUI and MaterialUI theme together using some sort of deepMerge function.
  3. It works!

The end code will look something like this:

const theme = deepMerge(muiTheme, chakraTheme);

const App = () => {
  return <ChakraProvider theme={theme} resetCSS>
    <ThemeProvider theme={theme}>
      <EverythingElse/>
    </ThemeProvider>
  </ChakraProvider>
}

This works because MaterialUI and ChakraUI both use ThemeUI-based themes, so they are technically compatible. Some things are not used in the same way, but they don't break each other where it matters, so for the transition between MaterialUI and ChakraUI, it will be more than enough IMO.

WouterrV pushed a commit to WouterrV/marketplace that referenced this issue Dec 30, 2022
@siriwatknp
Copy link
Member

Hey everyone, I have a POC for the solution.

All you have to do is put the theme of Material UI inside a special id that we provide from the library. Here are some examples:

Chakra + Material UI

import { ChakraProvider, extendTheme, Button } from "@chakra-ui/react";
import { THEME_IDENTIFIER, createTheme } from '@mui/material/styles';
import MuiButton from '@mui/material/Button';

const theme = extendTheme(…your custom theme for chakra);

const muiTheme = createTheme(…your custom theme for Material UI);

<ChakraProvider theme={{ …theme, [THEME_IDENTIFIER]: muiTheme }}>
  <MuiButton> Hey, it works! </MuiButton>
</ChakraProvider>

Whenever you access the theme via styled, useTheme, or sx prop from @mui/material/styles, you will get the Material UI's theme as if the themes are independent.

@castrosuellenx
Copy link

Hey everyone, I have a POC for the solution.

All you have to do is put the theme of Material UI inside a special id that we provide from the library. Here are some examples:

Chakra + Material UI

import { ChakraProvider, extendTheme, Button } from "@chakra-ui/react";
import { THEME_IDENTIFIER, createTheme } from '@mui/material/styles';
import MuiButton from '@mui/material/Button';

const theme = extendTheme(…your custom theme for chakra);

const muiTheme = createTheme(…your custom theme for Material UI);

<ChakraProvider theme={{ …theme, [THEME_IDENTIFIER]: muiTheme }}>
  <MuiButton> Hey, it works! </MuiButton>
</ChakraProvider>

Whenever you access the theme via styled, useTheme, or sx prop from @mui/material/styles, you will get the Material UI's theme as if the themes are independent.

Hey @siriwatknp this solution can be used on v5.11.15 ?

@siriwatknp
Copy link
Member

Hey @siriwatknp this solution can be used on v5.11.15 ?

Not yet, the PR is in review. Will let you know when it is released.

@siriwatknp
Copy link
Member

Theme scoping is introduced in v5.12.0, here is the doc on how to use it with Theme UI 🍾.

cc @castrosuellenx

@LinKassem
Copy link

Hi @siriwatknp ,
I am still facing an issue when using theme scoping & material ui data grid component (when trying to select a row using a checkbox). I am not sure how to fix it.

I have opened an issue for it here with more details: #38808

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion package: styled-engine Specific to @mui/styled-engine
Projects
None yet
Development

No branches or pull requests