fix(queryObserver): allow refetch interval in service worker (#4018)

[ElGrecode]

Resolves discussion in #4018

Note: The use of // eslint-disable-next-line is an intentional choice. The lint rule @typescript-eslint/no-unnecessary-condition works when typing is known. In this case globalThis is scoped to NodeJS's global namespace and therefore has the typings for only one environment of globalThis. It's a weird case which is why I put the ignore for this line only consistent with other uses of ignoring no-unnecessary-condition

[codesandbox-ci[bot]]

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 08684a2fca4fdea94590953391997502d2a230e5:

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

[TkDodo]

Thanks for the PR. I think the thing I have missed in the discussion is that setInterval also exists in nodeJs. So with that change, we would likely add setInterval calls during server side rendering, wouldn't we?

[ElGrecode]

Thanks for the quick comment @TkDodo !

I was under the assumption from your comment in #4018 that this subscription code would only be run in effects

No, I think we can decouple setInterval calls from isServer checks. None if this code runs on the server anyways, because subscriptions are only created on the client (they run in effects / useSyncExternalStore, so not on the server)

The check for isServer is a little tenuous at the moment, relying only on presence of !window. We can change that in the util to be more specific, but there's little consensus about which type of check is best representation of a server env. Usually it's checking specific properties of node's global object to be present.

[TkDodo]

Huh I guess you're right. Let me think about it and test it some more. Thanks for the contribution 🙌

[codecov-commenter]

Codecov Report

Base: 96.36% // Head: 96.83% // Increases project coverage by +0.46% :tada:

Coverage data is based on head (08684a2) compared to base (eab6e2c). Patch has no changes to coverable lines.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4158      +/-   ##
==========================================
+ Coverage   96.36%   96.83%   +0.46%     
==========================================
  Files          45       58      +13     
  Lines        2281     2682     +401     
  Branches      640      789     +149     
==========================================
+ Hits         2198     2597     +399     
- Misses         80       83       +3     
+ Partials        3        2       -1     

Impacted Files	Coverage Δ
...rc/createWebStoragePersistor-experimental/index.ts
src/devtools/styledComponents.ts
src/react/QueryClientProvider.tsx
src/react/reactBatchedUpdates.ts
src/core/utils.ts
src/core/subscribable.ts
src/core/focusManager.ts
src/react/logger.native.ts
src/devtools/useLocalStorage.ts
src/core/query.ts
... and 93 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[TkDodo]

what about if we would extend the isServer check to exclude the service worker environment? It seems like checking for a service worker can be done with checking for the WorkerGlobalScope ?

https://stackoverflow.com/questions/7931182/reliably-detect-if-the-script-is-executing-in-a-web-worker

I think other timeouts like staleTime and cacheTime would also profit from this?

[ElGrecode]

what about if we would extend the isServer check to exclude the service worker environment? It seems like checking for a service worker can be done with checking for the WorkerGlobalScope

Makes sense to me. I'll update

[TkDodo]

@ElGrecode how's it going? Did you have time to update the PR?

[ElGrecode]

@TkDodo I will get to this today. Sorry for dragging this PR out

[ElGrecode]

Closing as I'm unable to dedicate time to it this month unfortunately. I'm trying to maintain good house keeping for these PRs. I will reopen with this context next month.