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

Having problems after upgrading to v4. #15898

Closed
Ericnr opened this issue May 27, 2019 · 33 comments
Closed

Having problems after upgrading to v4. #15898

Ericnr opened this issue May 27, 2019 · 33 comments
Labels
status: waiting for author Issue with insufficient information

Comments

@Ericnr
Copy link

Ericnr commented May 27, 2019

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

eps1lon commented May 27, 2019

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

@eps1lon eps1lon added the status: waiting for author Issue with insufficient information label May 27, 2019
@oliviertassinari
Copy link
Member

oliviertassinari commented May 27, 2019

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

@Ericnr
Copy link
Author

Ericnr commented May 28, 2019

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

@Ericnr Ericnr closed this as completed May 28, 2019
@Kuirak
Copy link

Kuirak commented May 28, 2019

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

ztoben commented May 28, 2019

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

@travi
Copy link

travi commented May 29, 2019

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

travi commented May 29, 2019

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

travi commented May 29, 2019

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

RZsam commented Jun 13, 2019

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

@travi
Copy link

travi commented Jun 13, 2019

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

eps1lon commented Jun 13, 2019

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

@ztoben
Copy link

ztoben commented Jun 13, 2019

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

eps1lon commented Jun 13, 2019

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

travi commented Jun 13, 2019

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

travi commented Jun 13, 2019

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:

  • SSR on hapi using https://github.com/travi/hapi-react-router/ but the material specific details are contained within our private app
  • we bundle browser and node with webpack. both bundles reproduce the problem
  • when developing locally, we use webpack dev server for the browser, but nodemon and simple babel transpilation instead of the webpack bundling for the server. in this mode, we only see the error in the browser, not from the server
  • our components are in a separate npm package from the application. the separate package is bundled with rollup and produces an es module bundle, set as module in the package.json, and a common-js bundle, set as main in the package.json. we couldnt determine definitively whether it contributed or not, but it is worth noting that the es version would have been consumed by webpack in those final bundles, but the cjs bundle would have been consumed when in development mode using nodemon for the server
    • we were actually in the middle of extracting additional component packages when we found classname mismatches between the server render and the browser rehydration (that resolved themselves after unmounting and remounting of components), which was our motivation to try to upgrade to v4 since we were trying to configure the app to use the new styles engine when v4 dropped in hopes that it would generate the classnames more consistently between server and browser (we saw mention of hashed classnames rather than sequential)

@eps1lon
Copy link
Member

eps1lon commented Jun 13, 2019

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

travi commented Jun 13, 2019

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

travi commented Jun 13, 2019

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

RZsam commented Jul 1, 2019

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

RZsam commented Jul 1, 2019

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

eps1lon commented Jul 1, 2019

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

RZsam commented Jul 1, 2019

@eps1lon main project
new project

only diffrence is package.json

@eps1lon
Copy link
Member

eps1lon commented Jul 1, 2019

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

@AnaBrade
Copy link

AnaBrade commented Jul 2, 2019

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

@travi
Copy link

travi commented Jul 2, 2019

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

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

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

kjeske commented Oct 9, 2019

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

@minbelapps
Copy link

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

@minbelapps
Copy link

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

@jfaraklit
Copy link

jfaraklit commented Nov 14, 2019

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

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

travi commented Dec 3, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting for author Issue with insufficient information
Projects
None yet
Development

No branches or pull requests