fix(query-core): remove isServer check from refetch interval logic#10164
fix(query-core): remove isServer check from refetch interval logic#10164darshjme-codes wants to merge 1 commit intoTanStack:mainfrom
Conversation
Fixes TanStack#10139 ## Problem The isServer check in QueryObserver's #updateStaleTimeout and #updateRefetchInterval methods prevents refetch intervals from running in environments where window is undefined, such as: - Chrome extensions - VSCode extensions - Electron renderer processes This occurs because isServer is defined as: typeof window === 'undefined' || 'Deno' in globalThis These environments are NOT servers but have no window global, causing the isServer check to incorrectly skip interval setup. ## Solution Remove the isServer check from both timeout methods. This is safe because: 1. QueryObserver subscriptions only run on the client (they execute in React effects/useSyncExternalStore, never on the server) 2. The isServer check is redundant - if code reaches these methods, we're already on the client 3. Other guards (enabled check, timeout validation) provide sufficient protection ## Impact - Fixes refetch intervals in Chrome/VSCode extensions - Maintains existing behavior for normal browser environments - No server-side impact (code never runs on server anyway) Suggested by @TkDodo in discussion TanStack#4018
|
📝 WalkthroughWalkthroughRemoved Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/query-core/src/queryObserver.ts (1)
7-7:⚠️ Potential issue | 🟡 MinorRemove the unused
isServerimport introduced by this PR.The
isServervariable was removed from both#updateStaleTimeout(line 361) and#updateRefetchInterval(lines 391–397) in this PR, but the import on line 7 was not cleaned up. Since the project's roottsconfig.jsonhasnoUnusedLocals: trueenabled, this will cause a build failure.🧹 Cleanup
import { - isServer, isValidTimeout, noop, replaceData, resolveEnabled, resolveStaleTime, shallowEqualObjects, timeUntilStale, } from './utils'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/query-core/src/queryObserver.ts` at line 7, Remove the now-unused isServer import from the import list in queryObserver.ts; since isServer was removed from the private methods `#updateStaleTimeout` and `#updateRefetchInterval`, delete isServer from the top-level imports to satisfy noUnusedLocals and avoid the build failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/query-core/src/queryObserver.ts`:
- Line 7: Remove the now-unused isServer import from the import list in
queryObserver.ts; since isServer was removed from the private methods
`#updateStaleTimeout` and `#updateRefetchInterval`, delete isServer from the
top-level imports to satisfy noUnusedLocals and avoid the build failure.
Fixes #10139
Problem
The
isServercheck in QueryObserver's#updateStaleTimeoutand#updateRefetchIntervalmethods prevents refetch intervals from running in environments wherewindowis undefined, such as:globalThis.windowis undefinedThis occurs because
isServeris defined as:These environments are NOT servers but have no
windowglobal, causing theisServercheck to incorrectly skip interval setup.Solution
Remove the
isServercheck from both timeout methods. This is safe because:enabledcheck, timeout validation, etc.Changes
isServer ||check from#updateStaleTimeout(line 361)isServer ||check from#updateRefetchInterval(line 392)Impact
✅ Fixes: Refetch intervals now work in Chrome/VSCode extensions
✅ Maintains: Existing behavior for normal browser environments
✅ No server impact: Code never runs on server anyway
Testing
This fix should be tested in:
As suggested by @TkDodo in #4018 (reply in thread)
Summary by CodeRabbit