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

fix rehydrating of feature module state #76

Merged
merged 1 commit into from
Feb 15, 2018

Conversation

nhaesler
Copy link
Contributor

Currently rehydrating of feature module state doesn't work if you have keys defined in the root module. With every added module, the rehydration function gets called with the "@ngrx/store/update-reducers" action and syncs the initial state to the localStorage.

This change fixes it for me. On a quick glance it seems to fix the following issues: #75, #72, #64, #44

@bufke
Copy link
Collaborator

bufke commented Jan 31, 2018

Do you think this is a fix that could have a unit test that fails without and passes with? I'm not a maintainer here but that would decrease the likelihood of this ever happening again.

@nhaesler
Copy link
Contributor Author

I thought about it when submitting the PR, but it would probably be part of a unit test for the library as a whole, whereas now only parts of the library are tested individually. I'm not too familiar with the code and it was more of a quick fix because the whole library is more or less useless without it (if you use feature modules).
But it seems like @btroncone is busy otherwise at the moment so I don't know how alive the project actually is.

@tanyagray
Copy link
Collaborator

Can confirm this PR fixes rehydration of feature modules. Would love to get this merged + npm updated cos separately building @nhaesler's branch direct from GitHub is a PITA 😂

Is the lack of tests the only reason it hasn't been merged?

@tanyagray tanyagray merged commit 1073e81 into btroncone:master Feb 15, 2018
@akrueger
Copy link

Sweet! When would the NPM package be able to be published with this fix?

Thanks

@tanyagray
Copy link
Collaborator

tanyagray commented Feb 17, 2018 via email

@btroncone
Copy link
Owner

@tanyagray I can publish tonight if you would like, just let me know!

@BBlackwo BBlackwo mentioned this pull request Apr 17, 2020
@BBlackwo BBlackwo added released and removed released labels Sep 5, 2020
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

Successfully merging this pull request may close these issues.

6 participants