Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(query-sync-storage-persister): Relax undefined check against storage to handle null values #5095

Conversation

nfm
Copy link
Contributor

@nfm nfm commented Mar 7, 2023

Fixes #4957.

On Android WebView if setDomStorageEnabled is not configured, window.localStorage will be null (rather than undefined).

Relax the existing conditional to check if storage is present to handle both null and undefined values to remove a gotcha here.

@vercel
Copy link

vercel bot commented Mar 7, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated
query ⬜️ Ignored (Inspect) Mar 17, 2023 at 4:41PM (UTC)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 7, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 89dc2b2:

Sandbox Source
@tanstack/query-example-react-basic-typescript Configuration
@tanstack/query-example-solid-basic-typescript Configuration
@tanstack/query-example-svelte-basic Configuration
@tanstack/query-example-vue-basic Configuration

@TkDodo
Copy link
Collaborator

TkDodo commented Mar 10, 2023

@nfm doesn't the async persister have the same guard?

@nfm
Copy link
Contributor Author

nfm commented Mar 14, 2023

@nfm doesn't the async persister have the same guard?

Thanks @TkDodo - looks like it does 👍 I haven't used the async persister but it looks like it's susceptible to the same issue. I'll push a fix for this too.

Fixes TanStack#4957.

On Android WebView if `setDomStorageEnabled` is not configured,
`window.localStorage` will be `null` (rather than `undefined`).

Relax the existing conditional to check if `storage` is present to
handle both `null` and `undefined` values to remove a gotcha here.
@nfm nfm force-pushed the 4957-query-sync-storage-persister-raises-on-android-webview-if-setdomstorageenabled-is-not-configured branch from f6f21b6 to 620e3b4 Compare March 14, 2023 10:13
@nfm
Copy link
Contributor Author

nfm commented Mar 17, 2023

It looks like this failed a test but I'm not sure why - from what I can gather it looks unrelated:

 FAIL   react-query  src/__tests__/useQuery.test.tsx (33.03 s)
  ● useQuery › should call onError after a query has been fetched with an error

Please let me know if I've done something silly here 🙃

…android-webview-if-setdomstorageenabled-is-not-configured
@TkDodo
Copy link
Collaborator

TkDodo commented Mar 17, 2023

just our standard test flakiness ... working on it 😅

@TkDodo TkDodo merged commit d23d893 into TanStack:main Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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