scalable-react / scalable-react-boilerplate

:star: Scalable feature-first React micro-framework made for Udacity Alumni collaborative projects
https://scalable-react-boilerplate.herokuapp.com/
MIT License
259 stars 60 forks source link

Pages don't hot reload when not using grommet-udacity App component #64

Closed daan-nguyen closed 7 years ago

daan-nguyen commented 7 years ago

We're trying to remove the grommet-udacity dependency and have come across a seemingly odd issue that we haven't been able to quite figure out.

AppContainer/index.js imports grommet-udacity/components/App to act as an basic component wrapper. Judging from the source for App.js here, it looks like a pretty standard 'dumb' component. We have replaced it with what is effectively a div for the time being.

import React, { PropTypes, Component } from 'react';

class App extends Component { // eslint-disable-line react/prefer-stateless-function
  render() {
    return (
      <div>
        {this.props.children}
      </div>
    );
  }
}

App.propTypes = {
  children: PropTypes.node,
};

export default App;

However, this results in changes to pages no longer causing a visual refresh in the browser. HMR appears to run, as the normal console logs occur and there are XHR network logs and logs of the .json from the hot update .js being pulled (where you can actually see the changes in the code) but visually it doesn't update without a hard browser refresh. Changes to components and containers visually update fine.

The only new warning that shows up is one from react-router: browser.js:49 Warning: [react-router] You cannot change <Router routes>; it will be ignored

However, this warning exists for components and containers too and it doesn't prevent them from visually updating.

Putting the original App import from grommet-udacity/components/App appears to resolve this issue. I'm pretty stumped as to why this may be happening or what App from grommet-udacity does differently. We're making use of the sans-apollo branch with only some initial cleanup changes so far, found here.

We are using:

RyanCCollins commented 7 years ago

Hey @daan-nguyen , that's odd I have never seen that before. Maybe you can get rid of the App component and just surround the component with a regular div? I found this as a potential guide to solving it located here.

I am working on finishing up a project right now, but I will try to see if I can reproduce this later on tonight. It sounds like the Routes are someone being dynamically changed and thus can't be hot-reloaded due to an error of some sort. See if that link helps and if not, I will see what I can do later tonight.

daan-nguyen commented 7 years ago

Awesome, thanks for the lead. Yeah it strikes me as maybe we're missing something when we strip away the current components. Will have a bit of a dig down this line

daan-nguyen commented 7 years ago

Looks like that was the right track, we've been able to resolve it as per this comment by adding a random key onto the router in route.js.

Whilst this results in a reset of any component state, it doesn't appear to affect redux state which is probably the more important one

RyanCCollins commented 7 years ago

Hey @daan-nguyen. this must be a brand new issue since now I am seeing it for the first time ever in other apps of mine. I think the best solution comes at the end of that comment feed. The idea is to use the built in AppContainer from React Hot Loader. See here. I will be updating my projects this coming week with this solution. Let me know how it goes for you!

RyanCCollins commented 7 years ago

@daan-nguyen I've updated the sans-apollo branch with these fixes. Thanks for your patience. See here.

RyanCCollins commented 7 years ago

This is resolved, I believe, so I am going to close it. Please reopen it if you have any more issues.

daan-nguyen commented 7 years ago

Oddly I tried this and it doesn't seem to resolve it for us, App from grommet-udacity must be providing something a plain div doesn't. Our equivilent AppContainer looks like this:

class AppContainer extends Component { // eslint-disable-line react/prefer-stateless-function
  render() {
    return (
      <ReactHotLoader>
        <div>
          {React.cloneElement(this.props.children, this.props)}
        </div>
      </ReactHotLoader>
    );
  }
}

In any case we're able to get around it with the router key so I'm happy enough with that for the time being, we might revisit it a little later down track when react-hot-loader comes out of beta perhaps. Thanks a lot for your help!

RyanCCollins commented 7 years ago

You're quite welcome @daan-nguyen .

That's very strange. I would imagine that it's your node version, but I can't say for sure. I'm sticking with 6.9.1 until the react packages stabilize on later versions. Let me know if there is anything else that I can do to help.