mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.37k stars 32.14k forks source link

Having problems after upgrading to v4. #15898

Closed Ericnr closed 5 years ago

Ericnr commented 5 years ago

After upgrading to v4 I've been getting this error

TypeError: Cannot read property 'root' of undefined
Toolbar
node_modules/@material-ui/core/esm/Toolbar/Toolbar.js:46
  43 |     variant = _props$variant === void 0 ? 'regular' : _props$variant,
  44 |     other = _objectWithoutProperties(props, ["classes", "className", "component", "disableGutters", "variant"]);
  45 | 
> 46 | var className = clsx(classes.root, classes[variant], !disableGutters && classes.gutters, classNameProp);
     | ^  47 | return React.createElement(Component, _extends({
  48 |   className: className,
  49 |   ref: ref
View compiled
renderWithHooks
node_modules/react-dom/cjs/react-dom.development.js:13450
  13447 |     ReactCurrentDispatcher$1.current = HooksDispatcherOnMountInDEV;
  13448 |   }
  13449 | }
> 13450 | var children = Component(props, refOrContext);
        | ^  13451 | 
  13452 | if (didScheduleRenderPhaseUpdate) {
  13453 |   do {
View compiled
updateForwardRef
node_modules/react-dom/cjs/react-dom.development.js:15014
  15011 | {
  15012 |   ReactCurrentOwner$3.current = workInProgress;
[...]

It repeats itself 13 times for different Mui components Ive exported. Ive tried degrading back to v3.9.3, like I used before but the problems persist so it might no be something intrinsic to v4.

Tech Version
Material-UI v3.9.3 / 4.0.1
React 16.8.6
eps1lon commented 5 years ago

This is very likely caused by your build setup. Can you share a minimal reproducible example?

oliviertassinari commented 5 years ago

I have seen this class of error once last week, the resolution was to upgrade Next.js from v7 to v8: #15852.

Ericnr commented 5 years ago

Somehow it fixed itself after uninstalling and installing Mui a few times. Closing this.

Kuirak commented 5 years ago

I have the same Problem with Paper, Card and CircularProgress. I am using webpack 3, babel 6 and typescript 3.4.5 Any hints where I should start searching for the error in my build setup?

ztoben commented 5 years ago

I'm having similar issues as well. Is there a part of our build config you would want to see?

travi commented 5 years ago

combing through the docs and upgrade guide to try to find a solution to this issue, i found reference to a StylesProvider, but the docs seem to be inconsistent/incomplete about its use. would the absence of this provider potentially result in errors like this?

we are not using next, so trying to upgrade that isn't an option we could try.

travi commented 5 years ago

the references to the styles provider are

the second reference includes this description, but the example only shows the ThemeProvider:

When rendering, we will wrap App, our root component, inside a StylesProvider and ThemeProvider to make the style configuration and the theme available to all components in the component tree.

travi commented 5 years ago

i did try adding the provider, guessing at the proper way to configure it, but did not see any behavioral change. this at least seems like a logical reason for the errors, but i'm not sure if i've configured it incorrectly or if it is simply not necessary

RZsam commented 5 years ago

I have the same problem. @travi @Kuirak @ztoben did you find any solution?

travi commented 5 years ago

unfortunately, no. we ended up shelving our upgrade and the changes that prompted us to attempt to upgrade so early.

we attempted to go down the path of creating a minimal reproduction since our project is private, but had trouble finding what it was about our configuration that would reproduce the problem.

instead, we're hoping that the problem will be identified more generally so we can better understand what details to look deeper into.

eps1lon commented 5 years ago

@travi Do you get the exact same error that is included in the initial issue description?

ztoben commented 5 years ago

@eps1lon, @travi and I were working on the same project. Ours wasn't specific to toolbars but we saw the TypeError: Cannot read property 'root' of undefined on several components.

eps1lon commented 5 years ago

Yeah without any additional information about build setup (styles provider, core/styles or styles, used framework, ssr, prerender, csr etc) I can't help you much.

travi commented 5 years ago

one of the things that i had a hard time getting my head around was that the components where we saw the error all seemed to be using withStyles internally and we were not overriding styles explicitly ourselves that we could tell. the internal usage seemed to pass default style details, so i never did figure out how the object where root was being looked for could ever be undefined.

one piece that somewhat stood out to me toward the end, but i had spent so many hours digging that I didn't fully pursue, is that i could eliminate the error if i stopped rendering the lists of links that we render in a couple places (i had to remove all of them). we use react-router for navigation and are still using v3 of react-router to support some of the data-loading steps for SSR.

in the button section of the migration guide i noticed the following statement:

The component passed to the component prop needs to be able to hold a ref

followed by

This also applies to BottomNavigationAction, Button, CardActionArea, Checkbox, ExpansionPanelSummary, Fab, IconButton, MenuItem, Radio, StepButton, Tab, TableSortLabel as well as ListItem if the button prop is true.

i don't use refs enough to fully have my head around the implications but it did make me wonder if we somehow had something configured to put us in that situation and that maybe react-router v3's Link violated that new expectation

edit: wanted to provide the specific react-router version in case it does end up mattering. we are using v3.2.1, which is the latest of v3

travi commented 5 years ago

the list of other things unique to our app is potentially long, which made it difficult to decide which pieces to invest in with a more minimal reproduction. the basic ones we tried didn't reproduce the problem. here is a short list that comes to mind:

eps1lon commented 5 years ago

our components are in a separate npm package from the application.

Do they have a dependency on material-ui as well? Maybe you bundle multiple versions of material-ui

Can you do a yarn why hoist-non-react-statics or find out otherwise what versions of this package you use?

we were actually in the middle of extracting additional component packages when we found classname mismatches between the server render and the browser rehydration

It sounds like your setup was already broken before v4. If you can't resolve the issue with v4 I would start fixing the original issue.

travi commented 5 years ago

Do they have a dependency on material-ui as well? Maybe you bundle multiple versions of material-ui

the package defines only a peer dependency on material. also, all dependencies coming from node_modules are excluded from the rollup bundle, expecting webpack to include them in the consuming application

Can you do a yarn why hoist-non-react-statics or find out otherwise what versions of this package you use?

we use npm rather than yarn, but i was inspecting pretty closely with npm ls to ensure that there were no conflicting versions along the way

It sounds like your setup was already broken before v4

the problems we saw only came up once we tried to split our components package further. we've been successfully server-rendering with material for almost 3 years.

i'm not sure what caused the classname mismatch with the additional split, but we found several issues that made it sound like there were known (at least very similar) issues due to the sequential nature of classname generation, even when providing createGenerateClassName, etc. we also saw issues mentioning a switch to hashed classname generation with the new engine, so we were investigating that when v4 dropped. it seemed like the simplest way to get to the new styles engine (in hopes that the hashing was actually there) was to get to v4, but we never got far enough in the upgrade to confirm if the hashing was there or not.

travi commented 5 years ago

one more detail that i forgot in my list above is that we do still load matertial v0 for some components, but that was confirmed to be still supported, but not recommended. we've been slowly trying to get the remaining ones upgraded, but since we are three years into this project, there are some deep in areas that we havent updated yet. also, we would need a solution to nested menus to fully transition and since there is still an open issue for official support, we've been watching that conversation to understand what the best path forward would be.

RZsam commented 5 years ago

@travi @eps1lon @ztoben I still did not find any answer neither here or in stack overflow. I'm not using multiple version of material-ui and also using laravel-mix did you make any progress?

RZsam commented 5 years ago

since I'm using laravel, I created new laravel project (5.6) and tried material V4 there and did not get any errors. I replaced packages and versions in the main project package.json still have errors. this is packages which are the same in new and main project:

    "@babel/preset-react": "^7.0.0",
    "axios": "^0.17",
    "babel-preset-react": "^6.23.0",
    "bootstrap": "^4.0.0",
    "cross-env": "^5.2.0",
    "jquery": "^3.2",
    "laravel-mix": "^4.0.16",
    "lodash": "^4.17.4",
    "popper.js": "^1.12",
    "react": "^16.2.0",
    "react-dom": "^16.2.0",
    "resolve-url-loader": "2.3.1",
    "sass": "^1.22.1",
    "sass-loader": "7.*",
    "webpack": "^4.35.2",
    "webpack-cli": "^3.3.5"
    "@material-ui/core": "^4.1.3",
    "@material-ui/styles": "^4.1.2"

rest of the packages in main project:

"@date-io/date-fns": "^1.1.0",
"@date-io/moment": "^1.1.0",
"@fortawesome/react-fontawesome": "^0.1.4",
"@material-ui/icons": "^3.0.2",
"@material-ui/lab": "^3.0.0-alpha.30",
"@tinymce/tinymce-react": "^2.4.0",
"@types/googlemaps": "3.30.13",
"@types/markerclustererplus": "2.1.33",
"ajv": "^5.0.0",
"classnames": "2.2.6",
"d3-drag": "^1.2.3",
"d3-force": "^2.0.0",
"d3-selection": "^1.4.0",
"d3-shape": "^1.3.3",
"d3-zoom": "^1.7.3",
"date-fns": "^2.0.0-alpha.25",
"dhtmlx-gantt": "^6.1.6",
"downshift": "^3.2.2",
"jquery": "^3.3.1",
"jss-rtl": "^0.2.3",
"material-ui-color-picker": "^3.2.0",
"material-ui-pickers": "^2.2.1",
"material-ui-pickers-jalali-utils": "^0.4.3",
"moment": "^2.24.0",
"moment-jalaali": "^0.8.1",
"npm": "^6.9.0",
"npm-run-all": "4.1.3",
"path-to-regexp": "^2.4.0",
"perfect-scrollbar": "1.4.0",
"prop-types": "^15.6.2",
"react-dropzone": "^10.1.4",
"react-google-maps": "9.4.5",
"react-loadable": "^5.5.0",
"react-redux": "^5.1.1",
"react-router-dom": "4.3.1",
"react-scripts": "1.1.5",
"react-select": "^2.1.2",
"react-sizeme": "^2.6.7",
"react-swipeable-views": "0.12.17",
"redux": "^4.0.1",
"redux-form": "^7.4.2",
"redux-thunk": "^2.3.0",
"reselect": "^4.0.0"
"babel": "^6.23.0",
"babel-polyfill": "^6.26.0",
"babel-preset-env": "^1.7.0",
"@babel/cli": "^7.0.0-beta.40",
"@babel/plugin-proposal-class-properties": "^7.3.0",
"babel-cli": "6.26.0",
"babel-core": "^7.0.0-bridge.0",
"babel-loader": "^7.1.5",
"babel-plugin-import-rename": "1.0.1",
"babel-plugin-lodash": "^3.3.2",
"babel-plugin-module-resolver": "3.1.1",
"babel-plugin-react-transform": "^3.0.0",
"babel-plugin-transform-object-rest-spread": "6.26.0",
"babel-plugin-transform-react-jsx": "6.24.1",
"compression-webpack-plugin": "^1.1.11",
"file-loader": "^4.0.0",
"redux-devtools-extension": "^2.13.5",
"webpack-bundle-analyzer": "^3.0.3"

I hope it help solve this issue...

eps1lon commented 5 years ago

Could you do a yarn why for react, @material-ui/core and @material-ui/styles (or npm ls if you`re using npm)?

A clonable repository would help a lot here.

RZsam commented 5 years ago

@eps1lon main project new project

only diffrence is package.json

eps1lon commented 5 years ago

Please create a minimal clonable repository with explicit steps to reproduce the issue. yarn dev will crash with missing files.

AnaBrade commented 5 years ago

Adding the dependency hoist-non-react-statics version 3.3.0 to our package.json solved the issue for us.

travi commented 5 years ago

will likely be a bit before i can try another attempt at upgrading, but based on the description of what that package is responsible for and this output from npm ls hoist-non-react-statics (pre-upgrade attempt), i'd say thats very likely our issue as well:

├─┬ @material-ui/core@3.9.3
│ └── hoist-non-react-statics@3.3.0 
├─┬ material-ui@0.20.2
│ └─┬ recompose@0.26.0
│   └── hoist-non-react-statics@2.5.5  deduped
├─┬ react-hot-loader@4.12.0
│ └── hoist-non-react-statics@3.3.0 
├─┬ react-jss@8.6.1
│ └── hoist-non-react-statics@2.5.5 
├─┬ react-redux@7.1.0
│ └── hoist-non-react-statics@3.3.0 
├─┬ react-router@3.2.3
│ └── hoist-non-react-statics@2.5.5  deduped
└─┬ recompose@0.30.0
  └── hoist-non-react-statics@2.5.5  deduped

thanks a ton for sharing @AnaBrade!

keithpickering commented 5 years ago

Still getting this error even with the hoist-non-react-statics fix. Comes from node_modules/@material-ui/core/esm/Input/Input.js:119:0

majid-amiri commented 4 years ago

I'm also having the problem even by adding hoist-non-react-statics to package.json. TypeError: Cannot read property 'root' of undefined or null refrence in Typography.js browser: IE 11

kjeske commented 4 years ago

Before installing hoist-non-react-statics try to remove node_modules and install them again.

minbelapps commented 4 years ago

It didn't help to remove node_modules before the installation.

minbelapps commented 4 years ago

The issue was Next.js. It's required to update the Next.js as well.

jfaraklit commented 4 years ago

Still an issue for us. Next.js, hoist-non-react-statics, removed /styles and only re-installed /core - deleted node_modules... non of them helped. When trying the build with npm dev watch, I see an error with withstyle(undefined). What is wrong here. Any more thoughts?

jfaraklit commented 4 years ago

Ok. It turned out we were importing babel-polyfill twice ... one in the component and one in the entry through webpack to support es6. Hope this helps.

travi commented 4 years ago

will likely be a bit before i can try another attempt at upgrading, but based on the description of what that package is responsible for and this output from npm ls hoist-non-react-statics (pre-upgrade attempt), i'd say thats very likely our issue as well:

in case it is helpful again, we did finally get back to attempting to upgrade and were successful after handling the hoisting problem with hoist-non-react-statics as well as react-transition-group. hoisting those dependencies did result in conflicts with v0 (i think mostly the hoisting of react-transition-group), which we are still running, but upgrading v0 components that showed symptoms of the conflict seems to have been successful in resolving those problems too. those are at least clear problems and work that we needed to get priority behind anyway.