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

Hot Module Replacement with MuiThemeProvider always errors #9287

Closed
1 task done
jjwilliams42 opened this issue Nov 23, 2017 · 13 comments
Closed
1 task done

Hot Module Replacement with MuiThemeProvider always errors #9287

jjwilliams42 opened this issue Nov 23, 2017 · 13 comments
Labels
status: waiting for author Issue with insufficient information

Comments

@jjwilliams42
Copy link

jjwilliams42 commented Nov 23, 2017

  • I have searched the issues of this repository and believe that this is not a duplicate.

Possibly related:
#8353
#8878

Expected Behavior

Whenever I change a value in my const styles = theme => ({..}) in Drawer (or anything else in the application), material-ui withStyles should not throw an error.

Current Behavior

Anytime I change a value in my styles definition (or anything for that matter), after a hot module replacement, the following error occurs. The UI still updates as expected, but the errors start stacking up.

Uncaught TypeError: Cannot read property 'refs' of undefined
    at ProxyComponent.detach (withStyles.js:339)
    at ProxyComponent.detach (createPrototypeProxy.js:44)
    at ProxyComponent.<anonymous> (withStyles.js:251)
    at commitCallbacks (react-dom.development.js:6155)
    at commitLifeCycles (react-dom.development.js:8812)
    at commitAllLifeCycles (react-dom.development.js:9967)
    at HTMLUnknownElement.callCallback (react-dom.development.js:540)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:579)
    at invokeGuardedCallback (react-dom.development.js:436)
    at commitRoot (react-dom.development.js:10071)
react-dom.development.js:9769 The above error occurred in the <withStyles(ResponsiveDrawer)> component:
    in withStyles(ResponsiveDrawer) (created by App)
    in MuiThemeProvider (created by MuiThemeProviderWrapper)
    in MuiThemeProviderWrapper (created by App)
    in App
    in Router (created by ConnectedRouter)
    in ConnectedRouter
    in Provider
    in AppContainer
Cannot read property 'refs' of undefined
    at ProxyComponent.detach (withStyles.js:339)
    at ProxyComponent.detach (createPrototypeProxy.js:44)
    at ProxyComponent.<anonymous> (withStyles.js:251)
    at commitCallbacks (react-dom.development.js:6155)
    at commitLifeCycles (react-dom.development.js:8812)
    at commitAllLifeCycles (react-dom.development.js:9967)
    at HTMLUnknownElement.callCallback (react-dom.development.js:540)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:579)
    at invokeGuardedCallback (react-dom.development.js:436)
    at commitRoot (react-dom.development.js:10071)

Steps to Reproduce (for bugs)

Unfortunately this is a HMR issue, and I can't workup a codesandbox.io example. Here is my relevant setup (maybe I am doing something wrong?)

Steps

  1. Build / Run
  2. Modify any part of app under the hot.module.accept (styles, content, etc...)
  3. Error occurs but UI updates OK (the UI still updates fine - but the error is still thrown)

App.js

import React from 'react';
import ReactDOM from 'react-dom';
import { MuiThemeProvider, createMuiTheme } from 'material-ui/styles';
import purple from 'material-ui/colors/purple';
import blue from 'material-ui/colors/blue';
import green from 'material-ui/colors/green';
import red from 'material-ui/colors/red';

import Drawer from '../Drawer';
import DevTools from '../DevTools';

const theme = createMuiTheme({
  palette: {
    primary: green, // Purple and green play nicely together.
    secondary: {
      ...green,
      A400: '#00e677',
    },
    error: red,
  },
  overrides: {
    MuiDrawer: {
      docked: {
        flex: '0 0 auto',
        height: '100%'
      }
    }
  }
});

class App extends React.Component {
  render() {
    return (
        <MuiThemeProvider theme={theme}>
          <Drawer />
        </MuiThemeProvider>
    );
  }
}
export default App;

Drawer.js

import React from 'react';
import PropTypes from 'prop-types';
import { withStyles } from 'material-ui/styles';
import Drawer from 'material-ui/Drawer';
import AppBar from 'material-ui/AppBar';
import Toolbar from 'material-ui/Toolbar';
import List from 'material-ui/List';
import Typography from 'material-ui/Typography';
import IconButton from 'material-ui/IconButton';
import Hidden from 'material-ui/Hidden';
import Divider from 'material-ui/Divider';
import MenuIcon from 'material-ui-icons/Menu';
import Grid from 'material-ui/Grid';
import Paper from 'material-ui/Paper';
import { mailFolderListItems, otherMailFolderListItems } from './tileData';

const drawerWidth = 240;

