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

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

Open
Tracked by #34905
oliviertassinari opened this issue Sep 16, 2023 · 20 comments
Open
Tracked by #34905

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

oliviertassinari opened this issue Sep 16, 2023 · 20 comments
Assignees
Labels
bug 🐛 Something doesn't work nextjs package: system Specific to @mui/system ready to take Help wanted. Guidance available. There is a high chance the change will be accepted

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 16, 2023

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 🔦

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:

@mj12albert
Copy link
Member

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
Copy link
Member Author

oliviertassinari commented Sep 24, 2023

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

then

then

@cyrilchapon
Copy link

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

@mnajdova mnajdova assigned mnajdova and unassigned mj12albert Nov 9, 2023
@ZeeshanTamboli ZeeshanTamboli removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Nov 15, 2023
@bartlangelaan
Copy link

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 oliviertassinari changed the title [system] getInitColorSchemeScript shouldn't have "use client" [system] getInitColorSchemeScript, createTheme() shouldn't have "use client" Jan 17, 2024
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jan 18, 2024

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 #40199 (comment).

@jamietdavidson
Copy link

Following up from #40945 (the issue closed right above) - the recommended steps there did not work (still experiencing flickering). Will be following this issue for updates.

@siriwatknp
Copy link
Member

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

@siriwatknp siriwatknp assigned siriwatknp and unassigned mnajdova Feb 13, 2024
@oliviertassinari
Copy link
Member Author

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

@siriwatknp
Copy link
Member

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
Copy link
Member Author

oliviertassinari commented Feb 19, 2024

Functions cannot be passed directly to Client Components

@siriwatknp Arf, tricky.

It goes back to my point in: emotion-js/emotion#2978 (comment). 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

@bestickley
Copy link

@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 siriwatknp changed the title [system] getInitColorSchemeScript, createTheme() shouldn't have "use client" [system] getInitColorSchemeScript shouldn't have "use client" Mar 7, 2024
@siriwatknp
Copy link
Member

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

@LikeDreamwalker
Copy link

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
Copy link

eevloeev commented Jun 9, 2024

The bug is still relevant

@oliviertassinari oliviertassinari transferred this issue from mui/material-ui Jun 9, 2024
@oliviertassinari oliviertassinari changed the title [system] getInitColorSchemeScript shouldn't have "use client" [system] getInitColorSchemeScript, createTheme() shouldn't have "use client" Jun 9, 2024
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jun 9, 2024

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 siriwatknp added this to the Material UI: v6 milestone Jun 25, 2024
@siriwatknp siriwatknp transferred this issue from mui/pigment-css Jun 25, 2024
@siriwatknp siriwatknp changed the title [system] getInitColorSchemeScript, createTheme() shouldn't have "use client" [system] createTheme() shouldn't have "use client" Jul 8, 2024
@siriwatknp
Copy link
Member

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
Copy link

allicanseenow commented Aug 22, 2024

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
Copy link
Member

siriwatknp commented Aug 23, 2024

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 siriwatknp removed the bug 🐛 Something doesn't work label Aug 23, 2024
@oliviertassinari oliviertassinari added the bug 🐛 Something doesn't work label Aug 23, 2024
@Janpot
Copy link
Member

Janpot commented Aug 26, 2024

FYI: Just noticed the following in #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.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Aug 28, 2024

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 vercel/next.js#64518 (comment), it was getting fixed in #41956, we could resume this.

@oliviertassinari oliviertassinari added the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label Sep 29, 2024
@oliviertassinari oliviertassinari removed this from the Material UI: v7 (draft) milestone Sep 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work nextjs package: system Specific to @mui/system ready to take Help wanted. Guidance available. There is a high chance the change will be accepted
Projects
Status: Selected