-
Notifications
You must be signed in to change notification settings - Fork 866
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
v3 changelog & RFC #72
Comments
Big +1 on migrations, as I mentioned earlier. I'm about to do a backwards-incompatible restructuring of my app state and it's pretty painful as things are right now. |
Ok this is what I am thinking for migrations: It can be done via a transform. It might look something like this: const migrationManifest = {
0.1: {
reducerA: (state) => {...state, somethingStale: undefined}
},
0.2: {
// ...
}
}
let migrator = createMigration(migrationManifest)
persistStore(store, { preloaders: [migrator.preloader], transforms: [migrator.transform] }) So basically the manifest is an object with state "versions" as keys and objects which map per-reducer state as values. Notes:
Another issue is how to store the state version. I am thinking migrator can store it in a separate storage key These are just early thoughts. I am thinking of releasing this as a contrib module first and if it goes well we can look at direct integration. cc/ @corbt |
initial implementation: https://github.com/wildlifela/redux-persist-migration |
I don't think semver would help with migration versions. Monotonically increasing integers would actually work just fine, the only important thing is that there's a stable total ordering. I may not be totally understanding the implementation, but does each reducer inside a migration only get access to its own state? How do you deal with combining/splitting two reducers? For example, I just migrated data in my app from What if you tweaked the API to just look like: const migrationManifest = {
0.1: (state) => ({...state, staleReducer: undefined} ),
0.2: // ...
} I think it's ok if the implementation is slightly less performant, the migration code should only be called once per device in normal cases. Also good to know that the migrations may be called multiple times, but I'm curious why that's the case? Might be worth using multiSet in the AsyncStorage implementation which, while probably not totally transactional, makes it likely that the data will stay in sync. |
ah, so the current implementation is basically a minimal implementation using current functionality (well aside from the small addition of preloaders). I definitely like the whole-state approach you outline above, but it may require some larger api additions. Regarding running multiple times, the issue boils down to, how do we know the version of the stored state? If storage fails for some reason, the store will end up with initialState and no known version. The migrator will then apply all of it's migrations on the next rehydrate since the version is null. There may very well be a more elegant solution to this, but I have not thought of it yet :) Anyway, I will keep thinking it over. Let me know if you have any ideas for exactly what the api should look like. |
Migrations, nice one! I like the API. My approach until now was to have a schema version as the top-level state key, and have my app deal with noticing existing data which needs to be migrated. I tried replacing this in my project today 👍 When you have state and no known version number, that is certainly a challenge... The current behaviour is running all migrations, even against initialState... This is good for existing users, but seems it will break unless initialState remains in a pre-migrated state. Since I want initialState to be the latest state shape, I guess I want it to just set the latest version number and move on if there is no persisted state. I'll try that for myself now and see how it goes. |
👍 let us know how that goes. Getting the version number is the biggest challenge in the api design, perhaps keeping it in redux state is the way to go. |
hmm... well, here's my humble preloader: function setInitialStateMigrationVersionPreloader({ storage }) {
return new Promise((resolve, reject) => {
storage.getItem(storageKey).then((persistedState) => {
if (persistedState === null) {
storage.setItem(versionKey, latestVersionKey).then(resolve).catch(reject);
} else {
resolve();
}
}).catch(reject);
});
} New users with no persisted state and no migration version: No migrations run Existing users with persisted state and no migration version: All migrations run (no migrations run for new users because I'm ensuring they don't end up having persisted state and no migration version) So end behaviour seems to be ok, but I wonder how we could get this into the lib. I wonder how it would work if we checked migration version on rehydrate transform, and set migration version when state is persisted. Maybe having migration version in redux state would allow something like that. |
That does solve the re-running migrations issue, but the issue I see is as follows:
There are two ways we can get to the second scenario:
for 1) I would say since there was no pre-existing implementation it should be out of scope for migrations to handle this. If needed a developer can write an special custom migration in their reducer. |
after struggling with the api a bit, I am pretty convinced migration functionality is better of living inside of a store enhancer, similar to autoRehydrate. Here is a untested poc implementation of a createMigration store enhancer: Pros:
Cons:
As for the cons, 1 & 2 I am ok with, and 3 & 4 can be solved later if need be by adding a simple integration point in persistStore. Overall I am pretty happy with this approach and will publish a release after doing some testing. |
released 👍 📦 🎉 |
Breaking changes
autoRehydrate
no longer implement an action buffer. It was too automagical and caused some bugs with dispatch return values. Updgrade path: implement redux-action-bufferOther changes of note
createPersistor
method that handles subscribing to the store and persisting to the storage engine.persistStore
now simply callsgetStoredState
andcreatePersistor
getStoredState
now support promiseskeys
->getAllKeys
createTransform(inbound, outbound, {whitelist, blacklist})
methodtodo
The text was updated successfully, but these errors were encountered: