-
Notifications
You must be signed in to change notification settings - Fork 11
feat: Add igniter task to convert an app to use @tanstack/db #102
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: Add igniter task to convert an app to use @tanstack/db #102
Conversation
cfacb39 to
82d4baf
Compare
|
Just to flag Has a typo. Guessing this is just the PR description , not the code though. |
thruflo
left a comment
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.
Looks great. Slightly nervous that it's based on my burn auth code but perhaps it's as good as any as a starting point.
The main thing I'm thinking / aware of is that we have a TanStack Start based starter, and we're updating this for our new quickstart guide. I wonder if that could provide some externally maintained boilerplate that we could leverage here. I.e.: so the vendor frontend code in here doesn't get out of date over time.
I think this is great to start with though and it does the hard part of getting the Vite setup established.
The other thought I have is that the example app doesn't actually use a TanStack DB collection. It would perhaps be possible to generate with:
- either a proper sync stack, so add a schema, expose via the router, sync into a collection, render that collection, implement the mutation handler and expose an ingest endpoint with that schema allowed in a writer pipeline; this would actually show the end to end machinery working, with a harmless / dummy table
- or just with a localOnlyCollection -- so at least have a TanStack DB collection in the example front end code, with insert and delete operations working in the example React code in the index route. This could have a comment linking to the docs to migrate to an electric collection for actual Phoenix sync
I think without at least one actual collection being used we're missing a bit of a trick. There is obviously the auth collection in there which could be the one. But we're not rendering anything from is, as far as I can see? Maybe if we show the auth state in the template that could work but then we'd need to wire up to actual API auth which I think it larger scope than either of the two options above.
|
|
||
| @spec short_doc() :: String.t() | ||
| def short_doc do | ||
| "Convert a Phoenix application to a Tanstack DB based frontend" |
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.
... to use a Vite + TanStack DB based frontend?
|
|
||
| @spec example() :: String.t() | ||
| def example do | ||
| "mix phx.sync.tanstack_db" |
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.
There's no verb here. Like "phx.sync.convert_to_tanstack_db" or "phx.sync.gen_tanstack_db". I'm not sure what the right command is. Given it's a bit of a weird patchy thing, maybe just as is is fine but it's not the most descriptive command as is.
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.
Yeah. You've identified what's wrong with the task name. I'll have a think. Maybe the gen prefix is the right one. Maybe it's mix phx.sync.gen.tanstack_db which follows the patterns of the standard phx tasks like mix phx.gen.schema
I thought of this but it's a lot of backend code, migrations, models, etc etc that no-one is going to want. I think the right way to do this is to have another task that generates that example/bootstrap code from an existing schema.
That sounds much more useful for this stage. |
82d4baf to
797174c
Compare
|
@thruflo have replaced the frontend code with a simple todo list (surprise!) backed by a localonlycollection and renamed the task to |
thruflo
left a comment
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.
Awesome 👍
fix problems with js config
1e39c69 to
fe73d07
Compare
From
mix help phx.sync.tanstack_db.setup:Closes #83