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

refactor: better replicache strategy #3

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

hdoro
Copy link

@hdoro hdoro commented Oct 22, 2024

Heya 👋

Thank you for the inspiration - I was a bit stuck into overthinking how to turn my EdgeDB project into a local-first app, and Replicache is a great way forward.

I've outline a few questions in #2 , and in my research process ended up making various change to the repository. Reasons for merging:

  • the TodoList component has better UX and allows for editing todos' content
  • the syncing strategy is more efficient than the previous reset strategy, see Replicache's row version strategy
    • The current implementation feels incomplete for people looking to build serious apps, and it's not recommended by the Replicache team for apps with more frequent writes and larger data sets.
  • querying is done via the Typescript query builder, which makes for self-documenting code and great type checking & inference
  • better highlights EdgeDB's powerful SDL with abstract types, deletion policies, triggers, indexes, constraints, and mutation rewrites
  • clearer organization for defining mutations - mutators.client.ts for determining behavior in the front-end, mutators.db.ts for how to persist mutations to EdgeDB, and mutation.types.ts for their arguments' schema
  • upgraded to Next 15

Reasons for not merging:

  • I'm not 100% certain of the consistency of the sync strategy. Not an issue anymore I've gotten feedback from Aaron Boodman, the creator of Replicache, which revealed the issues with my approach. I've now successfully implemented the row versioning strategy
    • A second pair of eyes is highly appreciated, though!
  • Increases the barrier to entry - I've introduced zod as a new concept and more code
    • hopefully this is offset partially by better typescript types and comments on the sync endpoints

Before I go through the TODOs I've left in the code, I'd like to know if this PR makes sense to you. Thanks in advance for the attention 🙏

Copy link

vercel bot commented Oct 22, 2024

@hdoro is attempting to deploy a commit to the EdgeDB Team on Vercel.

A member of the Team first needs to authorize it.

@hdoro hdoro force-pushed the refactor/better-replicache-strategy branch from 11ad905 to 478d501 Compare October 23, 2024 17:32
@scotttrinh
Copy link
Contributor

Thanks @hdoro for your contribution here and it's great to hear from you! Hope the gororobas project is going well still. I haven't had a chance to really dive in, but I wanted to give you a sense of why we ended up with what we have here:

The template is a starting point for someone wanting to build their own thing, and as such, we wanted to build the most minimal setup that actually worked without people having to dive deeply into Replicache. I think that goal is somewhat at odds with "build[ing] serious apps", but it's a conscious choice. Our vision of how this template is used is that a developer would start with a very simple set up that works. Then as the developer wants to build it out in a more robust way, they can take a deeper dive into Replicache and start converting it to a more robust strategy. More of an iterative approach.

To that end, I think we're happy to take contributions that align with that goal and are happy to work with you on this PR to make it acceptable! It's very possible that we can find something that strikes the right balance between a simple starting point, and a robust setup.

Copy link
Contributor

@scotttrinh scotttrinh left a comment

Choose a reason for hiding this comment

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

Some early feedback!

@@ -1,49 +1,99 @@
module default {
global currentGroupID: str;
global currentGroup := (
global current_group_id: str;
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it's more EdgeQL-y to use snake_case, but for JS/TS projects, we've been leaning toward matching the common idiom there and using camelCase.

Copy link
Author

Choose a reason for hiding this comment

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

ahhhh why have I done this to myself? I've been finding snake_case easier to read, but you're right, the ecosystem points to camelCase

Should I go for it?

lib/components/TodoList.tsx Outdated Show resolved Hide resolved
lib/replicache.types.ts Outdated Show resolved Hide resolved
}

export const CustomPullRequest = z.object({
cookie: z.union([z.number(), z.null()]),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
cookie: z.union([z.number(), z.null()]),
cookie: z.number().nullable(),

Gives better error messages. z.union gives a pretty complicated error message since it gives you one for each member of the union that failed.

Copy link
Author

Choose a reason for hiding this comment

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

I've switched the Replicache strategy and made use of z.nullable, which I assume yields the same results?

app/replicache/pull/route.ts Outdated Show resolved Hide resolved
@hdoro
Copy link
Author

hdoro commented Oct 24, 2024

@scotttrinh I've done a big rehaul to follow the row-version strategy.

Thankfully, Aaron Boodman showed me my approach was completely incorrect, so now we're in a good shape. The code is more complex than the reset strategy, but it's also more robust and well documented.

I've recorded a video going through the working app and why I think the added complexity of the row-version strategy is worth it, may save you a few minutes of code review :)

2024-10-24.EdgeDB.Replicache.Walkthrough.mp4

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.

2 participants