Skip to content

feat(zero): offline #4652

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
Merged
merged 15 commits into from
Jul 30, 2025
Merged

feat(zero): offline #4652

merged 15 commits into from
Jul 30, 2025

Conversation

0xcadams
Copy link
Member

@0xcadams 0xcadams commented Jul 18, 2025 •
edited
Loading

There was a usability issue with the online status with Zero. React doesn't rerender when Zero's online property updates. This proposes an API, onOnline, to make it easier to listen for updates to this. And an accompanying hook in React/SolidJS, useZeroOnline.

@vercel Vercel
Copy link

vercel bot commented Jul 18, 2025

@0xcadams is attempting to deploy a commit to the Rocicorp Team on Vercel.

A member of the Team first needs to authorize it.

0xcadams
0xcadams commented Jul 18, 2025
View reviewed changes
apps/zbugs/src/pages/issue/issue-composer.tsx Outdated

const raceResult = await promiseRace([sleep(5000), result.server]);
if (raceResult === 0) {
// TODO show toast
Copy link
Member Author

Choose a reason for hiding this comment

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

Do we want this to be a toast?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that we should rely on a new global onError handler that Matt is constantly wanting to add. It is half-implemented in the codebase right now, but we got distracted by API design questions and never finished it.

I don't want every mutator to have to do this kind of error handling manually.

Copy link
Member Author

Choose a reason for hiding this comment

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

This makes sense - agreed it would be much better to have this in onError. So I can add this toast later and only have this PR include online indicator, once the onError handler lands.

0xcadams
0xcadams commented Jul 18, 2025
View reviewed changes
@aboodman
Copy link
Contributor

I'm confused - we already have onOnlineChange. Why not use that at the basis for useOnline?

@aboodman
Copy link
Contributor

aboodman commented Jul 18, 2025 •
edited
Loading

@alexhking can you design an online indicator for zbugs? Note that people will probably copy whatever we do into their apps.

Here are my product thoughts:

  1. I do not (cannot stress enough how much I do not want this) want it to animate or throb or do anything when syncing. This just becomes a spinner and destroys the feeling of instant.
  2. I think when online, it should be invisible. Linear doesn't have an 'online' state. You assume it is online.
  3. I am not sure where it should go visually.
  4. As always I think it should be simple, minimal, tasteful.
  5. Maybe we should consider animations and transitions when going offline and returning online.

@0xcadams
Copy link
Member Author

I'm confused - we already have onOnlineChange. Why not use that at the basis for useOnline?

Yeah I can do that - it just means managing that state in two places and adds a new Context. Will change.

@aboodman
Copy link
Contributor

I'm confused - we already have onOnlineChange. Why not use that at the basis for useOnline?

Yeah I can do that - it just means managing that state in two places and adds a new Context. Will change.

I am missing something.... it seems trivial to wrap a hook around the existing feature. I don't get why any change is needed in zero.ts.

@aboodman
Copy link
Contributor

Oooh it is because there can be many hooks, but there can only be one onOnlineChange event. Yeah that was super ghetto, let's fix that. Good call. Will review PR with that frame.

aboodman
aboodman reviewed Jul 18, 2025
View reviewed changes
Copy link
Contributor

@aboodman aboodman left a comment

Choose a reason for hiding this comment

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

Nice, I like this.

apps/zbugs/src/pages/issue/issue-composer.tsx Outdated

const raceResult = await promiseRace([sleep(5000), result.server]);
if (raceResult === 0) {
// TODO show toast
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that we should rely on a new global onError handler that Matt is constantly wanting to add. It is half-implemented in the codebase right now, but we got distracted by API design questions and never finished it.

I don't want every mutator to have to do this kind of error handling manually.

packages/shared/src/subscribable.ts
@@ -0,0 +1,38 @@
export class Subscribable<
Copy link
Contributor

Choose a reason for hiding this comment

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

I was going to suggest nanoevent but this is so simple maybe it makes sense to have it in our codebase in case we want to change it.

@arv @tantaman ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also @arv just confirming there's nothing like this already in Replicache so we don't duplicate code?

packages/zero-client/src/client/zero.ts Outdated
* @param listener - The listener to subscribe to.
* @returns A function to unsubscribe the listener.
*/
addOnlineChangeListener = (
Copy link
Contributor

Choose a reason for hiding this comment

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

How about onOnline and then we deprecate onOnlineChange.

packages/zero-client/src/client/zero.ts
);
}

this.#onOnlineChange = onOnlineChange;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a breaking change but it is so simple to update I am supportive.