example (sidecar): NodeJS sidecar example #772

kevin-dp · 2023-12-20T14:33:48Z

This PR introduces a NodeJS sidecar and an example application using the sidecar and the DB directly.

Steps to test the sidecar:

First build and pack the ts-client:

cd clients/typescript
pnpm build
npm pack

Modify the package.json of the sidecar (cd sidecar) to use the packed ts-client. The application can use the published ts-client.

First start the backend and apply your migrations:

cd clients/node
yarn backend:start
yarn db:migrate

Then run the sidecar:

cd sidecar
yarn start ../clients/node/foo.db

This will create the foo.db db file in the clients/node folder.

Then run the client application(s):

cd clients/node
yarn start foo.db

… sidecar.

linear · 2023-12-20T14:33:52Z

VAX-1427 Node sidecar example (that works on Windows)

balegas

It fails for me at the step of starting the sidecar. I was following the steps in the clients/node/README.md

❯ yarn build
yarn run v1.22.19
$ tsc
src/sidecar.ts:5:10 - error TS2305: Module '"electric-sql/client/model"' has no exported member 'ShapeManager'.

5 import { ShapeManager } from 'electric-sql/client/model'
           ~~~~~~~~~~~~

src/sidecar.ts:7:57 - error TS2307: Cannot find module 'electric-sql/cli/migrations' or its corresponding type declarations.

7 import { fetchMigrations, loadMigrationsMetaData } from 'electric-sql/cli/migrations'
                                                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

src/sidecar.ts:88:11 - error TS7006: Parameter 'm' implicitly has an 'any' type.

