Inject hot reload variables?

[peteruithoven]

Webpack works with module.hot variables in modules. See: https://webpack.github.io/docs/hot-module-replacement.html Having such variables would make some things seriously easier.

For Redux for example, the code in the following gist: https://gist.github.com/peteruithoven/59f188bfab035ce96948 Could turn into something like:

import {createStore} from 'redux';
import reducer from './reducers/index.js'
import { rehydrate, rehydratingStore } from './utils/rehydratingStore.js';
let initialState;
if(__hot) {
  initialState = __hot.deletedModule;
}
const store = rehydratingStore()(createStore)(reducer, initialState);

Would it be possible to "inject" such variables into hot reloaded modules?

[veggiemonk]

:+1: and this would not be special to redux. It is just one use case.

[peteruithoven]

@guybedford, do you perhaps have any tips? It would be very nice if this works in such a way it's compatible with how module loaders will natively work in the browser some day (With systemjs, but without the es6 module loader).

[guybedford]

Am I right in thinking the argument here is about injecting variables when the module is executed and not as a later function call?

I'm wondering how such a syntax could be mimicked in ES modules, perhaps something like -

import previousState from '@hotstate';

Or something similar might work instead of the __reload hook, where @hotstate uniquely normalizes for each module to say something with a unique Id - @hotstate:moduleName.js, and then gets removed from the registry after the module has loaded by the hot reloader.

So it would be possible, but it's not an elegant use of the module loader by any means, and I also don't like the idea that the previous state "sticks around" with its bindings from a memory perspective (as module imports are permanent bindings in the module scope. Say for example another function export that uses eval could then indirectly cause the state to never be garbage collected.

The nice thing about the __reload hook is that the binding representing the old state is cleanly only provided where it is used.

I know it doesn't look quite as aesthetically pleasing, but I'm really not sure the benefit is worth the cost to do something better here in a way that works well with modules.

[mikz]

I'm trying to use this with https://github.com/gaearon/redux-react and it complains when the state changes. https://github.com/rackt/react-redux/blob/3a96902e824fe2ebb2ae13df9c1fa7b52439d4ed/src/components/Provider.js#L11-L17

It would be really nice to have a way of injecting variables before the module execution. Because now, I don't see many options how to solve that. On load, the script has no idea whether it was hot loaded or not. And some need to behave differently if they were.

This Provider component has to take state from the previous module, otherwise it won't work. It could be done in the __reload hook, but then the module's execute would have to wait for this hook.

It would be enough, if the hook would be executed, before the module itself.

edit: I'd be ok with using the babel transforms to do that. Like https://github.com/tyscorp/react-transform-jspm-hmr/

[peteruithoven]

@mikz I also came across this problem and still have to look into this. Are you sure this would fix the issue? Isn't it that the provider react component persists (over hot reloads) (even though we recreate the code that created him) and changing it's store isn't supported?

It looks like if we can retain the same store instance and just rehydrate it this wouldn't be a problem. Then again that would be easier is we had this injecting option, and maybe that is what you meant?

Then again if the Provider component would have a method that would allow us to change the store, we could call that from the __reload and that issue would also be solved.

[mikz]

@peteruithoven React remembers somewhere in virtual dom, that Provider was already there. So when the component that renders Provider is reloaded, it tries to update existing Provider with new store.

https://github.com/mikz/jspm-react-hot-reload/blob/eea6a61598e1f18bb8aff5803eae51f051cfaba9/src/index.js#L11-L23

The key here, is that the store has to be exactly the same object, as it was in the previous module.

I done that by storing it in window. https://github.com/mikz/jspm-react-hot-reload/blob/eea6a61598e1f18bb8aff5803eae51f051cfaba9/src/store.js#L21-L33 It is ugly and not portable.

To follow the DevTools suggestion, we should retain the store, and call store.replaceReducer(reducer) with the new, reloaded reducer.

Other way, not requiring global variable would work, if modules would be aware of hot reloading.

export let store = configureStore(reducers)

function render() {
  ReactDOM.render((
    <Provider store={store}>
      <div>
        <HelloWorld />
        <Counter />
        <DevTools />
      </div>
    </Provider>
  ), document.getElementById('app'))
}

export function __reload(deletedModule) {
  store = deletedModule.store
  store.replaceReducer(reducers)
  render()
}

if (!__hot_) { render() }

Basically, deferring the execution, until __reload is called (and can extract state from the previous one).

Edit: In the end I store the variable in Window when module is about to be unloaded. https://github.com/mikz/jspm-react-hot-reload/blob/6204b2d407afb9857d665ebe625143dc2d36b507/src/index.js#L11-L20

[guybedford]

@mikz you could just wrap your renderer in a function:

import React from 'react'
import ReactDOM from 'react-dom'
import { Provider } from 'react-redux'
import configureStore from './store'

import DevTools from './containers/dev_tools'
import Counter from './containers/counter'
import HelloWorld from './containers/hello_world'
import reducers from './reducers/index';

export let store;

export function __reload(deletedModule) {
  store.replaceReducer(reducers)
}

export function render() {
  store = store || configureStore(reducers)

  ReactDOM.render((
    <Provider store={store}>
      <div>
        <HelloWorld />
        <Counter />
        <DevTools />
      </div>
    </Provider>
  ), document.getElementById('app'))
}

And then have a bootstrap.js:

import {render} from 'renderer';
render();

[mikz]

@guybedford unfortunately, systemjs-hot-loader calls __reload only on modules, that are imported via System.import. So the __reload would never be called and it could not extract the store.

Tried to do that as https://github.com/mikz/jspm-react-hot-reload/commit/3fc30e22a7ed2427563cfe0410ff689d926392dd but no dice.

[guybedford]

Ah, right. We should ensure that __reload and __unload are called on all modules in the tree I think for consistency?

[mikz]

@guybedford __unload is called on all modules, but __reload is not.

That would work IMO. It is nasty, that you have to export those variables, but guess the other way would be to have some API available, that would export them to some anonymous module and the newly loaded one would get that as a parameter.

But still, better this than nothing :)

[peteruithoven]

Relevant issue: https://github.com/rackt/react-redux/issues/223

[peteruithoven]

I'd prefer thinking along the lines of Guy's first idea. What if we could import the previous instance (instead of state)? Before the original module is reimported (https://github.com/capaj/systemjs-hot-reloader/blob/master/hot-reloader.js#L199) maybe we could add a new module that the original module can import, maybe @prevInstance orsystemjs-hot-reloader/prevInstance. This new module could then be the previous instance. We might be able to do this much like I did mocking in the following experiment: https://github.com/peteruithoven/mocking/blob/master/test/index.js#L8-L15 When the hot reloader cleans up it's reference to this previous instance there is less change of leaks. Also the fact that only the top / original module gets a change to do this limit's the amount of possible leaks.

[guybedford]

I do think the function wrapping approach in https://github.com/capaj/systemjs-hot-reloader/issues/34#issuecomment-174304336 is preferable, by supporting the __reload hook for all modules in the tree. It means a slight restructuring but then fully supports what you're trying to achieve.

[peteruithoven]

But, in most cases having the __reload in modules deeper isn't useful because there might be multiple instances of that module. I'm probably missing something but I don't understand why you would prefer that https://github.com/capaj/systemjs-hot-reloader/issues/34#issuecomment-174304336 approach

I've tried out my idea: https://github.com/capaj/systemjs-hot-reloader/pull/48

Simplified usage example:

import {getStore as getPrevStore} from 'capaj/systemjs-hot-reloader/prevInstance.js';

let store;

if(getPrevStore) {
  // use existing store
  store = getPrevStore();
  store.replaceReducer(reducer);
} else {
  // create new store
  store = createStore(reducer);
}

export function getStore() {
  return store;
}

Curious what you guys think.

[mikz]

Don't know. Somehow it feels dirtier than saving it to window on __unload like:

const store = window.__store || createStore(reducer);

if(store === window.__store)
  store.replaceReducer(reducer)

export function __unload() {
  window.__store = store
}

I understand that might cause some issues when the module is loaded several times in several contexts. So it would need some unique id persisted through the reloads. But maybe it is just because it is in wrong module and it should be in one that can't be used several times.

__reload feels a bit hacky to me, because it has to export the variables through to the "public". But the prevInstance does not fix that. Imho the hot module reloading should not drive design of your modules or force you to export variables that are not needed. Both __reload and prefInstance does that.

What I like about the module.hot (even though I haven't used it) is that you can hook into the process of reloading. You can react when dependencies change. Mimicking the same API is probably hard because they use the nodejs style require.

I guess the difference here is that webpack does not reimport the whole tree on single change? And every component can opt in into the reload process? I'm not saying it is right or desired with SystemJS.

Maybe, we should think about what is ideal for SystemJS ecosystem and for general development guidelines.

Here is my list:

1. hot module reload should not force to change design of your code (export functions, add more api)
2. need to persist some state across module reload (considering there can be several instances with different state)
3. offer a way to determine if the module is being hot loaded or not
4. easily use the code in production, so the hot reloading hacks never make it there

I think the second is driven by hot reloading replacing all the tree. I'm probably ok with that as I have not found any drawbacks. But I can imagine code where it could cause some pain, like double initializing some rendering etc. I understand it should be turned off in __unload but that might not always be possible. That's why point 3.

Cheers!

[peteruithoven]

@mikz, I have to agree, especially on point 1. I totally see the downside of that in my approach.

What if we use the fact that, an imported module, that isn't changed isn't reloaded and can retain state? We could create a module just for storing state over hot reloads. This would allow storing state, without introducing new api and without storing state in window. If we want to support multiple modules to be hot reload we could use the same same mechanic as the debug package to create specific names. Better would be if each module could store it's state under it's own normalized and unique name, automatically, but I don't know how we could do that.

import getReloadStore from 'hot-reload-store';
const hotStore = getReloadStore('myproject:index'); 

let store;
if(hotStore.store) {
  // use existing store
  store = hotStore.store;
  store.replaceReducer(reducer);
} else {
  // create new store
  store = createStore(reducer);
  hotStore.store = store;
}

[mikz]

@peteruithoven will update my post with another point. The code should be easily shipped to production. The whole module.hot thing is usually guarded by process.env check. Not sure if import could be guarded with condition.

[peteruithoven]

I've made a simple example demonstrating my last idea; https://github.com/peteruithoven/hot-reload-store-example One of the few downsides that I can see is possible name conflicts.

[peteruithoven]

For a more crude way to force reload React components, like the Provider or the Router: https://github.com/capaj/systemjs-hot-reloader/issues/51

[grady-lad]

@peteruithoven by using https://github.com/capaj/systemjs-hot-reloader/issues/51 does this mean that the hot reloader will reload when we make changes to our redux reducers? Having problems hot reloading when making changes to the redux reducers at the moment.

[peteruithoven]

I would only use #51 (unmountComponentAtNode) when there is no other option, for example when using the React-router. (and even then there is probably a better solution out there). With #51 or other solution you will still need to rehydrate / restore you're state after a hot reload, you could use the method I proposed in https://github.com/capaj/systemjs-hot-reloader/issues/34#issuecomment-174723848. When you do use #51 you can also use a hot reload store to restore the initialState, like:

import { createStore } from 'redux';
import getHotReloadStore from './utils/getHotReloadStore.js';
const hotStore = getHotReloadStore('myproject:store');

export default function configureStore(reducer) {
  let initialState;
  if (hotStore.store) initialState = hotStore.store.getState();
  const store = createStore(reducer, initialState);
  hotStore.store = store;
  return store;
}

[alexisvincent]

Tracking here -> https://github.com/alexisvincent/systemjs-hmr/issues/2