material-ui icon indicating copy to clipboard operation
material-ui copied to clipboard

[system] createTheme() shouldn't have "use client"

Open oliviertassinari opened this issue 2 years ago • 20 comments

Steps to reproduce 🕹

Link to live example:

Steps:

  1. Create a project with Next.js
  2. Add in the layout
import { getInitColorSchemeScript } from '@mui/material/styles';

  return (
    <html lang="en">
      <head>
        {getInitColorSchemeScript()}
      </head>

Current behavior 😯

Screenshot 2023-09-16 at 22 00 32

Expected behavior 🤔

It works

Context 🔦

  • mui/material-ui#34905
  • https://github.com/mui/material-ui/pull/38967
  • https://twitter.com/amanzrx4/status/1742985809316417913

Your environment 🌎

npx @mui/envinfo
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

Search keywords:

Search keywords:

oliviertassinari avatar Sep 16 '23 20:09 oliviertassinari

getInitColorSchemeScript does not have 'use client', but adding it back (PR) doesn't fix it 🤔: https://stackblitz.com/edit/github-ppgum4-bxmkqa?file=src%2Fapp%2Flayout.tsx

mj12albert avatar Sep 19 '23 08:09 mj12albert

@mj12albert I think that this is the opposite problem. There should be no 'use client' in the import path.

In today's import path, there are a good number of them:

https://github.com/mui/material-ui/blob/c23c40a39c46fff45cb8f762f478f63907f9a842/packages/mui-material/src/styles/index.js#L1

then

https://github.com/mui/material-ui/blob/c23c40a39c46fff45cb8f762f478f63907f9a842/packages/mui-material/src/styles/CssVarsProvider.tsx#L1

then

https://github.com/mui/material-ui/blob/c23c40a39c46fff45cb8f762f478f63907f9a842/packages/mui-system/src/index.js#L1

oliviertassinari avatar Sep 24 '23 15:09 oliviertassinari

Landed here after reproducing steps. Any current workaround to avoid flickering while still using app folder and layout ?

cyrilchapon avatar Oct 12 '23 22:10 cyrilchapon

As a workaround, it is possible to import from @mui/system/cssVars/getInitColorSchemeScript directly.

import { Experimental_CssVarsProvider } from "@mui/material";
import { AppRouterCacheProvider } from "@mui/material-nextjs/v14-appRouter";
import getInitColorSchemeScript from "@mui/system/cssVars/getInitColorSchemeScript";

export default function RootLayout({ children }: { children: React.ReactNode }) {
  return (
    <html>
      <head>
        <link rel="preconnect" href="https://fonts.googleapis.com" />
        <link
          rel="preconnect"
          href="https://fonts.gstatic.com"
          crossOrigin="anonymous"
        />
        <link
          rel="stylesheet"
          href="https://fonts.googleapis.com/css2?family=Roboto:wght@300;400;500;600;700&display=swap"
        />
      </head>
      <body>
        <AppRouterCacheProvider>
          <Experimental_CssVarsProvider defaultMode="system">
            {getInitColorSchemeScript({
              // These properties are normally set when importing from @mui/material,
              // but we have to set manually because we are importing from @mui/system.
              attribute: "data-mui-color-scheme",
              modeStorageKey: "mui-mode",
              colorSchemeStorageKey: "mui-color-scheme",
              // All options that you pass to CssVarsProvider you should also pass here.
              defaultMode: "system",
            })}
            {children}
          </Experimental_CssVarsProvider>
        </AppRouterCacheProvider>
      </body>
    </html>
  );
}

It would be nice if this could be integrated in the @mui/material-nextjs package, so that the provider and getInitColorSchemeScript function are combined (and you only have to pass the options once).

bartlangelaan avatar Jan 17 '24 20:01 bartlangelaan

Same bug with createTheme. There is no reason I'm aware of for createTheme to not be used on the server:

Screenshot 2024-01-18 at 00 16 34

https://stackblitz.com/edit/github-z45nfe-5k5dkz?file=src%2Fapp%2Fpage.tsx,src%2Fapp%2Flayout.tsx,src%2Ftheme.ts

I first saw a developer complain on Twitter, more recently on https://github.com/mui/material-ui/pull/40199#issuecomment-1876259824.

oliviertassinari avatar Jan 18 '24 00:01 oliviertassinari

Following up from https://github.com/mui/material-ui/issues/40945 (the issue closed right above) - the recommended steps there did not work (still experiencing flickering). Will be following this issue for updates.

jamietdavidson avatar Feb 08 '24 18:02 jamietdavidson

I'm on this. This should help with zero-runtime integration as well.

siriwatknp avatar Feb 13 '24 08:02 siriwatknp

I took a few steps to fix this in mui/material-ui#40663 but didn't go all in. Wanted to test the water.

oliviertassinari avatar Feb 14 '24 00:02 oliviertassinari

I got this error after removing use client directive from the index:

Error: Functions cannot be passed directly to Client Components unless you explicitly expose it by marking it with "use server".
  {keys: ..., values: ..., up: function, down: ..., between: ..., only: ..., not: ..., unit: ...}

I wonder if it would be a better DX to add use client to createTheme and related functions.

siriwatknp avatar Feb 19 '24 08:02 siriwatknp

Functions cannot be passed directly to Client Components

@siriwatknp Arf, tricky.

It goes back to my point in: https://github.com/emotion-js/emotion/issues/2978#issuecomment-1935131675. The right solution might be to have two default themes/two theme creators: one in the client-side bundle and one in the server-side bundle. I mean, it would be the exact same logic, but one would be flagged as use client, the other wouldn't, so two different entry points, one implementation.

How I understand things working:

SCR-20240221-oups

oliviertassinari avatar Feb 19 '24 14:02 oliviertassinari

@bartlangelaan's workaround didn't work for me. Seems like it's not exported anymore. I've resorted to adding data-mui-color-scheme="dark" as an attribute in my top level <html> to get around the AppBar flickering between custom colors. This works for me since I'm only using dark mode. If there is a better way I'd be interested in knowing :)

