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

[material-ui] Remove use client from the index styles #41131

Closed
wants to merge 8 commits into from

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Feb 16, 2024

closes #42750

@siriwatknp siriwatknp added package: material-ui Specific to @mui/material nextjs labels Feb 16, 2024
@mui-bot
Copy link

mui-bot commented Feb 16, 2024

Netlify deploy preview

https://deploy-preview-41131--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 914f8d6

@@ -1,9 +1,9 @@
'use client';
// do not remove the following import (https://github.com/microsoft/TypeScript/issues/29808#issuecomment-1320713018)
/* eslint-disable @typescript-eslint/no-unused-vars */
// @ts-ignore
import * as React from 'react';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead logic?

Suggested change
import * as React from 'react';

Copy link
Member Author

@siriwatknp siriwatknp Feb 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Required, see https://ci.codesandbox.io/status/mui/material-ui/pr/41131/builds/473769.

stdout: "../mui-material/src/styles/CssVarsProvider.tsx(15,9): error TS2742: The inferred type of 'CssVarsProvider' cannot be named without a reference to 'packages/mui-types/node_modules/@types/react'. This is likely not portable. A type annotation is necessary.\n" +
"../mui-material/src/styles/getInitColorSchemeScript.ts(13,25): error TS2742: The inferred type of 'getInitColorSchemeScript' cannot be named without a reference to 'packages/mui-types/node_modules/@types/react'. This is likely not portable. A type annotation is necessary.\n",
stderr: '

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, @Janpot left a comment about this above. The .d.ts file generated https://unpkg.com/browse/@mui/[email protected]/styles/CssVarsProvider.d.ts needs this import.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would the following work as well without the @ts-ignore?

/// <reference types="react" />

@oliviertassinari oliviertassinari changed the title [material-ui] Remove use-client from the index styles [material-ui] Remove use client from the index styles Feb 16, 2024
@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 16, 2024

I wonder if Next.js warns when not using https://nextjs.org/docs/app/building-your-application/optimizing/scripts#inline-scripts import Script from 'next/script.

@siriwatknp
Copy link
Member Author

I wonder if Next.js warns when not using https://nextjs.org/docs/app/building-your-application/optimizing/scripts#inline-scripts import Script from 'next/script.

Maybe the getInitColorSchemeScript should just return the string and let developers compose it.

@siriwatknp siriwatknp added the on hold There is a blocker, we need to wait label Feb 19, 2024
@siriwatknp
Copy link
Member Author

I got this error after removing use client from createTheme.

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: ...}

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 21, 2024
@siriwatknp siriwatknp closed this Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nextjs on hold There is a blocker, we need to wait package: material-ui Specific to @mui/material PR: out-of-date The pull request has merge conflicts and can't be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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