const styles = theme => ({
  root: {
    width: '100%',
    marginTop: 4,
    zIndex: 1,
    height: '100%',
    overflow: 'hidden',
  },
  appFrame: {
    position: 'relative',
    display: 'flex',
    width: '100%',
    height: '100%',
  },
  appBar: {
    position: 'absolute',
    marginLeft: drawerWidth,
    [theme.breakpoints.up('md')]: {
      width: `calc(100% - ${drawerWidth}px)`,
    },
  },
  navIconHide: {
    [theme.breakpoints.up('md')]: {
      display: 'none',
    },
  },
  drawerHeader: theme.mixins.toolbar,
  drawerPaper: {
    width: 250,
    height: '100%',
    [theme.breakpoints.up('md')]: {
      width: drawerWidth,
      position: 'relative',
      height: '100%',
    },
  },
  content: {
    backgroundColor: theme.palette.background.default,
    width: '100%',
    padding: theme.spacing.unit * 3, 
    height: 'calc(100% - 56px)',
    marginTop: 56,
    [theme.breakpoints.up('sm')]: {
      height: 'calc(100% - 64px)',
      marginTop: 64,
    },
  },
  gridRoot: {
    flexGrow: 1
  },
  gridPaper: {
    padding: theme.spacing.unit * 2,
    height: '100%'
  },
  docked: {
    height: '100%'
  }
});

class ResponsiveDrawer extends React.Component {
  state = {
    mobileOpen: false,
  };

  handleDrawerToggle = () => {
    this.setState({ mobileOpen: !this.state.mobileOpen });
  };

  render() {
    const { classes, theme } = this.props;

    const drawer = (
      <div>
        <div className={classes.drawerHeader} />
        <Divider />
        <List>{mailFolderListItems}</List>
        <Divider />
        <List>{otherMailFolderListItems}</List>
      </div>
    );

    return (
      <div className={classes.root}>
        <div className={classes.appFrame}>
          <AppBar className={classes.appBar}>
            <Toolbar>
              <IconButton
                color="contrast"
                aria-label="open drawer"
                onClick={this.handleDrawerToggle}
                className={classes.navIconHide}
              >
                <MenuIcon />
              </IconButton>
              <Typography type="title" color="inherit" noWrap>
                Responsive drawer
              </Typography>
            </Toolbar>
          </AppBar>
          
          <Hidden mdUp>
            <Drawer
              type="temporary"
              anchor={theme.direction === 'rtl' ? 'right' : 'left'}
              open={this.state.mobileOpen}
              classes={{
                paper: classes.drawerPaper,
              }}
              onRequestClose={this.handleDrawerToggle}
              ModalProps={{
                keepMounted: true, // Better open performance on mobile.
              }}
            >
              {drawer}
            </Drawer>
          </Hidden>
          <Hidden mdDown implementation="css">
            <Drawer
              type="permanent"
              open
              classes={{
                docked: classes.docked,
                paper: classes.drawerPaper,
              }}
            >
              {drawer}
            </Drawer>
          </Hidden>
          <main className={classes.content}>
              <Grid container className={classes.gridRoot}>
                <Grid item xs={12}>
                  <Grid container alignItems='stretch'>
                    <Grid item key={1}>
                      <Paper className={classes.gridPaper}>
                        {'Content 1'}
                      </Paper>
                    </Grid>
                    <Grid item key={2}>
                      <Paper className={classes.gridPaper}>
                        {'Content 2'}
                      </Paper>
                    </Grid>
                    <Grid item key={3}>
                      <Paper className={classes.gridPaper}>
                        {'Content 3'}
                      </Paper>
                    </Grid>
                  </Grid>
                  
                </Grid>
              </Grid>
          </main>
        </div>
      </div>
    );
  }
}

ResponsiveDrawer.propTypes = {
  classes: PropTypes.object.isRequired,
  theme: PropTypes.object.isRequired,
};

export default withStyles(styles, { withTheme: true })(ResponsiveDrawer);

Context

If I remove MuiThemeProvider from App.js, all works as expected with the default theme.

Doing some debugging, it looks like this line, at some point returns undefined / null:

var sheetManagerTheme = sheetManager.get(theme)
https://github.com/mui-org/material-ui/blob/023fee9092571e98814e64221015c6b669353fbe/src/styles/withStyles.js#L267

in the detach method.

Your Environment

Tech Version
Material-UI ^1.0.0-beta.21
React 16.0.0
browser Chrome latest
@mbrookes mbrookes added the core label Nov 24, 2017
@oliviertassinari
Copy link
Member

@jackjwilliams Could you set up a reproduction repository so we can dig into it?

@jjwilliams42
Copy link
Author

I'll go ahead and close this, at some point I made a bunch of changes and this started working fine with HMR.

@chaddjohnson
Copy link

chaddjohnson commented Dec 13, 2017