bestickley avatar Mar 06 '24 21:03 bestickley

@oliviertassinari I'm removing createTheme() from the title. I think that createTheme() should have use client; to be used with Context.

siriwatknp avatar Mar 07 '24 02:03 siriwatknp

As a workaround, it is possible to import from @mui/system/cssVars/getInitColorSchemeScript directly.

import { Experimental_CssVarsProvider } from "@mui/material";
import { AppRouterCacheProvider } from "@mui/material-nextjs/v14-appRouter";
import getInitColorSchemeScript from "@mui/system/cssVars/getInitColorSchemeScript";

export default function RootLayout({ children }: { children: React.ReactNode }) {
  return (
    <html>
      <head>
        <link rel="preconnect" href="https://fonts.googleapis.com" />
        <link
          rel="preconnect"
          href="https://fonts.gstatic.com"
          crossOrigin="anonymous"
        />
        <link
          rel="stylesheet"
          href="https://fonts.googleapis.com/css2?family=Roboto:wght@300;400;500;600;700&display=swap"
        />
      </head>
      <body>
        <AppRouterCacheProvider>
          <Experimental_CssVarsProvider defaultMode="system">
            {getInitColorSchemeScript({
              // These properties are normally set when importing from @mui/material,
              // but we have to set manually because we are importing from @mui/system.
              attribute: "data-mui-color-scheme",
              modeStorageKey: "mui-mode",
              colorSchemeStorageKey: "mui-color-scheme",
              // All options that you pass to CssVarsProvider you should also pass here.
              defaultMode: "system",
            })}
            {children}
          </Experimental_CssVarsProvider>
        </AppRouterCacheProvider>
      </body>
    </html>
  );
}

It would be nice if this could be integrated in the @mui/material-nextjs package, so that the provider and getInitColorSchemeScript function are combined (and you only have to pass the options once).

Thanks and it works for me!

LikeDreamwalker avatar Jun 06 '24 03:06 LikeDreamwalker

The bug is still relevant

eevloeev avatar Jun 09 '24 12:06 eevloeev

I think that createTheme() should have use client; to be used with Context.

@siriwatknp I would still expect the opposite. We need to be able to use the theme in server components, but also in client components. So I would expect that a theme is created twice.

oliviertassinari avatar Jun 09 '24 15:06 oliviertassinari

I removed getInitColorSchemeScript in favor of InitColorSchemeScript (without use client) and move this to v7. I don't see a possibility to get this done in v6.

siriwatknp avatar Jul 08 '24 08:07 siriwatknp

So this probably means the current stable (v5) and the upcoming version (v6) of MUI don't support a custom theme in a server component?

It seems I can follow this guide here to create the theme in a client component instead: https://mui.com/material-ui/integrations/nextjs/

allicanseenow avatar Aug 22 '24 05:08 allicanseenow

So this probably means the current stable (v5) and the upcoming version (v6) of MUI don't support a custom theme in a server component?

For the default Material UI v6, yes (because it still relies on Emotion). However, the theme will be removed at build time if you replace Emotion with Pigment CSS.

siriwatknp avatar Aug 23 '24 09:08 siriwatknp

FYI: Just noticed the following in https://github.com/mui/material-ui/pull/43264

While running pnpm --filter next-app build I see the following Next.js error

../../packages/mui-system/build/Box/index.mjs
Error: It's currently unsupported to use "export *" in a client boundary. Please use named exports instead.
    at Object.transformSource (/Users/janpotoms/Projects/material-ui/node_modules/.pnpm/[email protected]_@[email protected]_@[email protected]_@[email protected]_babel-plugin-_fn2ktr3o4nwwaygw7yjiv6t4d4/node_modules/next/dist/build/webpack/loaders/next-flight-loader/index.js:87:31)

Import trace for requested module:
../../packages/mui-system/build/Box/index.mjs
../../packages/mui-system/build/index.mjs
../../packages/mui-material/build/styles/index.mjs
./src/app/layout.tsx

Looks like Next.js RSC can't have both export * from '...' and 'use client' in the same file if it's a client boundary. I'll be further looking into this.

Removing 'use client' from the index files makes that build pass.

Janpot avatar Aug 26 '24 08:08 Janpot

Looks like Next.js RSC can't have both export * from '...' and 'use client' in the same file if it's a client boundary. I'll be further looking into this.

@Janpot It might be https://github.com/vercel/next.js/issues/64518#issuecomment-2062630420, it was getting fixed in https://github.com/mui/material-ui/pull/41956, we could resume this.

oliviertassinari avatar Aug 28 '24 00:08 oliviertassinari

So this probably means the current stable (v5) and the upcoming version (v6) of MUI don't support a custom theme in a server component?

For the default Material UI v6, yes (because it still relies on Emotion). However, the theme will be removed at build time if you replace Emotion with Pigment CSS.

Hi, I am trying to use Mui V6 with Next 15, and after I remove the getInitColorSchemeScript from upper examples everything works fine and avoid the "data-mui-color-scheme" SSR issue. So is there anything I should still be aware of? Here is my current layout:

import { CssBaseline, Box, Toolbar } from "@mui/material";
import { ThemeProvider } from "@mui/material/styles";
import { AppRouterCacheProvider } from "@mui/material-nextjs/v13-appRouter";
import type { Metadata } from "next";
import CustomAppBar from "@/components/AppBar";
import { CommonStoreProvider } from "@/providers/common-store-provider";
import theme from "@/theme";
import { NextIntlClientProvider } from "next-intl";
import { getTranslations } from "next-intl/server";
import { getMessages } from "next-intl/server";
import { notFound } from "next/navigation";
import { routing } from "@/i18n/routing";
import { Suspense } from "react";
import Loading from "@/app/[locale]/loading";

export const metadata: Metadata = {
  title: "nextjs-mui-zustand-template",
  description: "Next.js + MUI + Zustand template",
};

export default async function RootLayout(props: {
  children: React.ReactNode;
  params: { locale: string };
}) {
  const params = await props.params;

  const { locale } = params;

  const { children } = props;

  if (!routing.locales.includes(locale as any)) {
    notFound();
  }

  const messages = await getMessages();
  const t = await getTranslations("Common");
  return (
    <html lang="en">
      <body>
        <CommonStoreProvider>
          <AppRouterCacheProvider options={{ key: "css" }}>
            <NextIntlClientProvider messages={messages}>
              <ThemeProvider theme={theme} defaultMode="system">
                <CssBaseline />
                <Box
                  sx={{
                    height: "100vh",
                    width: "100vw",
                    overflow: "hidden",
                    display: "flex",
                  }}
                >
                  <Box
                    sx={{
                      display: "flex",
                      flex: "0 0 100vw",
                      height: "100vh",
                      flexWrap: "npwrap",
                      alignContent: "flex-start",
                      justifyContent: "space-between",
                    }}
                  >
                    <CustomAppBar title={t("appBarTitle")}></CustomAppBar>
                    <Box
                      sx={{
                        flexGrow: 1,
                        width: "100%",
                        height: "100%",
                        overflowY: "auto",
                      }}
                    >
                      <Toolbar></Toolbar>
                      <Suspense fallback={<Loading></Loading>}>
                        {children}
                      </Suspense>
                    </Box>
                  </Box>
                </Box>
              </ThemeProvider>
            </NextIntlClientProvider>
          </AppRouterCacheProvider>
        </CommonStoreProvider>
      </body>
    </html>
  );
}

LikeDreamwalker avatar Nov 26 '24 08:11 LikeDreamwalker