1.0.0-rc - mergePersistedState calls default merge incorrectly

[chrishoage]

Using 1.0.0-rc with mergePersistedState to deserialize to Immutable calls the default merge function first pass which "mutates" (does a copy, but this mangles the Immutable object) and THEN calls the user defined merge function which is then useless.

Expected: Only the user defined merge function gets called when specified.

Using this log in mergePersistedState.js where mergeState is the default merge function.

console.log('mergePersistedState', state, action.type, action.payload, merge === mergeState)

mergePersistedState Object {entities: Record, jobs: Object, library: Object, copyKeys: Object, dest: Record} redux-localstorage/INIT Object {entities: Object, jobs: Array[0], library: Object, copyKeys: Array[0], dest: Object} true
mergePersistedState Object {entities: Object, jobs: Object, library: Object, copyKeys: Object, dest: Object} redux-localstorage/INIT Object {entities: Object, jobs: Array[0], library: Object, copyKeys: Array[0], dest: Object} false

Note how entities in state goes from a Record to an Object.

[elgerlambert]

Hi @chrishoage, yeah I can reproduce the issue and you're right; only the user defined merge function should be called (but that's currently not the case).

I'm pretty sure the changes in this commit are the culprit. As a result of these changes the check on https://github.com/elgerlambert/redux-localstorage/blob/1.0-breaking-changes/src/persistState.js#L28 appears to always return true.

I'll try and look into it tomorrow, would welcome a PR, might turn out to be a tricky issue to solve though.

[chrishoage]

Thanks for the hint @elgerlambert I'll poke around and see what I can find. If I can figure out what is going on I'll submit a pull request.

[chrishoage]

So after looking at it again, I noticed you call mergePersistedState on line 29 with the reducer which would cause exactly the issue I am experiencing. As a test I removed the entire finalReducer line and passed reducer directly to next. This made everything work.

So I am not entirely sure the purpose of the finalReducer check but for my particular use case it did not seem to do anything.

[elgerlambert]

It's there to apply the default merge strategy. The check on the line above is suppose to detect whether you're handling the redux-localstorage/INIT event yourself (which -in your case- you are) and if not apply the default merge/rehydration strategy and otherwise do nothing. (but this appears to be broken)

The idea is that in the typical case you don't have to define/apply a reducer to rehydrate your store with the persisted data. An exception is when you have to merge values into an immutable collection.

Out of curiosity, what does Object.prototype.toString.call( state.entities ) return as value? In other words, what is the toString value of a Record..?

[chrishoage]

An instantiated Immutable Record returns "[object Object]" when called as the context of that toString method.

As for your explanation that makes sense. Not entirely sure why that check is failing though.

[elgerlambert]

While looking into this issue I realised there were a number of edge cases that would lead to unexpected behaviour. Auto applying the default merge strategy also felt too opinionated since it got in the way of rehydrating the store on a per reducer basis.

I've therefore decided to remove this behaviour and require that mergePeristedState be applied explicitly (https://github.com/elgerlambert/redux-localstorage/commit/600968e5dd4523215957596f9fff81eddb912b07). The Readme has been updated to reflect these changes and provides some additional info on rehydration.

These changes have been published as redux-localstorage@1.0.0-rc3.