-
Notifications
You must be signed in to change notification settings - Fork 295
feat: add @electric-sql/start package that interacts with Cloud
#3595
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
Conversation
✅ Deploy Preview for electric-next ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
0371010 to
280c242
Compare
280c242 to
da43856
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3595 +/- ##
==========================================
- Coverage 88.13% 87.91% -0.22%
==========================================
Files 18 21 +3
Lines 1643 1821 +178
Branches 412 462 +50
==========================================
+ Hits 1448 1601 +153
- Misses 193 218 +25
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
ac678e0 to
fc7cc2b
Compare
@electric-sql/start package that interacts with Cloud
fc7cc2b to
3451d04
Compare
3156614 to
e3e7d5e
Compare
This comment has been minimized.
This comment has been minimized.
e3e7d5e to
3bdbd55
Compare
This comment has been minimized.
This comment has been minimized.
|
The trouble with the https://tanstack-start-db-electric-starter.localhost approach in the current starter is it doesn't work in Safari:
There's no actual need to use the subdomain like this. We can just run on a normal / default vite address. In the port-sharing version of the start I did, #3228, I solve this and also remove the need for Caddy and avoid the double security popup when installing. For me, running this starter from scratch on new macOS, I get two system-level security popups and a console warning when I run |
|
There's also another DX problem which is that running the migrations after the dev server makes it likely to forget to run the migrations. You then open the web browser, try to login and get an auth error. I think it may be better to just do less magic and have explicit instructions in the README and docker version of the Quickstart to (a) ensure you have caddy and have run caddy trust (b) run the backend (c) then run the migrations (d) then run the dev server. This would mean adjusting the dev command to not run the backend services. |
|
@icehaunter have updated the readme as envisaged on call earlier. Needs your changes for this to actually work. |
|
Quickstart updated to match this: #3114 |
packages/start/src/cli.ts
Outdated
| console.log(``) | ||
| console.log(`Next steps:`) | ||
| console.log(` cd ${appName}`) | ||
| console.log(` pnpm install`) |
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.
The idea here: https://deploy-preview-3114--electric-next.netlify.app/docs/quickstart#get-started is that the start command wraps up install and migrate (because it can and it just simplifies the Quickstart flow).
(This also affects the .stackblitzrc as to whether it installs reps or not).
Any reason not to switch this over (aside from explicitness)?
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.
We can do a manual pnpm install & migrate if you want, although I slightly prefer the explicitness and not auto-installing stuff. Is it also worth noting in the quickstart guide the fact that NeonDB databases will expire in about an hour unless claimed?
This supports StackBlitz forking and generally enables switching a starter template to use cloud provisioned resources via `npx @electric-sql/start .`. Summary of changes: 1. cli.ts - Added . as valid app name: - Line 11: Added usage hint for . mode - Line 18-19: Skip alphanumeric validation when appName === '.' - Line 27-31: Show "Configuring current directory..." instead of "Creating app: ." - Line 44-46: Skip "cd" instruction when using . 2. template-setup.ts - Skip gitpick for current directory: - Line 25: appPath = appName === '.' ? process.cwd() : join(process.cwd(), appName) - Lines 29-35: Only run gitpick when appName !== '.' 3. Test files - Added 5 new tests and fixed 1 pre-existing incorrect expectation This enables the StackBlitz flow where `npx @electric-sql/start .` configures an existing template directory without re-cloning it.
|
@icehaunter see 9f4db14 |
|
@thruflo - addressed your feedback in bb25a02
|
6544287 to
bb25a02
Compare
This comment has been minimized.
This comment has been minimized.
1. Only one dev command, `pnpm dev`, driven by `.env` file 2. `ELECTRIC_URL` being set takes the precedence - if you want to run against docker, you need to unset it or set to the docker url 3. `@electric-sql/start` will run `pnpm install` & `pnpm migrate` immediately to match quickstart 4. Made sure Caddy runs on `localhost:4000` with an HTTPS cert instead of `.localhost` domain
bb25a02 to
02468b6
Compare
A simpler Quickstart based on the tanstack-db.web-starter and @electric-sql/start package: https://deploy-preview-3114--electric-next.netlify.app/docs/quickstart Relies on #3595 being finished and published and #3634 being merged and used to deploy the Quickstart demo.

@icehaunter I remember now that this WIP was me pulling out the start package specifics to a clean PR, then fixing stuff in it. See the TODO.md. When verified as working, this PR can be merged and the
@electric-sql/startpackage published, and then the quickstart branch can be published with just the website changes. That's the idea ...