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] Support theme scoping in useMediaQuery #44339

Merged
merged 4 commits into from
Nov 8, 2024

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Nov 7, 2024

closes #44335

Added a test and updated the types to use the Material UI Theme.

Review recommendation: hide whitespace

image

@siriwatknp siriwatknp added package: system Specific to @mui/system package: material-ui Specific to @mui/material labels Nov 7, 2024
@mui-bot
Copy link

mui-bot commented Nov 7, 2024

Netlify deploy preview

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

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against b4e1532

Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

LGTM 😊

@siriwatknp siriwatknp merged commit f567701 into mui:master Nov 8, 2024
19 checks passed
@seanparmelee
Copy link

seanparmelee commented Nov 13, 2024

Just a heads up that this change may require users to make code changes.

We had some code like this:

  const isMobile = useMediaQuery<Theme>((theme) => theme.breakpoints.down('sm'));

and when we updated to 6.1.7 our build started failing on this line with

Type error: Expected 0 type arguments, but got 1.

Granted, it's easy enough to remove the <Theme>

@DiegoAndai
Copy link
Member

Thanks for letting us know @seanparmelee!

@siriwatknp seems like we should've added the type argument here, right? https://github.com/mui/material-ui/pull/44339/files#diff-4a4c308f0f78aea6f2ff8d60829ebcbef84029157da66ce2c6a8f4411b1fbbc8

@seanparmelee
Copy link

Just spun up a dev server (we're using Next.js 14.2.17) with this version of MUI and received this error when navigating to our app:

Error: Attempted to call unstable_createUseMediaQuery() from the server but unstable_createUseMediaQuery is on the client. It's not possible to invoke a client function from the server, it can only be rendered as a Component or passed to props of a Client Component.

If I disable turbopack and try again, the error is:

Error: (0 , _mui_system_useMediaQuery__WEBPACK_IMPORTED_MODULE_0__.unstable_createUseMediaQuery) is not a function

I don't see these errors when running in production mode (i.e. next start) and the app works as expected in areas where we're using useMediaQuery

I can log a separate issue for this if you'd like.

@siriwatknp
Copy link
Member Author

Thanks for letting us know @seanparmelee!

@siriwatknp seems like we should've added the type argument here, right? https://github.com/mui/material-ui/pull/44339/files#diff-4a4c308f0f78aea6f2ff8d60829ebcbef84029157da66ce2c6a8f4411b1fbbc8

Technically, the useMediaQuery from Material UI should not have a Theme generic because the theme comes from the ThemeProvider context. However, to make it easy to upgrade, I will add it back with a comment to remove it in v7.

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

Successfully merging this pull request may close these issues.

[material-ui] useMediaQuery callback syntax ignores theme scoping
4 participants