Skip to content

feat: make stabilization delay configurable #348

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: 11-10-fix_add_delay_after_establishing_the_realtime_channel_so_it_is_stable
Choose a base branch
from
Open

feat: make stabilization delay configurable #348

wants to merge 1 commit into from

Conversation

@jumski
Copy link
Contributor

@jumski jumski commented Nov 11, 2025

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Nov 11, 2025 •
edited
Loading

⚠️ No Changeset found

Latest commit: 01e016c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@jumski Graphite App
Copy link
Contributor Author

jumski commented Nov 11, 2025 •
edited
Loading

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • merge:queue - adds this PR to the back of the merge queue
  • hotfix:queue - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@jumski jumski mentioned this pull request Nov 11, 2025
Open
@jumski jumski force-pushed the 11-10-fix_add_delay_after_establishing_the_realtime_channel_so_it_is_stable branch from 106f9d3 to 0c591fe Compare November 11, 2025 11:33
@jumski jumski force-pushed the 11-11-feat_make_stabilization_delay_configurable branch from 76f307e to 48e0581 Compare November 11, 2025 11:33
@nx-cloud
Copy link

nx-cloud bot commented Nov 11, 2025 •
edited
Loading

🤖 Nx Cloud AI Fix Eligible

An automatically generated fix could have helped fix failing tasks for this run, but Self-healing CI is disabled for this workspace. Visit workspace settings to enable it and get automatic fixes in future runs.

To disable these notifications, a workspace admin can disable them in workspace settings.

View your CI Pipeline Execution ↗ for commit 771a319

Command Status Duration Result
nx affected -t lint typecheck test --parallel -... ❌ Failed 9m 2s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-11 16:41:02 UTC

@jumski jumski force-pushed the 11-11-feat_make_stabilization_delay_configurable branch from 48e0581 to bc7239e Compare November 11, 2025 14:34
@jumski jumski force-pushed the 11-10-fix_add_delay_after_establishing_the_realtime_channel_so_it_is_stable branch from 7d415e0 to 1da476a Compare November 11, 2025 14:47
@jumski jumski force-pushed the 11-11-feat_make_stabilization_delay_configurable branch 2 times, most recently from 5c7b1b4 to 771a319 Compare November 11, 2025 16:30
@jumski jumski mentioned this pull request Nov 11, 2025
Open
graphite-app[bot]
graphite-app bot reviewed Nov 11, 2025
View reviewed changes
pkgs/client/src/lib/SupabaseBroadcastAdapter.ts
) {
this.#supabase = supabase;
this.#reconnectionDelay = opts.reconnectDelayMs ?? 2000;
this.#stabilizationDelay = opts.stabilizationDelayMs ?? 300;
Copy link
Contributor

@graphite-app graphite-app bot Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The nullish coalescing operator (??) treats 0 as a valid value, but since 0 is falsy in JavaScript, using || would cause issues. The current implementation with ?? is correct and will properly handle 0 as a valid delay value.

To verify this behavior:

const test1 = 0 ?? 300;  // result: 0
const test2 = 0 || 300;  // result: 300

The test case for 0ms delay should pass with the current implementation.

Spotted by Graphite Agent

Fix in Graphite

Is this helpful? React 👍 or 👎 to let us know.

@jumski jumski force-pushed the 11-10-fix_add_delay_after_establishing_the_realtime_channel_so_it_is_stable branch from 1da476a to 81fcc5b Compare November 12, 2025 23:13
@jumski jumski force-pushed the 11-11-feat_make_stabilization_delay_configurable branch from 771a319 to 01e016c Compare November 12, 2025 23:13
@jumski jumski mentioned this pull request Nov 12, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@graphite-app graphite-app[bot] graphite-app[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jumski