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

Client-side store memoization not working with hot module reloading #25

Closed
timotgl opened this issue May 22, 2017 · 9 comments
Closed

Comments

@timotgl
Copy link

timotgl commented May 22, 2017

When navigating to a different page in our app we see the following warning:
bildschirmfoto 2017-05-22 um 15 20 19

We believe that this is caused by using both withRedux and connect. The store is memoized in the module scope here but then HMR reloads the module, which creates another store instance:

[HMR] Updated modules:
[HMR]  - ./~/next-redux-wrapper/src/index.js
...

We initially thought that this might be related to using a _document.js template with next.js, especially after seeing this comment, but removing _document.js didn't solve our issue.

@kirill-konshin
Copy link
Owner

Good catch! Thanks. I will fix it in the next few days.

@neogermi
Copy link

I just stumbled over this. Is there any workaround for this that we could apply fast?

@timotgl
Copy link
Author

timotgl commented May 30, 2017

@neogermi as a quick and dirty hack you can fork this library and attach (and restore) the store instance to the window object in the memoization function.

@kirill-konshin
Copy link
Owner

kirill-konshin commented May 30, 2017

I suspect that window hack will be the default behavior, I suggest to name the property like __NEXT_REDUX_STORE__ or something similar... Don't hesitate to send a PR :)

The lib itself should not be hot-reloaded, because it does not change so referentially it should always be one object. Anyway, window fix should be just fine.

I have pushed a quick fix for you guys, you can install master branch and use it right away.

kirill-konshin added a commit that referenced this issue May 30, 2017
kirill-konshin added a commit that referenced this issue May 30, 2017
@kirill-konshin
Copy link
Owner

Fixed in 1.1.3

@neogermi
Copy link

Great thank you! Currently, 1.1.3 is not available through npm. When do you plan to publish it?

@kirill-konshin
Copy link
Owner

Oops, something went wrong with NPM publish after Travis CI, I will fix that tomorrow first thing in the morning.

@kirill-konshin
Copy link
Owner

Published.

@neogermi
Copy link

great thank you for the fast response!

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

No branches or pull requests

3 participants