I am experiencing exactly this same issue with material-ui@next -- even with this patch in place.

@chaddjohnson
Copy link

@jackjwilliams Mind sharing what changes might have fixed this?

@jjwilliams42
Copy link
Author

@chaddjohnson, I wish I knew :/. I reorganized a bunch of stuff and after a few days I happened to notice the error went away.

Sorry! I hate it when an error goes away and I don't know why lol.

@jjwilliams42
Copy link
Author

I will say that at some point I realized that asp.net core 2's HMR was conflicting with my HotModuleReplacementPlugin, so I removed it from my webpack config. I'm not sure if that's what it was, it was just a realization that I came to (that the two plugins are mutually exclusive, not complimentary).

@chaddjohnson
Copy link

chaddjohnson commented Dec 14, 2017

I changed

export default withStyles(styles, {withTheme: true})(Navigation);

to

export default withTheme()(withStyles(styles)(Navigation));

in my component wrapped by <MuiThemeProvider>, and now I just get a warning.

@alechstong
Copy link

I had a similar issue recently (same error message but different setup). I was using express + webpack-hot-middleware + react-hot-loader. It was working fine when I was using it with Mui v0.xx, but when I was tring to migrate to Mui next something broke.

And it is also the same with me that if I don't use MuiThemeprovider, or if I don't pass theme to styles, there's no error, and the setup would work.

I ended up ditching react-hot-loader, and only use webpack-hot-middleware for hmr

@waynepan
Copy link

waynepan commented Feb 2, 2018

Using react hot loader v4 (possibly 3), I think this can be solved by moving the MuiThemeProvider one level up. eg
render( <MuiThemeProvider theme={theme}> <App /> </MuiThemeProvider>, document.getElementById('app'))

Then having be the hot reload root component.

cooperka added a commit to cooperka/neuro-name that referenced this issue Feb 21, 2018
@ilhambrata
Copy link

So do I.. now the error log is gone 🙏🏼

I changed

export default withStyles(styles, {withTheme: true})(Navigation);
to

export default withTheme()(withStyles(styles)(Navigation));
in my component wrapped by , and now I just get a warning.

@chaddjohnson
Copy link

chaddjohnson commented Mar 4, 2018

@ilhambrata I was able to get around the problem using

export default withStyles(styles, {withTheme: true})(OrdersPage);

and moving <MuiThemeProvider> up one level outside app.js into index.js. Here is my index.js file:

'use strict';

import React from 'react';
import ReactDOM from 'react-dom';
import {AppContainer} from 'react-hot-loader';
import {Provider} from 'react-redux';
import {MuiThemeProvider} from 'material-ui/styles';
import theme from './theme';
import store from './store';
import App from './app';

const render = (Component) => {
    ReactDOM.render(
        <MuiThemeProvider theme={theme}>
            <Provider store={store}>
                <AppContainer>
                    <Component />
                </AppContainer>
            </Provider>
        </MuiThemeProvider>,
        document.getElementById('root')
    );
};

function bootstrap() {
    // Render the app now that the token is fetched.
    render(App);

    // Initialize hot module reloading.
    if (module.hot) {
        module.hot.accept('./app', () => {
            render(require('./app').default);
        });
    }
}

// Make it so.
bootstrap();

and my app.js file:

'use strict';

import React from 'react';
import {CookiesProvider} from 'react-cookie';
import AppRouter from './components/appRouter';

const App = () => (
    <CookiesProvider>
        <AppRouter />
    </CookiesProvider>
);

export default App;

@klapantius
Copy link

First of all thanks for the great hints.
I also applied the transition from withStyles(styles)(<my component>) to withTheme()(withStyles(styles)(<my component>)), and it solved my problem completely. Also no warning.
I'm using MuiThemeProvider in my index.js and I did the above change for all "first level" components used there. Further components don't need this, I can change them and the hot-reload works as fine as it did before the issue.

@Sawtaytoes
Copy link

Sawtaytoes commented Mar 19, 2018

Is there a solution for this if your MuiThemeProvider is wrapped in AppContainer?

I've got an npm package doing Webpack rendering, and I'm simply passing my themed app into the render function which wraps it in AppContainer.

I also tried doing withTheme()(withStyles(styles)(<Component>)) around the first component with styles down the chain and that didn't fix the issue at all for some reason.

From my testing, the only time I get these errors are when using MuiThemeProvider.

danielhoegel added a commit to danielhoegel/dfs-verwaltung that referenced this issue Oct 4, 2018
Error Message: "cannot read property refs of undefined"
Solution: mui/material-ui#9287 (comment)
@oliviertassinari oliviertassinari added the status: waiting for author Issue with insufficient information label Jan 4, 2020
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

9 participants