88           m => m.ops.forEach(
             ~

src/sidecar.ts:89:13 - error TS7006: Parameter 'op' implicitly has an 'any' type.

89             op => {
               ~~


Found 4 errors in the same file, starting at: src/sidecar.ts:5

error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

examples/node-sidecar/sidecar/README.md

examples/node-sidecar/sidecar/package.json

balegas · 2023-12-20T15:01:48Z

examples/node-sidecar/sidecar/README.md

+  </picture>
+</a>
+
+# ElectricSQL - NodeJS sidecar


We need top-level README.md explaining the purpose of the example project.
I would maybe put the instructions that you have on the client at top level, because it covers starting the app and the sidecar.

balegas · 2023-12-20T15:10:03Z

examples/node-sidecar/clients/node/README.md

+```sh
+git clone https://github.com/electric-sql/electric
+cd electric/examples/node-sidecar/clients/node
+yarn backend:start


yarn for installing deps

balegas · 2023-12-20T15:11:17Z

examples/node-sidecar/clients/node/README.md

+Then, start the sidecar:
+```sh
+cd electric/examples/node-sidecar/sidecar
+yarn start <path-to-db-file> # e.g. yarn start ../clients/node/electric.db


Would be nice to have a command to start the sidecar, but let's not do it now

Yep, yarn start is the command :-) you just have to specify the db file to use, that's it

without having to cd to another folder

I'm not particularly a fan of putting a single top level command that runs everything because it blurs away how it really works and the fact that the sidecar and the application are 2 different programs. In fact, i think that we want to make that explicit to the user such that they understand that they first need to run the sidecar and then their application that connects to the sidecar. We can discuss this :-)

balegas

Please double-check that all of the now exposed method have no side effects over the client state. If so, I think we can merge the changes to satellite and release a patch.

The instructions must be simple for the sidecar and right now they aren't. I have a bit of a strong opinion that the sidecar should start along the app that uses it. I'm open to discussion if you think otherwise, but I think that is the ideal experience for someone playing with this.

Note that we can't use pnpm, our scripts are not compatible with windows. I don't we need that once the patch is released.

examples/node-sidecar/clients/node/package.json

examples/node-sidecar/sidecar/package.json

balegas · 2023-12-21T12:01:29Z

examples/node-sidecar/sidecar/src/sidecar.ts

+   * Polls Electric's migration endpoint for new migrations.
+   * If there are new electrified tables we will sync them.
+   */
+  async pollMigrations(): Promise<void> {


This is okay for this shareable prototype, but for merging this couldn't we set a rawLiveQuery that watches when the migration version changes instead of polling?

The idea is to only poll the server when we know there are changes.

balegas · 2023-12-21T12:15:29Z

examples/node-sidecar/sidecar/src/ipc/server.ts

+  /**
+   * Notifies clients of actual data changes.
+   */
+  notifyDataChanged(): Promise<void>


Leave a TODO here that there are more events missing. e.g. connectivity state change

examples/node-sidecar/clients/node/src/auth.ts

balegas · 2023-12-21T12:16:56Z

examples/node-sidecar/clients/node/src/ipc/client.ts

I think client and server interfaces should live on a common location

How would you do that? The server IPC interface must be provided by the sidecar, the client IPC interface must be provided by the application. Those are two separate packages. The only way i can envision this is by moving the client and server IPC interfaces to a package of its own that would then be used by the sidecar and the application. So then we would have:

a package for the client and server IPC interfaces and their implementations

the sidecar package that uses the server-side components of the IPC package

the application that uses the client-side components of the IPC package

Let me know if this is what you want. We will need to think however how we will achieve this. Are we going to publish this IPC package to npmjs? Then it would probably also make sense to publish the sidecar.

Another possibility would be to define the IPC client and server interfaces and their implementations in the sidecar package and have the NodeJS client application use the sidecar as a dependency. Again, we need to think how we will make the sidecar available to those apps, it will probably need to be published.

balegas · 2023-12-21T12:18:45Z

examples/node-sidecar/sidecar/src/ipc/server.ts

I think client and server interfaces should live on a common location.

balegas · 2023-12-21T12:37:46Z

examples/node-sidecar/clients/node/src/index.ts

I think this organisation is conflating two concepts: the client of the sidecar and the code of the app. I think what we're calling clients/node is in fact an example that provides a client socket implementation and an example app. I would extract the application code to a separate folder (keeping the structure of different languages).

This is a bit of personal taste so I won't fight much about it. What I really really want is the following:

The developer runs a single yarn start command that runs a single file that starts the sidecar and the app. Users could manage the two things separately if they want, but I want to convey the pattern of starting the two things together. In other languages people would use shell or something to start the sidecar as a child process of the app.

Passing a port should be optional. In our case, since we're initialise the sidecar, we can get a random port from the OS and pass it to the app on the spot. Minimal static things.

About conflating the two: every application that wants to use the sidecar needs to be a client of the sidecar (i.e. use some IPC mechanism). I've put the IPC client inside the app. Do you want to make the IPC client a separate nodejs package that nodejs applications can use?

As i said in another comment, i'm not sure we want to provide a single start command for 2 reasons:

Users need to understand that the example consists of 2 parts: the sidecar and the application that connects to it. So imo it's better to make that explicit by having to run them separately.

Providing a single start command makes it more difficult for the user to run the sidecar example with several instances of the client application.
Whereas, with 2 separates commands it is immediately clear that they just need to run the sidecar once, and then as many application instances as they want.

Regarding the random port, sure we can do this but we're just increasing the complexity of the example a bit more. The minimal thing would be to make the port configurable.

balegas · 2023-12-21T12:58:31Z

clients/typescript/src/sockets/node.ts

@@ -23,7 +23,7 @@ export class WebSocketNode extends EventEmitter implements Socket {

    this.socket.on('open', () => this.emit('open'))
    this.socket.on('message', (data) => this.emit('message', data))
-    this.socket.on('error', (_unusedError) =>
+    this.socket.on('error', (_unusedError) => 


you don't have to commit this file

kevin-dp · 2023-12-21T14:20:44Z

The instructions must be simple for the sidecar and right now they aren't. I have a bit of a strong opinion that the sidecar should start along the app that uses it. I'm open to discussion if you think otherwise, but I think that is the ideal experience for someone playing with this.

Not sure. You only want 1 sidecar and potentially several applications. So if you run 2 applications you don't want the sidecar to be started twice. But we can discuss this :-)

balegas · 2023-12-22T09:51:12Z

I’m considering all comments have been addressed during our call. I’ll get back on the next push

… top-level readme

kevin-dp · 2023-12-22T15:30:20Z

@balegas I applied the changes we discussed and introduced a configuration file as originally proposed by @samwillis. Note the top-level readme and the top-level run.js file which should also work on windows (untested).

The sidecar still requires a few changes to the ts-client. Therefore, you will need to pnpm build && npm pack the client and modify the sidecar's electric-sql dependency to point to the packed file. I've already put the electric-sql dependency in the sidecar to 0.8.3 which anticipates the next patch release which should contain the necessary changes for the sidecar to work.

There are 2 subtleties still:

The app works on top of a db file and uses the configured token. If we keep the same token and the db file but we shut down the backend (including the volumes), then we can't restart the app because the backend will be started in a fresh state but the local database is already in a later state. So when that happens, the local db file must be deleted. We don't do that automatically because we could be erasing the user's content.
When the user quits the application (by inputting the "quit" command), we kill the process in which the sidecar is running. But it seems that node does not actually kill the process... Because of that the port on which the TCP server is listening is not freed and so next time we run the app it complains that the port is taken. We don't seem to be the only one experiencing the issue where a process is not killed. I've tried several variations but none worked.

balegas

Great work with making sidecar work with only public interfaces.
Thanks for removing the infra code from run.js
README.md should now have the instructions to launch the infrastructure for the readme to be self contained (note that we also need to cd into folders to install deps)

I think after that it's all done. I need to be able to run the project just by following instructions in top-level README.md

kevin-dp · 2023-12-28T08:25:07Z

Great work with making sidecar work with only public interfaces.

Thanks for removing the infra code from run.js

README.md should now have the instructions to launch the infrastructure for the readme to be self contained (note that we also need to cd into folders to install deps)

I think after that it's all done. I need to be able to run the project just by following instructions in top-level README.md

Updated readme with instructions for running the infrastructure.

kevin-dp · 2023-12-28T08:28:28Z

@samwillis may want to have a look at how the sidecar and the example application are built.
The application is using plain tsc to compile to JS and the compile files are ran with NodeJS but i had to make the utility scripts cjs modules.

samwillis · 2023-12-28T10:01:44Z

@kevin-dp, all looks good to me, vanilla tsc is perfect for building a node app. We could rewrite the util scripts as ESM but there is little point as once #664 is merged we can delete them.

If you wanted to, the top level run.js could be an ESM with a .mjs extension as it's not covered by a node package.json with "type": "module".

balegas

I quite felt the tension of running every single command in Windows, but everything worked!

I added a few more details to the README to incentivize exploration.
I think the commands for running the backend should live on the sidecar side, because things are tied to the JS ecosystem and people on other envs, would not necessarily have the package.json commands, or would have to port them.... there is maybe things to improve here, but I'll not obsess with that. I think the instructions are good and we can discuss with people
We should raise the attention of people from the community to this example and incentivise them to contribute

It's done and looks great. Thank you

kevin-dp added 2 commits December 20, 2023 15:16

Changes to the ts-client to expose necessary utility functions to the…

3b19c35

… sidecar.

Sidecar and example application.

85d0eba

balegas requested changes Dec 20, 2023

View reviewed changes

balegas requested changes Dec 21, 2023

View reviewed changes

Part 1 of addressing Valter's review.

b3ec4ea

kevin-dp added 2 commits December 22, 2023 16:18

Read from config file and other improvements.

f016e66

Minimal changes to the typescript client

c43259d

kevin-dp force-pushed the kevindp/vax-1427-sidecar branch from 13eec85 to c43259d Compare December 22, 2023 15:19

Removed readme of sidecar and application as they got replaced by the…

130fc97

… top-level readme

kevin-dp requested a review from balegas December 22, 2023 15:30

kevin-dp added 2 commits December 27, 2023 16:07

Changed sidecar in order not to require changes to the ts-client.

981f121

Modified run.js file to only run sidecar and app but not infrastructure.

1ad9613

kevin-dp force-pushed the kevindp/vax-1427-sidecar branch from 8638018 to 1ad9613 Compare December 27, 2023 15:08

Added blank line

e4df487

balegas requested changes Dec 27, 2023

View reviewed changes

Updated readme with instructions for running the infrastructure.

a27d938

kevin-dp requested a review from balegas December 28, 2023 08:24

added a bit more detail to the readme

581b429

balegas approved these changes Dec 29, 2023

View reviewed changes

balegas marked this pull request as ready for review December 29, 2023 11:34

removed empty line

b31d2fd

balegas force-pushed the kevindp/vax-1427-sidecar branch from aa57b7e to b31d2fd Compare December 29, 2023 11:37

balegas merged commit cc2b8df into main Dec 29, 2023
2 checks passed

balegas deleted the kevindp/vax-1427-sidecar branch December 29, 2023 11:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

example (sidecar): NodeJS sidecar example #772

example (sidecar): NodeJS sidecar example #772

kevin-dp commented Dec 20, 2023 •

edited

linear bot commented Dec 20, 2023

balegas left a comment

balegas Dec 20, 2023

balegas Dec 20, 2023

balegas Dec 20, 2023

kevin-dp Dec 20, 2023

balegas Dec 21, 2023

kevin-dp Dec 22, 2023

balegas left a comment

balegas Dec 21, 2023

balegas Dec 21, 2023

balegas Dec 21, 2023

balegas Dec 21, 2023

kevin-dp Dec 22, 2023

balegas Dec 21, 2023

balegas Dec 21, 2023 •

edited

kevin-dp Dec 22, 2023 •

edited

balegas Dec 21, 2023

kevin-dp commented Dec 21, 2023

balegas commented Dec 22, 2023

kevin-dp commented Dec 22, 2023

balegas left a comment

kevin-dp commented Dec 28, 2023

kevin-dp commented Dec 28, 2023

samwillis commented Dec 28, 2023

balegas left a comment

example (sidecar): NodeJS sidecar example #772

example (sidecar): NodeJS sidecar example #772

Conversation

kevin-dp commented Dec 20, 2023 • edited

linear bot commented Dec 20, 2023

balegas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balegas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balegas Dec 21, 2023 • edited

Choose a reason for hiding this comment

kevin-dp Dec 22, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevin-dp commented Dec 21, 2023

balegas commented Dec 22, 2023

kevin-dp commented Dec 22, 2023

balegas left a comment

Choose a reason for hiding this comment

kevin-dp commented Dec 28, 2023

kevin-dp commented Dec 28, 2023

samwillis commented Dec 28, 2023

balegas left a comment

Choose a reason for hiding this comment

kevin-dp commented Dec 20, 2023 •

edited

balegas Dec 21, 2023 •

edited

kevin-dp Dec 22, 2023 •

edited