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

oliviertassinari commented 1 year ago

Steps to reproduce 🕹

Link to live example:

Steps:

Create a project with Next.js
Add in the layout

import { getInitColorSchemeScript } from '@mui/material/styles';

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

Current behavior 😯

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:

mj12albert commented 1 year ago

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

oliviertassinari commented 1 year ago

@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

cyrilchapon commented 1 year ago

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

bartlangelaan commented 9 months ago

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).

oliviertassinari commented 9 months ago

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.

jamietdavidson commented 9 months ago

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.

siriwatknp commented 9 months ago

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

oliviertassinari commented 9 months ago

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

siriwatknp commented 8 months ago

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.

oliviertassinari commented 8 months ago

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:

bestickley commented 8 months ago

@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 :)

siriwatknp commented 8 months ago

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

LikeDreamwalker commented 5 months ago

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!

eevloeev commented 5 months ago

The bug is still relevant

oliviertassinari commented 5 months ago

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.

siriwatknp commented 4 months ago

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.

allicanseenow commented 2 months ago

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/

siriwatknp commented 2 months ago

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.

Janpot commented 2 months ago

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/next@14.2.6_@babel+core@7.25.2_@opentelemetry+api@1.8.0_@playwright+test@1.46.1_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.

oliviertassinari commented 2 months ago

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.

mui / material-ui