Skip to content

webpack2 + hot-reload: Not works with System.import #303

Closed
@aight8

Description

@aight8

The change is not applied. Webpack seems to generate a chunk with a new id with the changes for the chunk when using System.import.

Activity

calesce

calesce commented on Sep 14, 2016

@calesce
Collaborator

Hi, I'd like to take a look at this. I have yet to mess around much with Webpack 2. Do you have a (minimal) project that reproduces the issue?

ctrlplusb

ctrlplusb commented on Sep 27, 2016

@ctrlplusb
Contributor

Hi @aight8 and @calesce

I recently got my boilerplate working properly with System.imports without any RHL workarounds.

Originally I had issues when doing static path System.imports, like so:

System.import('../components/Foo')

RHL would not hot reload them the moment I navigated to another route of my application. As a workaround I used to have to do hard require statements whilst in dev mode:

if (process.env.NODE_ENV === 'development') {
  require('../components/Foo');
}

This was tedious to say the least as my async routes grew.

I then introduced an expression based version of my System.import, for example:

function asyncAppViewResolver(viewName) {
  return (nextState, cb) =>
    System.import('../components/App/views/' + viewName + '/index.js')
      .then(module => cb(null, module.default))
      .catch(err => console.log(err));
}

const routes = (
  <Route path="/" component={App}>
    <IndexRoute getComponent={asyncAppViewResolver('Home')} />
    <Route path="about" getComponent={asyncAppViewResolver('About')} />
  </Route>
);

This is more concise, and strangely enough I no longer need to do any workarounds to get RHL working with this. I suspect the way that webpack wraps these kinds of statements plays nicer with RHL.

You can try it out from my repo: https://github.com/ctrlplusb/react-universally

calesce

calesce commented on Oct 2, 2016

@calesce
Collaborator

Thanks for the example @ctrlplusb, we should definitely put this in our docs.

Do you think the issue with static path System.import is just a Webpack 2 bug?

birkir

birkir commented on Oct 11, 2016

@birkir

This is definitely a bug.

