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

Asynchronous React Router v3 routes fail to hot-reload #288

Closed
phyllisstein opened this issue May 5, 2016 · 32 comments
Closed

Asynchronous React Router v3 routes fail to hot-reload #288

phyllisstein opened this issue May 5, 2016 · 32 comments

Comments

@phyllisstein
Copy link
Contributor

Hey there! I'm having some difficulty getting [email protected] to hot-reload changes to views that are passed to React Router as lazy-loaded POJOs. Updating a view class triggers the usual console messages and throws the warning about changing <Router history>, but the rendered output doesn't refresh. Curiously, though, changing the components that the views require immediately hot-reloads them, even within the selfsame non-reloading async views.

Here's a quick screencast demonstrating the issue:

...and here's the code base that screencast's taken from: https://github.com/phyllisstein/hildy/tree/react-hot-3. (The master branch of which repo is still using react-transform-hmr without any evident issues.)

I'd be grateful for any suggestions you can provide, and happy to offer more troubleshooting info if I can! Thanks for looking into this.

@gaearon gaearon added this to the v3.0 milestone May 12, 2016
@gaearon
Copy link
Owner

gaearon commented May 12, 2016

Thanks for a repro case. Here’s another great repro case: gaearon/react-hot-boilerplate#61 (comment).

We’ll need to look into this before releasing 3.0. Probably something funky related to how require.ensure works with hot reloading. Also would be great to check if it’s still an issue with Webpack 2 ES modules.

@phyllisstein
Copy link
Contributor Author

It looks like this has something to do with using the es2015-webpack Babel preset, so I think you may be right. Another repo I've been working with (https://github.com/ignota/issue-0/tree/react-hot-3) is able to do some view reloading using es2015-loose in place of es2015-webpack-loose. Seems to still have some issues---in particular, the hot update usually fires once or twice then stops---but I think they could have something to do with redux-saga. Gonna try switching to redux-thunk and seeing if that improves things at all.

@phyllisstein
Copy link
Contributor Author

redux-thunk, lovely though it is, didn't get me too much further. The first change to a view registers with HMR, but logs a warning to the console: "[HMR] Unexpected require(583) from disposed module 581." (581 is my main App component, and 583 is the view that's being hot-updated.) After that, every subsequent change is detected by the dev server and the hot reloader and loaded by the browser, but fails to update the view. Quick screencast again:

2016-05-21 05_35_49

@MrEfrem
Copy link

MrEfrem commented Jun 12, 2016

With webpack 2 in System.import HMR too doesn't work.

@hoschi
Copy link

hoschi commented Jun 13, 2016

Beta.2 has the same issue.

MrEfrem [email protected] schrieb am So., 12. Juni 2016, 22:41:

With webpack 2 in System.import HMR too doesn't work.


You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub
#288 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAJ9OGWnHGWqhaw-nXrta7-ZB_S0hr26ks5qLG8VgaJpZM4IXvhK
.

@leecade
Copy link

leecade commented Jun 14, 2016

👍

@hoschi
Copy link

hoschi commented Jun 16, 2016

Now it stopped working for me with beta.0. I had this case already some days ago, which was fixed by a rm -rf node_modules; npm install but no luck this time. I cleaned up my /tmp, npm cache, babel-cache, but still broken.

@rosskevin
Copy link

When routes/history are imported in the index.js and passed down as props to the <App/>, it appears to kill hot reloading and issues full reload needed. I only moved my routes/history up to the index to avoid [react-router] You cannot change <Router routes>; it will be ignored.

I'll move my routes/history back as hot reloading is more important, then I'll try to add back the async routes.

@rosskevin
Copy link

rosskevin commented Jun 16, 2016

With async routes, it appears that Foo will reload, while FooComponent will not reload:

import React from 'react'

export default function (props:any) {
  return (<div>Foo3</div> )
}
import React, {Component} from 'react'

export default class FooComponent extends Component {
  render () {
    return (<div>FooComponent2</div>)
  }
}

@dkulichkin
Copy link

dkulichkin commented Aug 29, 2016

