Can't add new keys to persisted stores - feature request for different defaults handing

[tslocke]

Describe the bug

  const [,setStore] = makePersisted(createStore({a: 1}), {name: 'x'})
  setStore({a: 2}) // trigger storage (because initial values are not persisted)
  const [store2, setStore2] = makePersisted(createStore({a: 1, b: 1}), {name: 'x'})
  console.log(Object.keys(store2)) // ['a']

I would expect ['a', 'b']

While I'm here: these stores are so useful and convenient! Thanks for this.

Minimal Reproduction Link

https://stackblitz.com/edit/github-cickt7r1?file=src%2Findex.tsx

[atk]

The initial value of store2 is being immediately overwritten by the value from localStorage that is saved by the first persisted store with the name x. You can add new keys all right - you just cannot rely on them being there when initialized from a storage that does contain a version without them.

[tslocke]

This seems to make it difficult to develop the component later. If I add new store properties I can't rely on then being initialised. Why not merge the stored object with the initial one, if both are present?

[atk]

That's an interesting and valid argument and I will think about how to handle the issue. In the meantime, there's a workaround:

Additional defaults would need to be added after persistence,. Alternatively, you could also split your state.

[tslocke]

Did you come to any decision about this @atk ?

[atk]

Since the initialization is handled by createSignal/Store, there is nothing I can do about it. However, I can add a defaults value to the options that performs the same workaround in a more convenient manner; the rest would be a question of documentation.

[tslocke]

The change I was proposing would be inside makePersisted. The function has

  const init = storage.getItem(name, storageOptions);

to load the persisted data, which is then written to the store using your local set function, overwriting any default values.

I'm suggesting you instead do something like:

  const loaded = storage.getItem(name, storageOptions);
  const init = typeof signal[0] == 'function' ? {...signal[0](), ...loaded} : {...signal[0], ...loaded}

Another option would be not to use reconcile inside set, which means you would pick up the normal object merging behaviour of stores. Do I understand correctly that your persisted stores lose this behaviour?

Maybe the simple fix is just to make the reconcile behaviour optional?

[tslocke]

Oops posted without testing - that's all wrong! 🙈 Working on it...

[tslocke]

OK the change is a bit more convoluted than I thought because the merge needs to happen after deserialisation, so inside set, and set is also used to support the sync option. Not a big deal but not a one-liner.

For now I'm using a forked version that doesn't reconcile. What do you think about making that optional? What am I missing out on without that?

I don't want to work around this in my application code because that would mean unchanged initial values would be persisted which I would rather avoid.

[Azq2]

This package is missing two important features:

1. Merging with initial state
2. Migration

import { reconcile, SetStoreFunction, Store, unwrap } from "solid-js/store";

interface PersistentOptions<T> {
    name: string;
    storage?: Storage;
    version?: number;
    serialize?: (data: any) => string | undefined | null;
    deserialize?: (data: any) => string | undefined | null;
    migrate?: (persistedState: T, version: number) => T;
    merge?: (persistedState: T, currentState: T) => T;
}

interface PersistentInternal {
    version: number;
    state: any;
}

type PersistentStore<T> = [get: Store<T>, set: SetStoreFunction<T>];

export function makePersisted<T>(
    store: [Store<T>, SetStoreFunction<T>],
    options: PersistentOptions<T>
): PersistentStore<T> {
    const {
        name,
        storage = localStorage,
        version = 0,
        migrate = (persistedState) => persistedState,
        merge = (persistedState, currentState) => ({ ...currentState, ...persistedState }),
        serialize = JSON.stringify,
        deserialize = JSON.parse
    } = options;

    const storedValue = storage.getItem(name);
    if (storedValue !== null) {
        try {
            const parsed = deserialize(storedValue) as PersistentInternal;
            if (parsed != null && parsed.version !== null) {
                if (parsed.version !== version) {
                    parsed.state = migrate(parsed.state, parsed.version);
                    parsed.version = version;
                }
                parsed.state = merge(parsed.state, unwrap(store[0]));
                store[1](reconcile(parsed.state));
            }
        } catch (e) {
            console.warn("[makePersisted]", e);
        }
    }

    const updateStorage = () => {
        try {
            const serialized = serialize({ version, state: store[0] } as PersistentInternal);
            if (serialized != null) {
                storage.setItem(name, serialized);
            } else {
                storage.removeItem(name);
            }
        } catch (e) {
            console.warn("[makePersisted]", e);
        }
    };

    const setWithPersistence: SetStoreFunction<T> = (...args: any[]) => {
        (store[1] as any)(...args);
        updateStorage();
    };

    return [store[0], setWithPersistence];
}

[Azq2]

Or use alternative like zustand-solid