fix(useStorage): emit a custom event to support custom storage areas

[43081j]

Description

Fixes #2605
Fixes #2567

Basically when you introduced a fix to update instances on the same window using the same store (800f74f), you did this by dispatching a StorageEvent (such that the already existent listener would catch it and update the value as needed).

This caused a breaking change in that things like pinia stores would no longer work with it.

The reason for that is that you can only construct a StorageEvent if the storage area is built-in pretty much (e.g. local storage, session storage, etc). You definitely cannot construct one with a custom storage area such as pinia (anything that conforms to your StorageLike interface).

All of the useStorage tests were throwing an exception because of this, which was being eaten up by the onError handler (which the tests don't assert wasn't called, probably a good improvement in another PR to check that).

To fix, you could just fake a storage event (i.e. fire a CustomEvent with the name 'storage'). However, that will confuse any third party code elsewhere which relies on the storage event being an actual StorageEvent.

Even if you somehow create your own event with the same interface (so it can't be a CustomEvent, since that'd nest your values in detail), you'd then be breaking the fact that storage events are only meant to fire when another window mutated the store.

Because of all of the above, i opted here to create a custom event specific to vueuse which we listen for. That way we can safely dispatch it without affecting any third-party code and can do so with any StorageLike area.

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[43081j]

anyone i can nudge to move this forward?

happy to rework anything if needed, as long as we can fix it

[younggglcy]

This PR hasn't fixed these issues yet and calling useEventListener here caused #2782.

IMO, the core problem is that after executing window.dispatchEvent, update was called but watch didn't be resumed correctly, that's why localStorage didn't sync up with pinia store after the first change. Having no ideas about how to solve this and why it goes this way.

[43081j]

@GODLiangCY no worries, i hadn't seen that related issue at all so thanks for pointing that out

confident the fix in this pr was needed, but does seem like there's still a bug (introduced by this or not).

i did try debug this and got a little tangled up in the watcher logic. will take another look soon and see if i can figure it out

since the original event changes, it never worked with non-native stores like pinia. so i suspect whatever is broken now was also broken before this PR's change (and introduced by the original event listener change).

[43081j]

to be clear it also seems the issue you linked has a repro with a version before this particular PR landed (so the extra useEventListener call did not cause that issue)

this PR seems to be shipped in 9.13.0 while the referenced issue is using 9.8.2

so it does indeed look like its a bug that was in the original event changes, before this PR. will still try figure it out though

[younggglcy]

to be clear it also seems the issue you linked has a repro with a version before this particular PR landed (so the extra useEventListener call did not cause that issue)

Apologize for my carelessness!!

I agree with you that this pr is needed and solves problems you mentioned in desciptions. 800f74 commit do not work well with pinia's store, maybe something like store.count++ did not trigger watch? I've made a demo which runs expectly, hope it could help

[43081j]

i think i roughly understand it now

if you have a watcher on storage.value, the following will happen as of that mentioned commit:

• set storage.value
• useStorage instance triggers, sets the inner storage value (e.g. local storage)
• useStorage instance dispatches event
• all useStorage instances receive event, pause their own watcher then set their internal value

the instance which fired the event will also catch its own event, leading to setting it twice. though i'd hope that'd be fine since you would be setting the value to what it already is, i.e. noop.

the issue you mentioned before, too, could be a separate thing. hes mutating a key within an object in a store. doing that will dispatch an event to tell other instances the overall object changed, so any watcher watching anything inside that object will fire i imagine. that probably explains the behaviour they saw

no solution off the top of my head (yet) but at least we probably now understand the two issues.