@dferber90, @gaearon, @rosskevin
Guys, what is the last status with an issue? Is it still really no way to leverage hot reloading and chunking working together simultaneously when in development so f.e. when I want a hot reloading to work I need to sacrifice chunking and assemble everything into a main bundle?

if (process.env.NODE_ENV !== 'production') {
    require('...')
    require('...')
    ... // and so on - listing of all async top react components, f.e. redux containers
}

having a routing config in the form of:

const loadContainerAsync = bundle => (loc, cb) => {
    bundle(component => cb(null, component));
};

const routes = {
    path: '/',
    component: WrapperContainer,
    indexRoute: { component: LoginContainer },
    childRoutes: [
        {
            path: 'dashboard',
            name: 'Dashboard',
            getComponent: loadContainerAsync(require('bundle?lazy&name=...'))  // or simply using react-router loader for simplicity -  does not matter
        },
        ...
]

@dferber90
Copy link

@dkulichkin Yes, that workaround seems to be the only way for now. As the sacrifice of not being able to use code-splitting is only for development, this isn't that much of a problem is it?

Of course it would be nice if this wasn't necessary, but I don't even know why that fixes the problem in the first place so a lot of investigation would be necessary.

The code you provided isn't helpful because it leaves out the important parts (importing the component that is being split out in the second snippet). Your snippets only contain ... there.
See gaearon/react-hot-boilerplate#61 (comment) for how to set things up.

@dkulichkin
Copy link

dkulichkin commented Aug 30, 2016

@dferber90 the main point of having a chunking in development is the profiling, f.e. with help of such tools like webpack-dashboard. That allows for instance immediately see an impact of the changes to files size without necessity to build and switch to prod. So the drawback is pretty much serious.

Anyway what's a main bottleneck in this issue making only pure stateless components reloaded in chunks? Router, react-hot-reloader or bundler?

Regarding '...' character in my snippet - that's just skipped pathes to component files, nothing else

@sthzg
Copy link

sthzg commented Sep 9, 2016

I ran into the problem with hot reloading async routes and updated by additionally registering amodule.hot.accept in the require.ensure() callbacks. E.g. like this

getChildRoutes(partialNextState, callback) {
  require.ensure([], function (require) {

    if (module.hot) {
      module.hot.accept('../routes/Blog', () => 
        callback(null, [require('../routes/Blog').default]);
    }

    callback(null, [
      require('../routes/Blog').default,
    ]);
  });
},

@dkulichkin
Copy link

@sthzg Did you by chance also manage to make it working with a webpack-bundle loader?

@sthzg
Copy link

sthzg commented Sep 21, 2016

@dkulichkin No, sorry. So far I have only tried it with an app that uses code splitting based on the react-router async routes.

@calesce
Copy link
Collaborator

calesce commented Oct 18, 2016

Just wanted to add that I was playing around with require.ensure and RR, using @sthzg's solution, and it seems to work fine with getChildRoutes, but not getComponent.

It looks like the getComponent callback can't be called multiple times (tested out with setTimeout loading a different component). Another reason to move on to RR4? 😛

@flut1
Copy link

flut1 commented Nov 28, 2016

I worked around the getComponent() issue by adding a key prop to Router that changes on each hot update. This will force a different Router instance to be created, and re-executing getComponent() calls. This also fixed the You cannot change <Router router> warnings. Not sure if creating new Router instances causes other (performance) issues but it seems to work for me.

// create key variable
let routerKey = 0;

// initial router rendering
<Router key={routerKey} ... />

if (module.hot) {
 module.hot.accept('./containers/Root', () => {
  ...
  // first bump the key
  renderKey++;
  // re-render
  render(
    <AppContainer>
       ...
      <Router key={renderKey} ... />
    </AppContainer>
  , mountNode);
 });
}

@calesce
Copy link
Collaborator

calesce commented Nov 28, 2016

@flut1 yeah that will allow updates to happen, but component state is lost because it's deeply re-rendering all of the components, so it's not the ideal workaround.

@phyllisstein
Copy link
Contributor Author

phyllisstein commented Jan 19, 2017

Grain of salt and all, but it seems this issue has mysteriously resolved itself with Webpack 2.2.0. Previously, in order to make hot reloading work, I was manually adding chunks of code like this to any parent view that asynchronously loaded children:

if (process.env.NODE_ENV === 'development') {
  require('./child1');
  require('./child2');
  require('./child3');
}

I removed all such lines on a whim to see what it'd break, and much to my surprise and delight, HMR continued to function beautifully.

I don't think anything relevant has changed in my setup other than the Webpack version, but this wasn't carefully tested. Would be glad to see if others could corroborate my story.

@MrEfrem
Copy link

MrEfrem commented Jan 19, 2017

Good news.

@jharris4
Copy link

jharris4 commented Jan 22, 2017

I ran into the same issue where HMR was not working with react-router, and I managed to work around the issue, so I thought I'd share my solution:

class SplitComponent extends Component {

  constructor(props) {
    super(props);
    this.state = { Component: false }
  }

  componentWillMount() {
    const { componentPath } = this.props;

    const that = this;

    // chunks will be given file names like:
    // 0.route-PagesRoot.js
    // 2.route-PagesHome.js
    require('bundle-loader?lazy&name=route-[name]!./' + componentPath)(
      Component => {
        that.setState( { Component });
      }
    );
  }

  render() {
    const { Component } = this.state;
    if (Component) {
      return <Component {...this.props}/>;
    }
    else {
      return false;
    }
  }
}

SplitComponent.propTypes = {
  componentPath: PropTypes.string.isRequired
};

function loadComponent(componentPath) {
  return function (location, callback) {
    callback(null, props => <SplitComponent {...props} componentPath={componentPath}/> );
  };
}

Used with the Router like this:

<Router history={browserHistory}>
  <Route path="/" component={App}>
    <IndexRoute component={Home}/>
    <Route path="/pages" getComponent={loadComponent('pages/PagesRoot')}>
      <IndexRoute getComponent={loadComponent('pages/PagesHome')}/>
      <Route path="/pages/:id" getComponent={loadComponent('pages/PagesDetail')}/>
    </Route>
    <Route path="/chapters" getComponent={loadComponent('chapters/ChaptersRoot')}>
      <IndexRoute getComponent={loadComponent('chapters/ChaptersHome')}/>
      <Route path="/chapters/:id" getComponent={loadComponent('chapters/ChaptersDetail')}/>
    </Route>
  </Route>
</Router>

@Dreem-Devices
Copy link

Dreem-Devices commented Jan 23, 2017

@sthzg Made it for me with async routes and react-router 3.x (Did not try yet react-router 4.x with history 4.x)
Unfortunately transition to webpack 2.2 broke live reloading, In the console, I only get

client:37 [HMR] Waiting for update signal from WDS...
client:37 [WDS] Hot Module Replacement enabled.
client:37 [WDS] App updated. Recompiling...
client:37 [WDS] App hot update...

It seems like the dev-server.js code is not executed.
I don't know what I'm missing.
I'm using WebpackDevServer node API for launching my development server, and live reloading worked well using transform-hmre or react-hot-loader (though react-hot-loader need module.hot.accept for all my childRoutes and the first component to be synchrone...)

The logs missings are:

  • with hmre
dev-server.js:55 [HMR] Checking for updates on the server...
index.js:81 [React Transform HMR] Patching Index
log-apply-result.js: 20 [HMR] Updated modules:
log-apply-result.js: 22 [HMR]   - 598
  • with react-hot-loader

dev-server.js:55 [HMR] Checking for updates on the server...
log-apply-result.js:20 [HMR] Updated modules:
log-apply-result.js:22 [HMR]  - 806
log-apply-result.js:22 [HMR]  - 804
log-apply-result.js:22 [HMR]  - 320
dev-server.js:41 [HMR] App is up to date.

If anyone has an idea? Does someone has the same issue?
I've just updated to webpack 2 following the migration guide.

What do you think @gaearon? (sorry for hard poking)

EDIT : Just found the issue... Events are not the same between webpack and webpack 2. I just needed to update my webpack-dev-server dependency to 2.2.0...
Just spent 1.5 day on this issue... There is nothing on the doc or on the webpack-dev-server github project...
But I learned a lot about webpack hot reloading and events emission by the way :)
It works well with hmre even if this preset includes require.resolve.
Works great with react-hot-loader v3, but does not work works perfectly with current react-loader, hot reloading is done but I have some warning in the console : reactjs/react-router-redux#179

Thanks for these tools :)