TypeError: Cannot read property 'call' of undefined
    at __webpack_require__ (http://localhost:3000/vendor.js:686:29)
    at fn (http://localhost:3000/vendor.js:111:20)
    at http://localhost:3000/client.js:20574:10

Happening here:

modules[moduleId].call(module.exports, module, module.exports, hotCreateRequire(moduleId));
calesce

calesce commented on Oct 11, 2016

@calesce
Collaborator

@birkir: I'm not sure where that comes from without more context. What version of RHL/Webpack, and do you have a simple repo project? Also, it might not strictly fall under this issue, so please open a new issue if you think it's a problem with React Hot Loader.

birkir

birkir commented on Oct 11, 2016

@birkir

Oh yeah, sorry. ueno-llc/starter-kit-historical#16
I'm still looking into this, may have posted too soon.

I get this error on the first request to the server, once i hot reload (any code) everything starts to work as normally.

elodszopos

elodszopos commented on Oct 11, 2016

@elodszopos

Hey @ctrlplusb. Regarding your workaround. I've built out async route splitting / chunking using react router too with Webpack 2 and I got all my containers / routes loading on-demand with System.import.

While in dev mode, I get the following:


process-update.js:100 [HMR] Updated modules:
process-update.js:102 [HMR]  - ./UI/helpers/Paginate.jsx
process-update.js:102 [HMR]  - ./UI/pages/Users/Users.jsx
process-update.js:102 [HMR]  - ./UI/pages/Users/index.jsx
process-update.js:102 [HMR]  - ./UI/pages async ^\.\/.*\/index$
process-update.js:102 [HMR]  - ./UI/routes/index.jsx
process-update.js:102 [HMR]  - ./UI/containers/Root/index.jsx
process-update.js:107 [HMR] App is up to date.

All nice and fine, http://localhost:8080/15.c1b371b1060596a59e4e.hot-update.js also appears in the network tab as a request, containing

webpackHotUpdate(15,{

/***/ 303:
/***/ function(module, exports, __webpack_require__) {
...

as expected. Function gets called, module replacement happensss .... but then it doesn't. The UI never updates, the new component never gets rendered. I tried changing the new components (new meaning the one I'm trying to update) render to also log something on the console. Never gets called. No log, no render.

I will try your workaround now and see if that helps. So if that works.. I can officially confirm this one. Unfortunately.

Maybe I'm not on the right track, but I'm running out of ideas now.

elodszopos

elodszopos commented on Oct 11, 2016

@elodszopos

Update: using require instead of System.import does update the container with the changes after HMR did what it should. But only upon navigating elsewhere and then back to the container that you updated, so router loads it again for you (containing the changes). Same page updating still does not happen. Possibly manually module.hot.accept them?

That can get out hand real quick though. Even with the leaf and parent route abstraction that I did. You can see below.

function getLeafRoute(route) {
  return (nextState, cb) => {
    if (process.env.NODE_ENV === 'development') {
      return loadRoute(cb)(require(`../pages/${route}/index`));
    } else {
      return System.import(`../pages/${route}/index`).then(loadRoute(cb)).catch(errorLoading);
    }
  };
}

Update:
If I try to hot accept the modules, I get:

[HMR] unexpected require(116) from disposed module 26

elodszopos

elodszopos commented on Oct 11, 2016

@elodszopos

@ctrlplusb I looked at what I had and compared with yours, and it's basically the same.

function errorLoading(err) {
  if (process.env.NODE_ENV === 'production') {
    // force reload with force get the page and load new chunks
    window.location.reload(true);
  } else {
    console.error('Dynamic page loading failed', err);
  }
}

function loadRoute(cb) {
  return (module) => cb(null, module.default);
}

function getParentRoute(route) {
  return (nextState, cb) => System.import('../containers/' + route + '/index.jsx')
    .then(loadRoute(cb))
    .catch(errorLoading);
}

function getLeafRoute(route) {
  return (nextState, cb) => System.import('../pages/' + route + '/index.jsx')
    .then(loadRoute(cb))
    .catch(errorLoading);
}

Here's a group:

const loginGroup = moduleName => (nextState, cb) => {
  const groupItems = {
    login: System.import('../pages/Login'),
    resetPassword: System.import('../pages/ResetPassword'),
  };

  groupItems[moduleName].then(loadRoute(cb)).catch(errorLoading);
};

And some route definitions:

const appRoutes = {
  component: App,
  path: '/',
  childRoutes: [
    {
      getComponents: loggedOutLayout,
      childRoutes: [
        { path: '/login', getComponent: loginGroup('login') },
        { path: '/reset-password', getComponent: loginGroup('resetPassword') },
      ],
    },
...
    childRoutes: [
            { path: 'accountInfo', getComponent: getLeafRoute('AccountInfo') },
            { path: 'users', getComponent: getLeafRoute('Users') },
...

I cannot get it to work though. HMR does happen, I get the update, but the old component never updates. If anyone can provide some assistance it'd be most welcome.

Also, I can confirm that I isolated the problem and it is System.import. Non-asynchronously loaded routes work perfectly fine.

Thank you!

Jessidhia

Jessidhia commented on Oct 12, 2016

@Jessidhia

Just hit this, and I have the same symptoms/conclusion as @elodszopos. Components belonging to an async chunk will log, to the console, as if they have been updated, but their appearance in the UI doesn't change. Even if the component is something simple like only returning <div>foo</div>, changing it to <div>bar</div> will not update the render if the component is in an async chunk (as cut by System.import), but will if the component comes from a regular hoisted import.

This also happens if the component is imported through a hoisted import if the file that has the import / module.hot.accept is a file loaded through System.import; the rerender will misbehave.

As this is an accept for a sync import (inside an async chunk), code generated by webpack is still the same as usual:

import Component from './module/request'

module.hot.accept('./module/request', acceptCallback)

is compiled as (whitespace / newlines added, identifiers simplified for readability):

/* harmony import */ var __WEBPACK_IMPORTED_MODULE_4__module_request__ =
  __webpack_require__(/*! ./module/request */ "moduleId");

module.hot.accept(/*! ./module/request */ "moduleId",
  function(__WEBPACK_OUTDATED_DEPENDENCIES__) {
    /* harmony import */ __WEBPACK_IMPORTED_MODULE_4__module_request__ =
      __webpack_require__(/*! ./module/request */ "moduleId");
    (acceptCallback)(__WEBPACK_OUTDATED_DEPENDENCIES__);
  }
);

EDIT: to clarify, the acceptCallback is called; but nothing changes when ReactDOM.render runs.

Jessidhia

Jessidhia commented on Oct 12, 2016

@Jessidhia

Correction: hot reloading actually does work, but only in one of the copies of the component. Only last component to be rendered in the first pass gets updated.

Is webpack losing the module.hot.accept callbacks?

EDIT: looks like webpack only keeps the last callback. I changed my code to a method where I keep track of each callback, and execute all of them in the callback actually given to module.hot.accept.

birkir

birkir commented on Oct 14, 2016

@birkir

Just a little update.

I fixed the issue by invalidating the first build from webpack. So to sum up, hot reloading system.imported code would only work on the second time the webpack builds.

I don't know if I'm just using hot reloader the wrong way, but It's very similar to @ctrlplusb solution.

https://github.com/ueno-llc/starter-kit/blob/master/webpack/dev.js#L118

25 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @wmertens@Jessidhia@birkir@critic47@olslash

        Issue actions

          webpack2 + hot-reload: Not works with System.import · Issue #303 · gaearon/react-hot-loader