-
Notifications
You must be signed in to change notification settings - Fork 305
Add configurable delay for pausing sync when tab is backgrounded #3762
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
base: main
Are you sure you want to change the base?
Conversation
…ab is backgrounded This adds a new option `backgroundPauseDelayMs` to ShapeStreamOptions that allows users to configure a delay before sync is paused when a tab is backgrounded. Previously, sync would pause immediately when the tab was hidden. This could cause a jarring "jump" when users return to a tab, as updates need to sync back in. With this option, users can keep sync active for a configurable period (e.g., 5 minutes) after the tab is backgrounded, improving the experience when quickly switching tabs. The default value is 0 (immediate pause) to maintain backwards compatibility.
Update the default from 0 (immediate pause) to 10 minutes to provide a better out-of-the-box experience. Users can still set it to 0 for immediate pause behavior. Also update existing tests to explicitly set backgroundPauseDelayMs: 0 to maintain their expected immediate-pause behavior.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe PR adds a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
commit: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3762 +/- ##
==========================================
- Coverage 87.36% 87.22% -0.15%
==========================================
Files 23 23
Lines 2011 2027 +16
Branches 531 534 +3
==========================================
+ Hits 1757 1768 +11
- Misses 252 257 +5
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/typescript-client/test/client.test.ts`:
- Around line 551-647: The tests creating ShapeStream (the two background-pause
tests that instantiate new ShapeStream({... backgroundPauseDelayMs: 500 })) rely
on the real fetch and should inject a mocked fetch client via the ShapeStream
options (shapeOptions.fetchClient) instead; update both test cases to pass a
mocked fetch client (use your test harness/mock helper used elsewhere) in the
ShapeStream constructor options so the stream uses the mock fetch, and apply the
same pattern to the other background-pause tests in this file to follow testing
guidelines.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/typescript-client/src/client.tspackages/typescript-client/test/client.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Use
@tanstack/{angular,react,solid,svelte,vue}-dbframework-specific imports instead of generic TanStack DB
Files:
packages/typescript-client/src/client.tspackages/typescript-client/test/client.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Mock fetch client in tests via
shapeOptions.fetchClientparameter
Files:
packages/typescript-client/test/client.test.ts
🧠 Learnings (1)
📚 Learning: 2026-01-14T14:45:05.855Z
Learnt from: CR
Repo: electric-sql/electric PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-14T14:45:05.855Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Mock fetch client in tests via `shapeOptions.fetchClient` parameter
Applied to files:
packages/typescript-client/test/client.test.ts
🧬 Code graph analysis (1)
packages/typescript-client/test/client.test.ts (1)
packages/typescript-client/src/client.ts (1)
ShapeStream(547-1730)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test packages/y-electric w/ sync-service
- GitHub Check: Test packages/experimental w/ sync-service
- GitHub Check: Test packages/typescript-client w/ sync-service
- GitHub Check: Test packages/start w/ sync-service
- GitHub Check: Test packages/react-hooks w/ sync-service
- GitHub Check: Run Lux integration tests
🔇 Additional comments (7)
packages/typescript-client/src/client.ts (5)
432-453: Clear API docs for the new delay option.Examples and default behavior are explicit and easy to follow.
602-603: State tracking for delayed pause looks good.
648-649: Default delay wiring matches the documented behavior.
1435-1439: Nice cleanup of pending timeout during unsubscribe.
1558-1581: Visibility handler correctly schedules and cancels delayed pauses.packages/typescript-client/test/client.test.ts (2)
451-452: Explicit immediate-pause configuration is clear for this test.
478-479: Immediate-pause configuration is clear here as well.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| it(`should delay pausing the stream when backgroundPauseDelayMs is set`, async ({ | ||
| issuesTableUrl, | ||
| insertIssues, | ||
| aborter, | ||
| }) => { | ||
| const { pause, resume } = mockVisibilityApi() | ||
| const shapeStream = new ShapeStream({ | ||
| url: `${BASE_URL}/v1/shape`, | ||
| params: { | ||
| table: issuesTableUrl, | ||
| }, | ||
| signal: aborter.signal, | ||
| liveSse, | ||
| backgroundPauseDelayMs: 500, // 500ms delay before pausing | ||
| }) | ||
| const shape = new Shape(shapeStream) | ||
|
|
||
| const values: Row[][] = [] | ||
| shape.subscribe(({ rows }) => { | ||
| values.push(rows) | ||
| }) | ||
|
|
||
| // Insert an issue and wait for the initial sync | ||
| await insertIssues({ title: `test title` }) | ||
| await vi.waitFor(() => expect(values.length).toBeGreaterThan(0)) | ||
| await vi.waitFor(() => expect(shapeStream.isConnected()).true) | ||
|
|
||
| // Pause (hide tab) | ||
| pause() | ||
|
|
||
| // Stream should still be connected immediately after pausing | ||
| // because of the 500ms delay | ||
| expect(shapeStream.isConnected()).true | ||
|
|
||
| // Insert another issue while the delay is pending | ||
| const [id2] = await insertIssues({ title: `during delay` }) | ||
|
|
||
| // Wait a bit but less than the delay | ||
| await sleep(100) | ||
|
|
||
| // Should still be connected | ||
| expect(shapeStream.isConnected()).true | ||
|
|
||
| // Wait for the update to arrive (stream should still be active) | ||
| await vi.waitFor( | ||
| () => expect(values.some((rows) => rows.some((r) => r.id === id2))).true | ||
| ) | ||
|
|
||
| // Now wait for the full delay to expire | ||
| await sleep(500) | ||
|
|
||
| // Stream should now be paused | ||
| await vi.waitFor(() => expect(shapeStream.isConnected()).false) | ||
|
|
||
| // Resume | ||
| resume() | ||
| await vi.waitFor(() => expect(shapeStream.isConnected()).true) | ||
| }) | ||
|
|
||
| it(`should cancel delayed pause when tab becomes visible again`, async ({ | ||
| issuesTableUrl, | ||
| aborter, | ||
| }) => { | ||
| const { pause, resume } = mockVisibilityApi() | ||
| const shapeStream = new ShapeStream({ | ||
| url: `${BASE_URL}/v1/shape`, | ||
| params: { | ||
| table: issuesTableUrl, | ||
| }, | ||
| signal: aborter.signal, | ||
| liveSse, | ||
| backgroundPauseDelayMs: 500, | ||
| }) | ||
|
|
||
| const unsubscribe = shapeStream.subscribe(() => unsubscribe()) | ||
|
|
||
| await vi.waitFor(() => expect(shapeStream.isConnected()).true) | ||
|
|
||
| // Pause (hide tab) | ||
| pause() | ||
|
|
||
| // Wait less than the delay | ||
| await sleep(100) | ||
|
|
||
| // Should still be connected | ||
| expect(shapeStream.isConnected()).true | ||
|
|
||
| // Resume before the delay expires | ||
| resume() | ||
|
|
||
| // Wait longer than the original delay | ||
| await sleep(600) | ||
|
|
||
| // Stream should still be connected (pause was cancelled) | ||
| expect(shapeStream.isConnected()).true | ||
| }) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inject a mocked fetchClient in the new background-pause tests.
These tests rely on the default fetch. Please pass a mocked fetchClient via options to align with the testing guideline, and consider applying the same pattern to the other background-pause tests in this file.
🧩 Suggested update
it(`should delay pausing the stream when backgroundPauseDelayMs is set`, async ({
issuesTableUrl,
insertIssues,
aborter,
}) => {
+ const fetchClient = vi.fn(
+ (...args: Parameters<typeof fetch>) => fetch(...args)
+ )
const { pause, resume } = mockVisibilityApi()
const shapeStream = new ShapeStream({
url: `${BASE_URL}/v1/shape`,
params: {
table: issuesTableUrl,
},
signal: aborter.signal,
liveSse,
backgroundPauseDelayMs: 500, // 500ms delay before pausing
+ fetchClient,
}) it(`should cancel delayed pause when tab becomes visible again`, async ({
issuesTableUrl,
aborter,
}) => {
+ const fetchClient = vi.fn(
+ (...args: Parameters<typeof fetch>) => fetch(...args)
+ )
const { pause, resume } = mockVisibilityApi()
const shapeStream = new ShapeStream({
url: `${BASE_URL}/v1/shape`,
params: {
table: issuesTableUrl,
},
signal: aborter.signal,
liveSse,
backgroundPauseDelayMs: 500,
+ fetchClient,
})As per coding guidelines, mock fetch client in tests via shapeOptions.fetchClient.
🤖 Prompt for AI Agents
In `@packages/typescript-client/test/client.test.ts` around lines 551 - 647, The
tests creating ShapeStream (the two background-pause tests that instantiate new
ShapeStream({... backgroundPauseDelayMs: 500 })) rely on the real fetch and
should inject a mocked fetch client via the ShapeStream options
(shapeOptions.fetchClient) instead; update both test cases to pass a mocked
fetch client (use your test harness/mock helper used elsewhere) in the
ShapeStream constructor options so the stream uses the mock fetch, and apply the
same pattern to the other background-pause tests in this file to follow testing
guidelines.
The fetch-event-source library has its own visibility handler that immediately aborts SSE connections when the tab is hidden. This conflicts with our delayed pause behavior, causing unhandled AbortErrors in tests. Skip these tests when liveSse=true since the delayed pause feature is primarily useful for long-polling mode.
✅ Deploy Preview for electric-next ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary
This PR adds a new
backgroundPauseDelayMsoption toShapeStreamthat allows developers to configure a delay before the sync pauses when the user switches to a background tab. This reduces the perceived "jump" when returning to the tab while accounting for browsers that terminate long-running fetches in background tabs.Key Changes
backgroundPauseDelayMs?: numberoption toShapeStreamOptionsinterface with comprehensive documentation and examplesunsubscribeAll()to properly clean up any pending pause timeoutsImplementation Details
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.