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

♻️Collaboration process #528

Merged
merged 4 commits into from
Dec 24, 2024
Merged

♻️Collaboration process #528

merged 4 commits into from
Dec 24, 2024

Conversation

AntoLC
Copy link
Collaborator

@AntoLC AntoLC commented Dec 23, 2024

Purpose

It is the first step to introduce the long polling feature (see: #517)

We improve the architecture of the collaboration server, it will help us to introduce new routes without impacting the other routes.
We create useProviderStore on the frontend side, it will be dedicated to the provider.
We add as well useCollaboration hook, that will manage the collaboration connections.

Proposal

  • 🏗️(y-provider) organize yjs server
  • 🔒️(y-provider) add cors middlewares
  • ♻️(frontend) update already existing tasks
  • ♻️(frontend) create useProviderStore

@AntoLC AntoLC self-assigned this Dec 23, 2024
@AntoLC AntoLC force-pushed the refacto/collaboration-process branch 3 times, most recently from f416aa7 to 69c2c91 Compare December 23, 2024 15:54
Many routes were in the server.ts file, now they
are in their own files in the handlers folder.
The server.ts file is now AppServer that handles
the routes.
We split as well the tests.
Add cors middlewares to y-provider server.
It will control how clients connect to the server
with http requests.
When a task was already existing, we were not
updating it. This commit fixes that.
We created useProviderStore, a store dedicated
to managing the provider of the document.
We created as well a new hook useCollaboration,
it will be use to interact with the provider store.
This refacto is a first step to implement
the long polling.
@AntoLC AntoLC force-pushed the refacto/collaboration-process branch from 69c2c91 to 904fd05 Compare December 23, 2024 15:56
Comment on lines +66 to +68
setTimeout(() => {
isInitializing = false;
}, 1000);
Copy link
Contributor

Choose a reason for hiding this comment

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

why ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

task.observe triggers the task lightly after you bind it to it, it seems like a bug, it is due to the architecture of y.js apparently. We want to exec the task only when we trigger it from broadcast method, so isInitializing is a trick to blocked this initial trigger.

https://github.com/numerique-gouv/impress/blob/375418df5438f36f48e8de21697aa650310137c2/src/frontend/apps/impress/src/stores/useBroadcastStore.tsx#L57-L68

@AntoLC AntoLC merged commit 02a4740 into main Dec 24, 2024
15 of 16 checks passed
@AntoLC AntoLC deleted the refacto/collaboration-process branch December 24, 2024 11:29
@AntoLC AntoLC mentioned this pull request Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants