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

[styles] Cannot read pxToRem of undefined #28496

Closed
2 tasks done
SomervilleTom opened this issue Sep 20, 2021 · 11 comments · Fixed by #28530
Closed
2 tasks done

[styles] Cannot read pxToRem of undefined #28496

SomervilleTom opened this issue Sep 20, 2021 · 11 comments · Fixed by #28530
Labels
package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. v5.x migration

Comments

@SomervilleTom
Copy link

My attempt to migrate from v4 to v5 has broken my app. The stack trace of the multiple errors suggests that they originate from issues within the @mui components themselves. I have only limited ability to debug issues within the node_modules subdirectory tree.

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Here is a screenshot after following the upgrade recipe:

20210920_stack_trace

Expected Behavior 🤔

Here is a screenshot of the v4 behavior:

20210920_v4_example

Steps to Reproduce 🕹

Steps:

  1. Contact me so that I can launch the app
  2. Attempt to connect at https://covid.tms-dev.zeetix.com

Context 🔦

I've built an interactive data browser that currently uses up-to-date covid data. Other apps will follow.

I consciously use the Google Material UIX in order to optimize the user experience of my visitors, users, and clients.

Your Environment 🌎

`npx @mui/envinfo`
  System:
    OS: Linux 3.10 CentOS Linux 7 (Core)
  Binaries:
    Node: 14.17.6 - ~/.nvm/versions/node/v14.17.6/bin/node
    Yarn: 1.22.5 - /usr/bin/yarn
    npm: 7.24.0 - ~/.nvm/versions/node/v14.17.6/bin/npm
  Browsers:
    Chrome: 90.0.4430.72
    Firefox: support!
Mozilla Firefox 78.9.0esr
  npmPackages:
    @emotion/react: ^11.4.1 => 11.4.1 
    @emotion/styled: ^11.3.0 => 11.3.0 
    @mui/core:  5.0.0-alpha.46 
    @mui/icons-material: ^5.0.0 => 5.0.0 
    @mui/material: ^5.0.0-rc.1 => 5.0.0-rc.1 
    @mui/private-theming:  5.0.0-rc.1 
    @mui/styled-engine:  5.0.0-rc.1 
    @mui/styles: ^5.0.0-rc.1 => 5.0.0-rc.1 
    @mui/system:  5.0.0-rc.1 
    @mui/types:  7.0.0-rc.1 
    @mui/utils:  5.0.0-rc.1 
    @types/react:  17.0.3 
    react: 17.0.1 => 17.0.1 
    react-dom: 17.0.1 => 17.0.1 
    typescript: ^4.1.3 => 4.2.4 

The above screenshots are from a Chrome browser running on Windows 10 Pro. The browser is running Version 93.0.4577.82 (Official Build) (64-bit).

@SomervilleTom SomervilleTom added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 20, 2021
@siriwatknp
Copy link
Member

Seems like some of your components are not wrapped within ThemeProvider, that's why it cannot read values inside theme. Here is a similar issue in troubleshooting section

I am happy to help if you could provide code sample or the repo that I can take a look.

@siriwatknp siriwatknp changed the title Multiple failures while attempting to migrate from v4 to v5 [Migration] Cannot read pxToRem of undefined Sep 21, 2021
@siriwatknp siriwatknp added v5.x migration and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 21, 2021
@eps1lon
Copy link
Member

eps1lon commented Sep 21, 2021

Seems like some of your components are not wrapped within ThemeProvider, that's why it cannot read values inside theme

Components shouldn't need to be wrapped in a ThemeProvider. Only if you want a different theme.

It doesn't look like this bug report has enough info for one of us to reproduce it.

