(graphcache) - default storage's offline metadata not shared between 2 tabs

[ctesniere]

Describe the bug

Hi 👋,

This bug is from urql/offlineExchange.
When I'm in offline mode on both tabs:

Tab 1 :
I click several times on the "Click to simulate a new mutation" button (see demo link).
My queries fail because of the offline mode and are stored in metadata of indexedDB.
=> metadata (indexedDB) contains multiple queries.

Tab 2 :
I click once on the "Click to simulate a new mutation" button.
My request fails because of the offline mode and is stored in metadata of indexedDB.
=> metadata (indexedDB) now contains more than one query, others have been removed !

Proposition of a solution :
This bug occurs because we add an element in the failedQueue array without updating it just before with storage.readMetadata()
Here :

urql/exchanges/graphcache/src/offlineExchange.ts

Lines 126 to 130 in 3dd23be

	) {
	failedQueue.push(res.operation);
	updateMetadata();
	return false;
	}

The flushQueue function should use storage.readMetadata() instead to failedQueue

urql/exchanges/graphcache/src/offlineExchange.ts

Line 102 in 3dd23be

for (let i = 0; i < failedQueue.length; i++) {

Reproduction

Source code : https://codesandbox.io/s/admiring-panini-yczq7c
Demo : https://yczq7c.csb.app/

Urql version

"@urql/exchange-graphcache": "5.0.5"
"urql": "3.0.3"

Validations

• I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which GitHub Discussions should be used
• Read the docs.
• Follow our Code of Conduct

[kitten]

I think, while it's possible for us to address this, I'm unsure of whether to actively track this, for two reasons basically,

The first reason is that we haven't address this exchange RFC yet: #1062
While it's vaguely unrelated, I think, there's a chance that offline access will never be perfect between these two methods.

This blends into the second reason. While we're using IndexedDB, we strategically chose it becaus it's the only solution that's sufficiently fast for our cached data. Optimally for the metadata however, we'd want to instead use localStorage in an ideal world. However, it seemed like an odd choice to do both.

Consider this, if we allow two tabs do we also let both caches' override each others data? We probably would not want that either. Do we then try to additively add more data from both tabs? That can be really dangerous. Even though we split patches into periodic time segments, we have no guarantee that they'll merge cleanly between each other.

Hence, the only sane way to solve this is to have a synchronisation mechanism between tabs in place already that we could test at the same time — in fact, an optimal solution would synchronise the offline exchange's data backing between multiple tabs at all time anyway.

The reason why we haven't done this is the time/maintenance burden.
That's basically why we call it the "Default Storage" — to imply that it's not the only option. So I'll have to give this a quick think, but I reckon there may not be a complete solution here. If we fix the metadata issue, we may run straight ahead into a cache batch sync issue with the default storage.

[ctesniere]

Thank you for your answer.
Indeed, this issue is a duplicate of #1062.