Skip to content

Commit

Permalink
Improve rehydration logic, merge state instead of replacing it.
Browse files Browse the repository at this point in the history
  • Loading branch information
zewish committed Nov 16, 2023
1 parent 991ceeb commit 3d9b0ee
Showing 1 changed file with 10 additions and 7 deletions.
17 changes: 10 additions & 7 deletions src/rehydrate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,19 +64,22 @@ export const rehydrate = async (
unserialize
}: RehydrateOptions
) => {
let state = {};
let state = store.getState();

try {
const load = persistWholeStore
? loadAll
: loadAllKeyed;

state = await load({
rememberedKeys,
driver,
prefix,
unserialize
});
state = {
...state,
...await load({
rememberedKeys,
driver,
prefix,
unserialize
})
};
} catch (err) {
console.warn(
'redux-remember: rehydrate error',
Expand Down

4 comments on commit 3d9b0ee

@psychedelicious
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this feels like a substantial change in behaviour. Would you mind elaborating on the reason for the change?

@zewish
Copy link
Owner Author

@zewish zewish commented on 3d9b0ee Nov 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @psychedelicious, there was already a state merge happening at the rememberReducer() level. So we basically did the same thing previously on a different level, but the state might become out of date by the time we rehydrated inside the enhancer (e.g. in case Next.js SSR rehydration fired before ours).

This change makes sure we always have updated state before rehydrating over it.
I've tested this extensively and made sure none of the existing functionality is broken.
Would this by any chance break anything on your side?

@psychedelicious
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying. I think I need to read through the code again to fully grok the implications, but I agree with the change insofar as it makes the behavior consistent.

I don't think it would break anything. I'll test before we update and create an issue if needed.

Thanks again for your continued work on this library!

@psychedelicious
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wanted to follow up on this - I haven't noticed any issues or changes in behavior 👍

Please sign in to comment.