@satazor
Copy link

satazor commented Feb 21, 2017

Hello guys!

For those who want to make react-router's routes to work with react-hot-loader without sacrificing generating chunks for each async route:

// Example bellow is for webpack2

const dev = true;  // Should be false when building for production

// later..
    module: {
        rules: [
            {
                test: /\.js$/,
                exclude: /node_modules/,
                use: [
                    {
                        loader: 'babel-loader',
                        options: {
                             // babel options here
                        },
                    },
                    dev ? 'preprocess-loader?+DEV' : 'preprocess-loader',
                ],
            },
           // Other rules...
        ],
    },

And then use preprocess in your async routes:

// Example bellow uses import() but works with require.ensure() too

{
   path: 'about',
   // Ensure component is required synchronously in DEV so that it works with react-hot-loader
   // When not in DEV, we want to make sure that it generates a chunk
   // @ifdef DEV
   component: require('./About').default,
   // @endif
   getComponent: () => import('./About'),
}

Enjoy!

@Jessidhia
Copy link

Jessidhia commented Feb 22, 2017

Using anything that is not an ES6 import is probably the biggest complicating factor for hot reloading.

Also note that, if you save the require('./About').default in the component property, it will never ever get updated if you don't write code to do it. This would also be the case with ES6 import though (imports are live, but if you save a copy of the current value elsewhere the copy isn't).

