-
-
Notifications
You must be signed in to change notification settings - Fork 15.3k
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
Change examples to explicitly use replaceReducer() for hot reloading #667
Conversation
Change examples to explicitly use replaceReducer() for hot reloading
@SirCmpwn I'm experiencing the same issue as you. Can you share your solution? Many thanks. |
Are you using Babel 5 or 6? @eenewbsauce |
My configureStore looks something like this, @eenewbsauce:
|
@gaearon I am using Babel 6.3.26 Here is my configureStore.js: import React from 'react';
import { applyMiddleware, compose, combineReducers, createStore } from 'redux';
import thunk from 'redux-thunk';
import { syncHistory, routeReducer } from 'react-router-redux'
import { browserHistory } from 'react-router'
const historySyncMiddleware = syncHistory(browserHistory);
import createLogger from 'redux-logger'
import rootReducer from '../reducers'
import { DevTools } from '../components'
const createStoreWithMiddleware = compose(
applyMiddleware(thunk, historySyncMiddleware, createLogger()),
DevTools.instrument()
)(createStore);
export default function configureStore(initialState) {
const store = createStoreWithMiddleware(rootReducer, initialState);
if (module.hot) {
console.log('hit');
module.hot.accept('../reducers', () => {
console.log('hit inside');
store.replaceReducer(require('../reducers').default)
});
}
historySyncMiddleware.listenForReplays(store)
return store;
} and here is my reducers.index.js: import { combineReducers } from 'redux';
import { routeReducer } from 'react-router-redux'
import count from './count';
const rootReducer = combineReducers({
routing: routeReducer,
count
});
export default rootReducer; |
I recall now that I had to make some other change, but I can't find the relevant commit in my git log. Sorry, man. |
@eenewbsauce Please publish the project reproducing the issue in isolation. |
Thanks for your help @SirCmpwn. @gaearon Thank you very much for looking into this issue. Here is the repo: https://github.com/eenewbsauce/react-router-redux |
@eenewbsauce I can't reproduce the problem: Maybe you have some watcher issue like this. Verify whether Webpack rebuilds when you change files. |
Thank you very much @gaearon. I'll take a peak and let you know. |
@gaearon I was trying to hot reload the |
Interested how you set it up. |
@gaearon It seems that I was a bit misleading in my previous post. I didn't get HMR working with |
Ah, okay. This is expected. |
I followed the instructions and got the hot reloading to work with browserify and livereactload, but I'm still see this warning:
Is this expected or is there something wrong with my setup? |
I presume that livereactload, unlike Webpack, does not offer something like let store
if (window.__store) {
window.__store.replaceReducer(reducer)
} else {
window.__store = store = createStore(reducer)
} |
Here's what my configureStore looks like - very similar to the webpack examples, and it seems to work as I would expect, despite the warning:
Ah I see, I'm not replacing a specific module, but rather on every change because other than module.hot, onReload only takes one argument, right? |
Hm, according to this that code should not trigger the warning. I guess this is more of an issue in livereactload, so I'll open an issue there. |
I'm seeing the same warning as c089. The callback isn't being called, although hot-reloading does work... My createStore-code:
|
|
Ah sorry, should have mentioned that. In my webpack.config I have
I tried replacing |
Please publish a project reproducing the issue. Or you can start with our examples and work backwards. |
@gaearon I'm also getting the error configureStore.js import { createStore, applyMiddleware } from 'redux';
import thunkMiddleware from 'redux-thunk';
import RootReducer from './reducers';
export default () => {
const persistedState = undefined;
const store = createStore(
RootReducer,
persistedState,
applyMiddleware(
thunkMiddleware
)
);
if (module.hot) {
// Enable Webpack hot module replacement for reducers
module.hot.accept('./reducers', () => {
console.info('executing callback');
const nextRootReducer = require('./reducers').default;
store.replaceReducer(nextRootReducer);
});
}
return store;
}; reducers/index.js import { combineReducers } from 'redux';
import NotesReducer from './reducer_notes';
const rootReducer = combineReducers({
notes: NotesReducer
});
export default rootReducer; |
I'm running into the same problem like jcarenza. This error occures when I manipulate the elements which are written into the redux store. With other components the hot reload works fine. |
@jcarenza I'm having the exact same issue. Did you find a fix? |
@gaearon |
@giest4life |
@Ganitzsh @HorseBinky @jcarenza @giest4life For example, like in @jcarenza code - #667 (comment) import configureStore from './configureStore';
const store = configureStore();
ReactDOM.render((
<Provider store={store}>
<Router history={browserHistory}>
{Routes()}
</Router>
</Provider>
), document.getElementById('mount')); You need to move if (module.hot) module.hot.accept(); to the |
Thanks for that tip! |
@maullerz mind sharing an example, please ? I'm also getting "warning.js:14 does not support changing |
@giest4life @heldrida |
I end up doing the following in the app entry point:
the store configurator:
the root component:
|
@heldrida That worked perfectly for me. I was passing |
@heldrida That worked perfectly! |
This works like a charm:
import { combineReducers } from 'redux';
import reducerA from './reducerA';
import reducerB from './reducerB';
import reducerC from './reducerC';
const reducer = combineReducers({
reducerA,
reducerB,
reducerC,
});
export default reducer;
import React from 'react';
import ReactDOM from 'react-dom';
import { AppContainer } from 'react-hot-loader';
import { createStore, compose } from 'redux';
import { Provider } from 'react-redux';
import reducer from './app/reducers/index';
import App from './app/App.jsx';
const store = createStore(reducer, compose);
const render = Component => {
ReactDOM.render(
<AppContainer>
<Provider store={store}>
<Component />
</Provider>
</AppContainer>,
document.getElementById('root')
);
};
render(App);
if (module.hot) {
module.hot.accept('./app/App.jsx', () => { render(App); });
module.hot.accept('./app/reducers/index', () => { store.replaceReducer(reducer); });
} |
This changes the examples to call
replaceReducer()
directly where appropriate instead of relying on the current “magic” behavior of React Redux.On the surface, this contradicts #350, but the real intention behind #350 is to reduce the public API and remove “magic” from hot reloading because this magic often gets in the way (#301, #340).
This change lets us:
getReducer()
completely from the Store public API, which is a win;<Root>
component everywhere, and thus make examples simpler.We can decide later whether or not we want to move ahead with something like #625 or not—it doesn't matter because whatever solution we end up with, we'll definitely force you to use HMR API directly for hot reloading, like in these examples.
Because actually removing
getReducer()
is a breaking change, we will do it separately (potentially with other breaking changes).