Rehydrate should do a deep merge only on existing keys

[JohnKis]

On 6.0.0 the rehydrated state is incomplete when the store schema changes. There is a PR to fix this (#114) however there is a scenario it doesn't cater for, and it was suggested that this scenario should be documented in a new issue.

Scenario

If a key has been removed from the store's initialState but still included in the saved state in local storage, the rehydrated state will include the deleted key. This is not ideal as it makes it impossible to remove keys from your store schema. This also applies to nested keys.

Example

Store initial state

{
    "key1": {
        "nestedKey1": "value",
        "nestedKey2": "another value",
    },
    "key2": false
}

Saved state in local storage

{
    "key1": {
        "nestedKey1": "update value",
        "nestedKey2": "another value",
        "nestedKey3": "deleted value"
    },
    "key3": true
}

Expected rehydrated state

{
    "key1": {
        "nestedKey1": "updated value",
        "nestedKey2": "another value",
    },
    "key2": false
}

Actual rehydrated state (on #114)

{
    "key1": {
        "nestedKey1": "updated value",
        "nestedKey2": "another value",
        "nestedKey3": "deleted value"
    },
    "key2": false,
    "key3": true
}

If this issue is fixed, it would also be ideal to remove to redundant keys from local storage as well. However this might be complicated especially on nested keys.

[moniuch]

@JohnKis Do you perhaps have your config (LocalStorageConfig) at hand?

[JohnKis]

@moniuch Sure, please find it below.

export function localStorageSyncReducer(reducer: ActionReducer<any>): ActionReducer<any> {
  return localStorageSync({
    keys: ['key1', 'key2', 'key3'],
    rehydrate: true,
    restoreDates: true
  })(reducer);
}

[moniuch]

Thank you so much!

@bufke David, what would you say if we provided an issue template with instructions for the users who want to submit a bug ticket, asking them to provide their config and actual/expected state? Just the way @JohnKis did ^^.

[moniuch]

This case is a bit disputable to me. IMO, with this config, we should not expect that nestedKey3 should get deleted.

While rehydrating, we are only told that we should pick and apply ['key1', 'key2', 'key3'] - that implicitly means to me: "...whatever they contain". That's because, even though initialState only contains

"key1": {
  "nestedKey1": "value",
  "nestedKey2": "another value",
},

doesn't mean that nestedKey3 would be illegal. After all your state at key1 might be defined as:

key1: {
  nestedKey1: any;
  nestedKey2: any;
  nestedKey3?: any;
},

What would make nestedKey3 illegal and removed though, would be the following config:

keys: [{'key1': ['nestedKey1', 'nestedKey2']}],

which explicitly says that nestedKey3 is unwanted.

Not sure though, this mixed config for keys is supported:

keys: [{'key1': ['nestedKey1', 'nestedKey2']}, 'key2', 'key3'],

)

So maybe we should support wildcards, as in

keys: [{'key1': ['nestedKey1', 'nestedKey2']}, {'key2': '*'}, {'key3': '*'}],

to enable this sort of cherry-pick on some, and en gross selection on other keys.

What are your thoughts? @btroncone @bufke

[JohnKis]

@moniuch You're right, however this behaviour restores keys that you've removed from your state intentionally. I guess there are cases where it'd be valid not to remove non-existent keys (e.g.: where the initial state does not contain the full structure) so maybe it could be an option instead of the default behaviour.

[moniuch]

What I wanted to say is that there is no way AFAICT to tell, while rehydrating the state from local storage, which keys may are missing by intention and which by not being populated yet. To my understanding, the only way of conveying the intention is config.keys. But here I will surrender to the knowledge of others which have been much longer in this project than myself. Might be that my understanding is wrong.

[moniuch]

We could now also look into #100 which introduces an option to provide a custom merging function with the config.