@satazor
Copy link

satazor commented Feb 22, 2017

@Kovensky I've tested using ES6 imports like so:

// @ifdef DEV
import About from './About';
// @endif

export default [
    {
        path: 'about',
        getComponent: () => {
            // @ifdef DEV
            return Promise.resolve(About);
            // @endif
            return import('./About');
        },
    },
];

but it always needs module.hot.accept to integrate with react-router (just like using require), see: https://github.com/moxystudio/react-with-moxy/blob/master/src/client-renderer.js#L49

I've got all of this working great in https://github.com/moxystudio/react-with-moxy if you guys want to see how all the pieces are glued together.

@wkwiatek wkwiatek changed the title Asynchronous React Router routes fail to hot-reload under beta.1. Asynchronous React Router v3 routes fail to hot-reload Feb 22, 2017
@kettanaito
Copy link

Is there any update regarding this issue?

So far there has been only a few suggestions, each having its own drawbacks and all being but workarounds:

Provide "key" prop to <Route />

Fixed the problem, but as mentioned causes deep re-render, therefore loosing components' state.
However, you can find this workaround in officially listed examples of react-hot-loader (i.e. here).

Using asyncComponent from react-async-component

As suggested here, it is possible to wrap the modules you wish to load async with asyncComponent and pass them as component prop to Route directly. This will eliminate hot-reloading issue, however forces you to create additional files doing nothing but resolving modules async.

Various other workarounds

Which are not meant for good code maintenance.

So, it this somehow being solved, or already has been solved with the new release of react-hot-loader?

@morajabi
Copy link

@kettanaito Why don't you migrate to react-router 4?

@kettanaito
Copy link

@morajabi I haven't got enough time to read about it. So far, I don't see how to migrate my code to use react-router 4, especially since my SSR relies on match, which was removed from the fourth version.

@kettanaito
Copy link

@morajabi sorry, there is nothing about SSR in the migration guide.
Regardless, the issue with v3 still remains, and there should be some statement how to cope with it.

@morajabi
Copy link

morajabi commented Aug 17, 2017

@kettanaito You're right, but read this:

Known limitations

React Router v3 is not fully supported (e.g. async routes). If you want to get most of React Hot Loader, consider switching to React Router v4. If you want to understand the reasoning, it's good to start in React Router v4 FAQ

And maybe it's worth a reading: https://github.com/ReactTraining/react-router/tree/v4.0.0-beta.8#why-did-you-get-rid-of-feature-x

@gregberge
Copy link
Collaborator

I close it. React Router v4 is supported and stable.

erchaves added a commit to erchaves/kyt that referenced this issue Jul 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests