ether / etherpad-next

Rewrite of Etherpad using NextJS
Apache License 2.0
7 stars 2 forks source link

test(yjs): Spike #84

Open JannikStreek opened 3 months ago

JannikStreek commented 3 months ago

closes #68

This is not to be considered production ready code or anything to merge. This is supposed to be an example on how yjs works.

Learnings:

Bildschirm­foto 2024-04-02 um 20 43 28

Instructions

You need redis, Postgres and the branch => Better use my docker compose setup.

docker compose up -d
docker compose exec app npm run dev

EDIT: and please update the .env file, some new additions there.

JannikStreek commented 3 months ago

@SamTV12345 @AugustinMauroy feel free to test

AugustinMauroy commented 3 months ago

It's possible to bind the editor with our custom tools bar ? Or we should have our custom style on the editor

JannikStreek commented 3 months ago

It's possible to bind the editor with our custom tools bar ?

you have to write your own binding. yjs comes with bindings for the popular editors, if you want to build a custom one, you need your own bindings.

AugustinMauroy commented 3 months ago

you have to write your own binding. yjs comes with bindings for the popular editors, if you want to build a custom one, you need your own bindings.

Humm very interesting. I would have liked to see our publisher. But you have to look at the development/maintenance costs compared to the flexibility it brings. Also, if we made our own, the security holes would be with us.

AugustinMauroy commented 3 months ago

Actually some people just answered my question:/api/rooms/[id].js would be the way to go, haven't tried it yet with the next-ws stuff.

I didn't 100% catch you issue but. if you do something like that /api/rooms/[id]/route.ts next will compile

next docs about route handler

JannikStreek commented 3 months ago

Actually some people just answered my question:/api/rooms/[id].js would be the way to go, haven't tried it yet with the next-ws stuff.

I didn't 100% catch you issue but. if you do something like that /api/rooms/[id]/route.ts next will compile

yes, would also be possible. would leave the logic when to store the whole document and when the diff to us. We could choose if we want to go the redis or the custom backend path...both possible I guess. redis is more complicated as setup but the algorithm is already there.

AugustinMauroy commented 3 months ago

yes, would also be possible. would leave the logic when to store the whole document and when the diff to us. We could choose if we want to go the redis or the custom backend path...both possible I guess. redis is more complicated as setup but the algorithm is already there.

It's good that you said it again. But if the instance doesn't use docker (for example a simple VPS) this will add complexity to the setup. How is this currently done? And again, what is the maintenance cost? Because a custom algo isn't death. Also, from a performance point of view, you have to look at whether you're running a full redis or just running a bit of JS when you need it (normally nextjs handles this well).

JannikStreek commented 3 months ago

yes, would also be possible. would leave the logic when to store the whole document and when the diff to us. We could choose if we want to go the redis or the custom backend path...both possible I guess. redis is more complicated as setup but the algorithm is already there.

It's good that you said it again. But if the instance doesn't use docker (for example a simple VPS) this will add complexity to the setup. How is this currently done? And again, what is the maintenance cost? Because a custom algo isn't death. Also, from a performance point of view, you have to look at whether you're running a full redis or just running a bit of JS when you need it (normally nextjs handles this well).

there are no perfect solutions, just tradeoffs. Yes, on a simple vps it may be difficult, but most are also offering databases and redis. And even without, it's still possible. We will need to implement the web socket backend part ourselves which is currently handled by y-redis. As a reminder: the scope of this PR was not to offer a perfect solution. the scope of this pr was to get a feeling for yjs and see it in action. I am happy to try out another way of implementing yjs, but there is no silver bullet here.

Because a custom algo isn't death.

I strongly disagree here, even if it's not death. Merging conflicting updates in a reliable way is complex, time consuming to code, prone to errors and results in higher code complexity. And this is completely solved within yjs even with all the editor bindings. Why reinvent the wheel here if you get it for free?

AugustinMauroy commented 3 months ago

I strongly disagree here, even if it's not death. Merging conflicting updates in a reliable way is complex, time consuming to code, prone to errors and results in higher code complexity. And this is completely solved within yjs even with all the editor bindings. Why reinvent the wheel here if you get it for free?

I allow with you. But I still think adding redis on the project is to make it more cumbersome

JannikStreek commented 3 months ago

@AugustinMauroy I pushed a commit where I am trying to connect to an API endpoint in next.js api/room/room_id which is located here: api/room/[id]/route.ts. However, I still get the message:

[next-ws] could not find module for page /api/room/quill-demo-room-websocket-server

Please feel free to play with it. Not sure why it can't find the module, either I am doing something wrong or next-ws has problems supporting dynamic routes (https://nextjs.org/docs/pages/building-your-application/routing/dynamic-routes)? Unfortunately I don't have more time today.

JannikStreek commented 3 months ago

ok last update: the GET request is reaching the api endpoint, but not the web socket request...

AugustinMauroy commented 3 months ago

ok last update: the GET request is reaching the api endpoint, but not the web socket request...

I'll take a look it's shouldn't do that. EDIT:

  1. try to use next-socket as client such as app/socket-demo.
  2. There are a strange behavior, when you are on dev env sometime it's not compile socket route but on build it's work fine.
JannikStreek commented 3 months ago

ok last update: the GET request is reaching the api endpoint, but not the web socket request...

I'll take a look it's shouldn't do that. EDIT:

  1. try to use next-socket as client such as app/socket-demo.
  2. There are a strange behavior, when you are on dev env sometime it's not compile socket route but on build it's work fine.

well, this would make using the existing bindings more complicated (as you can't simply use the yjs web socket adapter) and I honestly do not really understand why the path resolutions isn't working as the correct route is called. But would be interesting to see if that actually works. But I wouldn't consider this for real use.

SamTV12345 commented 3 months ago

ok last update: the GET request is reaching the api endpoint, but not the web socket request...

I'll take a look it's shouldn't do that. EDIT:

  1. try to use next-socket as client such as app/socket-demo.
  2. There are a strange behavior, when you are on dev env sometime it's not compile socket route but on build it's work fine.

well, this would make using the existing bindings more complicated (as you can't simply use the yjs web socket adapter) and I honestly do not really understand why the path resolutions isn't working as the correct route is called. But would be interesting to see if that actually works. But I wouldn't consider this for real use.

The question is what would be the benefit of using next-socket over the yjs implementation.

JannikStreek commented 3 months ago

ok last update: the GET request is reaching the api endpoint, but not the web socket request...

I'll take a look it's shouldn't do that. EDIT:

  1. try to use next-socket as client such as app/socket-demo.
  2. There are a strange behavior, when you are on dev env sometime it's not compile socket route but on build it's work fine.

well, this would make using the existing bindings more complicated (as you can't simply use the yjs web socket adapter) and I honestly do not really understand why the path resolutions isn't working as the correct route is called. But would be interesting to see if that actually works. But I wouldn't consider this for real use.

The question is what would be the benefit of using next-socket over the yjs implementation.

i don't see any but maybe it has to be used so that the routes work properly, we have to test that. Currently I see the following options:

AugustinMauroy commented 3 months ago

We make the routes work, so that we can integrate the web socket backend into the next.js server. Currently stuck because the route is not picked up from web socket calls to next-ws, but normal http requests are. I agree that on first glance this seems to be the most straight forward solution, but we need to make next-ws work. => Help wanted

I'm more in favor of that. It's more custom things but it's reduce number of backend. We can ask help from maintainer of next-ws ?