useStorage incorrectly handles reactive default values

[valerio-farriciello]

Describe the bug

There's an issue with useStorage when passing a reactive value as the default value. Currently, the composable reuses the reactive defaults reference for storing the data. This means that when the local storage value changes, it also updates the original defaults reactive reference, which is not the expected behaviour.

Here is where the issue seems to be:

const data = (shallow ? shallowRef : ref)(typeof defaults === 'function' ? defaults() : defaults) as RemovableRef<T>

Reproduction

https://stackblitz.com/edit/vitejs-vite-efjb7e?file=src%2FApp.vue

System Info

N/A - Bug can be replicated in any environment

Used Package Manager

pnpm

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Make sure this is a VueUse issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to https://github.com/vuejs/core instead.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion.
• The provided reproduction is a minimal reproducible example of the bug.

[ilyaliao]

This appears to be the expected behavior - there's a two-way binding between the storage value and initial value. When initial value changes, the storage updates.

Correct me if I'm wrong.

[gkubisa]

The current behavior is inconsistent and unexpected, for example:

• if defaultValue = 'test', changing the stored value does not affect defaultValue (as expected)
• if defaultValue = () => 'test', changing the stored value does not affect defaultValue (as expected)
• if defaultValue = shallowRef('test'), changing the stored value affects defaultValue (unexpected change of the default value)
• if defaultValue = computed(() => 'test'), changing the stored value does not affect defaultValue but results in a warning (unexpected console warning "Write operation failed: computed value is readonly")

Changing the value in the storage should never affect the default value, regardless of how it is provided. The code below should result in consistent behavior, regardless of how the default value is provided.

const data = (shallow ? shallowRef : ref)(toValue(defaults)) as RemovableRef<T>

[ilyaliao]

@gkubisa Thanks for the summary.

It sounds like the expected behavior should be:

• When defaultValue changes, the stored value should update accordingly
• When the stored value changes, it should not modify the defaultValue

Is this understanding correct?

[gkubisa]

When the stored value changes, it should not modify the defaultValue

Yes, definitely. 👍

When defaultValue changes, the stored value should update accordingly

This one is a bit more tricky and I'm less certain about the best behavior, so here's just my opinion:

• If writeDefaults=true, then changing defaultValue after it was stored should not change the stored value, so that we could avoid edge cases and ambiguity, when the stored value happens to match the default value. (I'm pretty sure that this is the optimal behavior.)
• If writeDefaults=false and there's no value in the storage, then:
  • we could update the ref returned by useStorage in a consistent and predictable way when defaultValue changes, OR
  • we could not update it to keep the behavior consistent regardless of writeDefaults (I slightly prefer this.)

[amw]

Just ran into this issue too. I passed read-only computed as a default and it breaks useStorage entirely – it becomes read-only too.