Please provide a CodeSandbox (https://material-ui.com/r/issue-template-latest), a link to a repository on GitHub, or provide a minimal code example that reproduces the problem.

Here are some tips for providing a minimal example: https://stackoverflow.com/help/mcve

@eps1lon eps1lon added the status: waiting for author Issue with insufficient information label Sep 21, 2021
@TTMoon
Copy link

TTMoon commented Sep 21, 2021

I have the same problem.

CodeSandbox

@SomervilleTom
Copy link
Author

It doesn't look like this bug report has enough info for one of us to reproduce it.

Please provide a CodeSandbox (https://material-ui.com/r/issue-template-latest), a link to a repository on GitHub, or provide a minimal code example that reproduces the problem.

Can you deal with a bitbucket git repo? I'll see if I can abstract something useful in the meantime.

@siriwatknp
Copy link
Member

siriwatknp commented Sep 21, 2021

I have the same problem.

@TTMoon

Thanks for the codesandbox. The correct way is to use `import { styled } from '@mui/material/styles', here is the working sandbox. May I ask where do you get the code sample from because it is from our documentation, we will need to fix it.

@TTMoon
Copy link

TTMoon commented Sep 21, 2021

@siriwatknp

Thank you for your quick answer.
I solved the problem.

I referred to this page. link

@SomervilleTom
Copy link
Author

I think I've solved the issue. The main issue seems to be a simple error on my part. It appears to me that there is also an issue in the v4-to-v5 migration page (https://next.material-ui.com/guides/migration-v4/).

In my code, I missed the theme={theme} prop assignment in the ThemeProvider setup section.

There are some other issues in the code sample that contributed to my confusion. Here is the code sample as presented in the documentation:

import { ThemeProvider, createMuiTheme, makeStyles } from '@mui/material/styles';

const theme = createMuiTheme();

const useStyles = makeStyles((theme) => {
  root: {
    // some css that access to theme
  }
});

function App() {
  const classes = useStyles(); // ❌ If you have this, consider moving <ThemeProvider> to HOC and wrap the App
  return <ThemeProvider theme={theme}>{children}</ThemeProvider>;
}

The first issue is that this code, as provided, fails to compile because @mui/material/styles has not yet been installed (according to the recipe). I went ahead and did the installation suggested in the following section.

The second issue is that, even with the new material installed, the makeStyles method is not exported, and so the provided definition of useStyles can't compile. Which is fine, because it is defining a function that shouldn't be called anyway!

I suggest at least commenting out the useStyles call in the example, or removing it altogether and putting the explanation in a note. When that call to useStyles is absent, then its definition can be removed. This, in turn, means that makeStyles can be removed from the import line.

Here is the analogous code from my app.js that now works:

/**
 * App.js
 * @copyright: 2021 by Thomas M. Stambaugh & Zeetix, LLC (http://www.zeetix.com)
 * All rights reserved.
 * 
 * The contents of this file may not be copied, duplicated, or used without the
 * written consent of Zeetix, LLC.
 * Uses CssBaseline to reset margins, paddings, and so on.
 *
 */
import React from 'react';
import { CssBaseline } from '@mui/material';
import { ThemeProvider, createMuiTheme } from '@mui/material/styles';
import { useAuth0 } from "@auth0/auth0-react";
import Loading from '../auth0/Loading';

import AppContainer from './AppContainer';
const theme = createMuiTheme();

const App = () => {
  const { isLoading } = useAuth0();
  if (isLoading) {
    return <Loading />
  }

  return (
    <ThemeProvider
      theme={theme}
    >
      <CssBaseline />
      <AppContainer
      />
    </ThemeProvider>
  );
};

export default App;

I apologize for the jerk-around -- I suspect that adding the theme={theme} to the ThemeProvider wrapper fixed my issue.

@siriwatknp
Copy link
Member

There are some other issues in the code sample

Thanks for the input, I will open a fix for this.

@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 22, 2021

Components shouldn't need to be wrapped in a ThemeProvider. Only if you want a different theme.

@eps1lon Ideally this wouldn't be needed but it changed in v5. Now, the @mui/styles package is not aware of the default Material Design theme. We couldn't find a better tradeoff with Marija when we worked on moving the JSS dependency out of the @mui/material package.

If we see more complaints on this pain, maybe we could log a warning when developers use the (theme) => API with an empty theme? @mnajdova

@oliviertassinari oliviertassinari added the package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. label Sep 22, 2021
@oliviertassinari oliviertassinari changed the title [Migration] Cannot read pxToRem of undefined [styles] Cannot read pxToRem of undefined Sep 22, 2021
@oliviertassinari oliviertassinari removed the status: waiting for author Issue with insufficient information label Sep 22, 2021
@ZHUZZCODER
Copy link

May I ask if this problem has been solved

@mnajdova
Copy link
Member

May I ask if this problem has been solved

Based on the comments it seems like it has been solved. Please open a new issue with a reproduction if face similar problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. v5.x migration
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants