alexisvincent / systemjs-hot-reloader

reloads your modules as needed so that you can have satisfyingly fast feedback loop when developing your app
MIT License
228 stars 36 forks source link

__reload isn't called, page refreshes #76

Closed kyeotic closed 8 years ago

kyeotic commented 8 years ago

I had hot reloading working at one point, but its not working anymore. The __reload function is never called (I even put a debugger line in and a console.log), instead the entire page seems to refresh, losing state and resetting the chrome dev tools.

My problem is that I don't really know how to troubleshoot this. I don't know what's causing the page refresh; I have a theory that because __reload isn't getting called that the hot reloader is deciding to reload everything without notifying anyone. I am not sure how to verify this. The page refresh only happens every other time. It's like the first event is ignored, and the second event refreshes everything (and resets this save count). Not sure what is causing this.

The relevant bit of my main.js file is

//Export the store so that the __reload hook can see it
export const store = configureStore(initialState, appReducer)

//Hot reload hook
export function __reload(oldModule) {
    debugger
    if (oldModule.store.dispatch) {
        store.dispatch({
            type: HYDRATE_STATE,
            state: oldModule.store.getState()
        })
    }
}

Chokidar is emitting events, according to the console output. The websocket shows the change frame. image

This occurs even when a page refresh occurs, but then a new websocket connection is established. The __reload function still isn't called, in either case.

Is there anywhere else I can check for configuration problems? Are there any situations that would cause __reload not to be called?

peteruithoven commented 8 years ago

The first check is a simple one, you probably already checked, check if the webserver you're using doesn't refresh the page, like browser-sync can for example.

The existence of a __reload function doesn't influence whether to refresh or not.

I'm pretty sure __reload is only called on the root module, your main.js is the root module?

This is the relevant peice of code, I would add console.log's there, see what's going on: https://github.com/capaj/systemjs-hot-reloader/blob/master/hot-reloader.js#L221-L226

There are some cases where document.location.reload() is called, I would try to figure out which one is doing that in your case and why.

kyeotic commented 8 years ago

I'm just using the npm http-server, which doesn't do any kind of reload automatically. main.js is my root module.

kyeotic commented 8 years ago

Looks like locaotion.reload() is only called for a special socket event , index and config changes, and exports being missing

kyeotic commented 8 years ago

Those breakpoints really helped, I've discovered the issue, but I don't really understand how its happening. It is either a problem with SystemJS, my code, or the hot reloader api is our of sync with jspm. But moduleToDelete.exports is undefined, which causes a reload.

image

I am relatively confident this is an api-out-of-date issue, since the exports object can be found at moduleToDelete.metadata.entry.module.exports

image

kyeotic commented 8 years ago

__reload isn't enough to capture the reload event, it really does have to be the root module. main.js had a circular import with another module that I was able to remove, and that fixed it. Problem solved =)

peteruithoven commented 8 years ago

@tyrsius I can also recommend perserving/restoring state with something like the systemjs-hot-reloader-store. This approach also works in non-root modules and it doesn't necessary require a special action + reducer.

kyeotic commented 8 years ago

Preserving state with redux is pretty trivial, actually. The code I posted in the original message on this thread combines with this store enhancer to move state across reloads

export default function configureStore(initialState, reducer) {
  return createStore(
    makeHydratable(reducer, HYDRATE_STATE),
    initialState,
    compose(applyMiddleware(thunkMiddleware))
  )
}

function makeHydratable(reducer, hydrateActionType) {
  return function (state, action) {
    switch (action.type) {
    case hydrateActionType:
      return reducer(action.state, action);
    default:
      return reducer(state, action);
    } 
  }
}
peteruithoven commented 8 years ago

True, but this is all the code that's needed when using something like the systemjs-hot-reloader-store, so not __reload and this code can be placed in a seperate module.

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

let prevState;
if (hotStore.prevStore) {
  prevState = hotStore.prevStore.getState(); 
}

export default function configureStore(reducers, initialState = prevState) {
  const store = createStore(reducers, initialState); 
  hotStore.prevStore = store;
  return store;
}

It should even be possible to create a reausable createHotReloadableStore() module/package that abstracts this logic. Something like:

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

let prevState;
if (hotStore.prevStore) {
  prevState = hotStore.prevStore.getState(); 
}

export default function createHotReloadableStore(...args) {
  args[1] = prevState || args[1];
  const store = createStore.apply(this, args); 
  hotStore.prevStore = store;
  return store;
}
kyeotic commented 8 years ago

I am very confused on how that even works. Your store is an object inside the module. How does the reload not cause that object to get recreated as empty before being passed to the newly-hot-loaded store module?

In any case, I actually prefer the __reload function. It's dead code, so when the app is built for production Rollup discards it. The Hydrade state is reusable for server side rendering, so its useful on its own.

peteruithoven commented 8 years ago

@tyrsius Because the hot-reloader-store didn't change it doesn't have to be removed and reimported, which means it's not changed, it's just readded to the newly-hot-loaded store module. It's what capaj explains here: https://github.com/capaj/systemjs-hot-reloader#preserving-state

Those are good points.

kyeotic commented 8 years ago

Ok, yeah that makes sense. I had some cognitive dissonance about what was getting reloaded; I knew only the changed modules got hot loaded, but for some reason when I thought about the store I assumed it was always reloaded.