query-sync-storage-persister raises on Android WebView if setDomStorageEnabled is not configured

[nfm]

Describe the bug

An Android WebView that is not configured with settings.setDomStorageEnabled(true) will not expose window.localStorage for use.

query-sync-storage-persister guards against the value for storage being undefined:

query/packages/query-sync-storage-persister/src/index.ts

Line 45 in b82f05e

if (typeof storage !== 'undefined') {

I think this is necessary for old browsers that predate local storage, where window.localStorage === undefined. Given the persister is typically initialized with createSyncStoragePersister({ storage: window.localStorage }) that makes sense.

However, in the Android WebView case, it appears that window.localStorage === null - the property is present on window but the value is null rather than undefined.

As a result, the conditional linked above passes (typeof null is "object" 🤷 ), and removeClient can later raise with TypeError: Cannot read properties of null (reading 'removeItem'):

query/packages/query-sync-storage-persister/src/index.ts

Lines 81 to 83 in b82f05e

	removeClient: () => {
	storage.removeItem(key)
	},

It looks like persistClient doesn't hit this because trySave has some error handling around it:

query/packages/query-sync-storage-persister/src/index.ts

Lines 46 to 53 in b82f05e

	const trySave = (persistedClient: PersistedClient): Error | undefined => {
	try {
	storage.setItem(key, serialize(persistedClient))
	return
	} catch (error) {
	return error as Error
	}
	}

I can guard against this when initializing createSyncStoragePersister, but I think it probably makes sense to relax the conditional in createSyncStoragePersister to just if (storage) { ... } unless there's a good reason not to.

Your minimal, reproducible example

None sorry, but the bug is fairly trivial

Steps to reproduce

To reproduce you'd need an app installed with a WebView configured in a particular way (unsure if it would need to be configured as settings.setDomStorageEnabled(false), or if that's the default).

Whatever triggers removeClient would then throw rather than noop.

Expected behavior

If window.localStorage is null, initializing the query persister via createSyncStoragePersister({ storage: window.localStorage }) should behave the same as if window.localStorage is undefined - queries aren't persisted but no exceptions are raised.

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

• OS: Android
• Browser: Chrome Mobile WebView
• Version: Appears to affect all versions (have seen Sentry reports from Chrome Mobile WebView 72.0.3626 all the way through to Chrome Mobile WebView 110.0.5481)

TanStack Query version

@tanstack/query-sync-storage-persister@4.24.4

TypeScript version

4.7.4

Additional context

No response

[TkDodo]

I think this is necessary for old browsers that predate local storage, where window.localStorage === undefined.

not quite. we guard against undefined because we allow to explicitly pass undefined in for cases where you server-render you application with local persistence. On the server, there is no storage, but you still have to render the app:

query/packages/query-sync-storage-persister/src/index.ts

Lines 14 to 17 in b82f05e

	/** The storage client used for setting and retrieving items from cache.
	* For SSR pass in `undefined`.
	*/
	storage: Storage | undefined

But yeah, we can relax the check, because the typings don't allow anything else anyways. Would you like to contribute that?

[nfm]

Thanks @TkDodo, and sorry for the delayed response on my end!

I've just opened a PR for this: #5095. First time contributing to TanStack/query - please let me know if there's anything I need to adjust.