Skip to content
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

feat(zbugs): useWatchQuery #3307

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(zbugs): useWatchQuery #3307

wants to merge 1 commit into from

Conversation

arv
Copy link
Contributor

@arv arv commented Dec 13, 2024

This is WIP but I want some feedback.

This creates a new hook that takes a callback that gets called with the changes that come out at the end of the pipeline.

This is based on the ideas I had last week (or the week before that?) when I first worked on adding the emoji change callbacks.

Usage:

useWatchQuery(z.query.comment.where('issueID', id), change => {
  if (change.type === 'add') {
    console.log(change.row, 'was added');
  } else if (change.type === 'remove') {
    console.log(change.row, 'was removed');
  }
});

Things that are not obvious/clean.

  • Name of the hook?
  • The implementation waits for the transaction end but this information is lost in the callback. We could call the callback with an array of all the changes instead.
  • What to do about child changes since at this level there is no memory of the previous children?
  • Should the hook have an option to also be called for non complete results or maybe add that info to the change objects?

Copy link

vercel bot commented Dec 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
replicache-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 13, 2024 3:17pm
zbugs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 13, 2024 3:17pm

@tantaman
Copy link
Contributor

tantaman commented Dec 13, 2024

  • Name of the hook?

useDiffQuery ?

  • The implementation waits for the transaction end but this information is lost in the callback. We could call the callback with an array of all the changes instead.

Array of all the changes would make sense but I think each change has to be processed as it is received if the callback is consuming relationships.

  • What to do about child changes since at this level there is no memory of the previous children?

I don't follow.

@arv
Copy link
Contributor Author

arv commented Dec 14, 2024

Array of all the changes would make sense but I think each change has to be processed as it is received if the callback is consuming relationships.

Now I don't follow :-)

What to do about child changes since at this level there is no memory of the previous children?

I don't follow.

At this point in time the child relationships are ignored. What should really happen is that I should use the applyChange logic (which needs some fixes) so that the relationships gets filled in.


This is pretty incomplete and that is why I decided to put it in zbugs and not in zero-client.

return changeView;
}

export type Change<TSchema extends TableSchema> =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to define a new type to prevent tight coupling with the ivm Change type

export type Change<TSchema extends TableSchema> =
| {
type: 'add';
row: Row<TSchema>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These do not contain the relationships

}
}

function changeViewFactory<
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm I wonder if we can reasonably push this down a layer so that it can be used in non-React environments like Cloudflare-DO, NodeJS, vanilla JS.

@aboodman
Copy link
Contributor

aboodman commented Dec 21, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants