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

Does HMR actually work with universal-router? #42

Open
DominikLevitsky opened this issue Aug 29, 2016 · 6 comments · May be fixed by #86
Open

Does HMR actually work with universal-router? #42

DominikLevitsky opened this issue Aug 29, 2016 · 6 comments · May be fixed by #86

Comments

@DominikLevitsky
Copy link

DominikLevitsky commented Aug 29, 2016


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@nightwolfz
Copy link

Yeah it works

@frenzzy frenzzy linked a pull request Mar 26, 2017 that will close this issue
20 tasks
@kpaxqin
Copy link

kpaxqin commented Sep 20, 2017

@nightwolfz

I have to say it depends.

Currently it's not easy to support HMR with static module import.

In the react-starter-kit , it requires dynamic import to make HMR work:

//routes/index.js
const routes = {
  path: '/',
  children: [
    {
      path: '/',
      load: () => import(/* webpackChunkName: 'home' */ './home'), //dynamic import
    }
  ]
}

//router.js
export default new Router(routes, {
  resolveRoute(context, params) {
    if (typeof context.route.load === 'function') {
      return context.route
        .load()
        .then(action => action.default(context, params)); // call load function
    }
    if (typeof context.route.action === 'function') {
      return context.route.action(context, params);
    }
    return null;
  },
});

//client.js
if (module.hot) {
  module.hot.accept('./router', () => { 
    onLocationChange(currentLocation);
  });
}

When you change code in home.js, the resolveRoute will be called through onLocationChange, and it calls load to import the changed module.

However, in cases with out dynamic import:

//routes/index.js
import home from './home'
const routes = {
  path: '/',
  children: [
    {
      path: '/',
      load: () => home,
    }
  ]
}

// client.js
import routes from 'routes';
let router = new UniversalRouter(routes);

if (module.hot) {
  module.hot.accept('./routes', () => {
    const nextRotues = require('./routes').default;
    // How can we replace routes in router ?
  });
}

The load method will return the old home module.

We can get new routes object with changed code in HMR callback, but how can we replace it in router instance?

Currently I'm replace it manually:

module.hot.accept('./routes', () => {
    // eslint-disable-next-line global-require
    const routes = require('./routes').default;
    router.root = Array.isArray(routes) ? { path: '', children: routes, parent: null } : routes;
    router.root.parent = null;
    renderApp(history.location);
  });

But I believe there should be better solution:

  1. expose an static resolve API like #86
  2. add setRoutes method on prototype of class UniversalRouter

Both of them looks great for me, and personally I would prefer the solution 1

@frenzzy
Copy link
Member

frenzzy commented Sep 20, 2017

@kpaxqin why don't you want to create a new instance of router after hot update?

// entry.js
import UniversalRouter from 'universal-router';
import routes from './routes';

let router = new UniversalRouter(routes);

// ...

if (module.hot) {
  module.hot.accept('./routes', () => {
    const nextRoutes = require('./routes').default;
    router = new UniversalRouter(nextRoutes);
  });
}

or

// router.js
import UniversalRouter from 'universal-router';
import routes from './routes';

export default new UniversalRouter(routes);
// entry.js
import router from './router';

// ...

if (module.hot) {
  module.hot.accept('./router');
}

@kpaxqin
Copy link

kpaxqin commented Sep 20, 2017

@frenzzy
My bad! I just made a mistake thinking router instance as a listener and avoid to create duplicated ones.

Actually it just resolve url and returns component, so its safe to re-create router instance.

Thanks for your reply.

@adamchenwei
Copy link

adamchenwei commented Oct 28, 2017

Anyone knows how to intercept the dynamic route value as well as route query params? I can't find the info regarding how to obtain those values when it is using load property.

@frenzzy
Copy link
Member

frenzzy commented Oct 30, 2017

@adamchenwei if you are talking about react-starter-kit resolve route approach, then you can may use context object as usual:

const route = {
  path: '/page',
  action(context) { // action method
    context.pathname // or context.<any>
  },
};

vs

const route = {
  path: '/page',
  load: () => import('./route-action.js'),
};

// route-action.js
function action(context) { // the same action method as above
  context.pathname // or context.<any>
}
export default action;

You may pass any data to routes via router.resolve({ pathname, ...context })

router.resolve({
  pathname: window.location.pathname,
  dynamicValue: 'here',
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants