-
Notifications
You must be signed in to change notification settings - Fork 25
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
Enable initial migrations, clarify README #14
Conversation
Previously a `0` would be interpreted as falsy, but I believe the real intent here is to avoid an undefined due to an empty `versionKeys` array.
Previously, if there was not already a stored version, migrations would not happen at all. This change allows _all_ migrations to occur, the first time redux-persist-migrate is deployed and users have stored state, but no stored version. I think this is safe, because the only other realistic time the version will be null is when persisted data has been purged, in which case there will be no other `state` to migrate. The only other situation I can think of which might cause problems, is if the developer has not created a reducer at all in which to store the version. In that case, all migrations will be performed, but the new version will not be saved, so the migrations will occur each time. I think the solution to this is to improve the documentation to clarify that a reducer needs to be created to store the version, and that `redux-persist-migrate` will not create any reducers.
This is a great. Just spend a while fighting the Would love it merged if possible. |
Only other thought is that I'm using Probably obvious that it needs to be persisted - but wasn't in my case not when I was starting out! |
@rt2zz do you have any comments or changes for this PR? |
@rt2zz, if this is too many changes to review at once, I can try breaking it into multiple PRs, if that's easier. |
@IanVS apologize this took so long to get around to. Looks great! I have been working on redux-persist v5 which includes a concept of migrations built it (basically this lib). It turns out the versioning bug survived all the way through! I just fixed it on v5 now. The main difference is now we enforce the keeping of persist state (version and rehydrated), so creating the migrations is simpler (no need to define setter / getters). |
I had a bit of trouble getting started with this library, and this PR attempts to make it a bit easier for others. It does a few things:
0
. Previously,0
was being turned into-1
. I don't think this was intentional.redux-persist-migrate
. This is a fairly substantial change, and I put some notes in the commit message. I'm happy to discuss the pros/cons further.version
in.