Skip to content

♻️Collaboration process #528

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

Merged
merged 4 commits into from
Dec 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ and this project adheres to

🔧(helm) add option to disable default tls setting by @dominikkaminski #519

## Changed

- 🏗️(yjs-server) organize yjs server #528
- ♻️(frontend) better separation collaboration process #528


## [1.10.0] - 2024-12-17

Expand Down
2 changes: 1 addition & 1 deletion src/frontend/apps/e2e/__tests__/app-impress/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export const createDoc = async (
.click();

await page.getByRole('heading', { name: 'Untitled document' }).click();
await page.keyboard.type(randomDocs[i]);
await page.keyboard.type(randomDocs[i], { delay: 100 });
await page.getByText('Created at ').click();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { DocHeader } from '@/features/docs/doc-header';
import {
Doc,
base64ToBlocknoteXmlFragment,
useDocStore,
useProviderStore,
} from '@/features/docs/doc-management';
import { Versions, useDocVersion } from '@/features/docs/doc-versioning/';
import { useResponsiveStore } from '@/stores';
Expand All @@ -33,8 +33,7 @@ export const DocEditor = ({ doc }: DocEditorProps) => {

const { colorsTokens } = useCunninghamTheme();

const { providers } = useDocStore();
const provider = providers?.[doc.id];
const { provider } = useProviderStore();

if (!provider) {
return null;
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from './useCollaboration';
export * from './useTrans';
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { useEffect } from 'react';

import { useCollaborationUrl } from '@/core/config';
import { useBroadcastStore } from '@/stores';

import { useProviderStore } from '../stores/useProviderStore';
import { Base64 } from '../types';

export const useCollaboration = (room?: string, initialContent?: Base64) => {
const collaborationUrl = useCollaborationUrl(room);
const { setBroadcastProvider } = useBroadcastStore();
const { provider, createProvider, destroyProvider } = useProviderStore();

useEffect(() => {
if (!room || !collaborationUrl || provider) {
return;
}

const newProvider = createProvider(collaborationUrl, room, initialContent);
setBroadcastProvider(newProvider);
}, [
provider,
collaborationUrl,
room,
initialContent,
createProvider,
setBroadcastProvider,
]);

useEffect(() => {
return () => {
destroyProvider();
};
}, [destroyProvider]);
};
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from './useDocStore';
export * from './useProviderStore';
Original file line number Diff line number Diff line change
@@ -1,53 +1,14 @@
import { HocuspocusProvider } from '@hocuspocus/provider';
import * as Y from 'yjs';
import { create } from 'zustand';

import { Base64, Doc } from '@/features/docs/doc-management';
import { Doc } from '@/features/docs/doc-management';

export interface UseDocStore {
currentDoc?: Doc;
providers: {
[storeId: string]: HocuspocusProvider;
};
createProvider: (
providerUrl: string,
storeId: string,
initialDoc: Base64,
) => HocuspocusProvider;
setProviders: (storeId: string, providers: HocuspocusProvider) => void;
setCurrentDoc: (doc: Doc | undefined) => void;
}

export const useDocStore = create<UseDocStore>((set, get) => ({
export const useDocStore = create<UseDocStore>((set) => ({
currentDoc: undefined,
providers: {},
createProvider: (providerUrl, storeId, initialDoc) => {
const doc = new Y.Doc({
guid: storeId,
});

if (initialDoc) {
Y.applyUpdate(doc, Buffer.from(initialDoc, 'base64'));
}

const provider = new HocuspocusProvider({
url: providerUrl,
name: storeId,
document: doc,
});

get().setProviders(storeId, provider);

return provider;
},
setProviders: (storeId, provider) => {
set(({ providers }) => ({
providers: {
...providers,
[storeId]: provider,
},
}));
},
setCurrentDoc: (doc) => {
set({ currentDoc: doc });
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { HocuspocusProvider } from '@hocuspocus/provider';
import * as Y from 'yjs';
import { create } from 'zustand';

import { Base64 } from '@/features/docs/doc-management';

export interface UseCollaborationStore {
createProvider: (
providerUrl: string,
storeId: string,
initialDoc?: Base64,
) => HocuspocusProvider;
destroyProvider: () => void;
provider: HocuspocusProvider | undefined;
}

const defaultValues = {
provider: undefined,
};

export const useProviderStore = create<UseCollaborationStore>((set, get) => ({
...defaultValues,
createProvider: (wsUrl, storeId, initialDoc) => {
const doc = new Y.Doc({
guid: storeId,
});

if (initialDoc) {
Y.applyUpdate(doc, Buffer.from(initialDoc, 'base64'));
}

const provider = new HocuspocusProvider({
url: wsUrl,
name: storeId,
document: doc,
});

set({
provider,
});

return provider;
},
destroyProvider: () => {
const provider = get().provider;
if (provider) {
provider.destroy();
}

set(defaultValues);
},
}));
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import { Box, Text } from '@/components';
import {
Doc,
base64ToYDoc,
useDocStore,
useProviderStore,
useUpdateDoc,
} from '@/features/docs/doc-management';
} from '@/features/docs/doc-management/';

import { useDocVersion } from '../api';
import { KEY_LIST_DOC_VERSIONS } from '../api/useDocVersions';
Expand All @@ -40,7 +40,7 @@ export const ModalVersion = ({
const { t } = useTranslation();
const { toast } = useToastProvider();
const { push } = useRouter();
const { providers } = useDocStore();
const { provider } = useProviderStore();
const { mutate: updateDoc } = useUpdateDoc({
listInvalideQueries: [KEY_LIST_DOC_VERSIONS],
onSuccess: () => {
Expand All @@ -49,14 +49,14 @@ export const ModalVersion = ({
void push(`/docs/${docId}`);
};

if (!providers?.[docId] || !version?.content) {
if (!provider || !version?.content) {
onDisplaySuccess();
return;
}

revertUpdate(
providers[docId].document,
providers[docId].document,
provider.document,
provider.document,
base64ToYDoc(version.content),
);

Expand Down
49 changes: 26 additions & 23 deletions src/frontend/apps/impress/src/pages/docs/[id]/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,15 @@ import { useEffect, useState } from 'react';

import { Box, Text } from '@/components';
import { TextErrors } from '@/components/TextErrors';
import { useCollaborationUrl } from '@/core';
import { useAuthStore } from '@/core/auth';
import { DocEditor } from '@/features/docs/doc-editor';
import { KEY_DOC, useDoc, useDocStore } from '@/features/docs/doc-management';
import {
Doc,
KEY_DOC,
useCollaboration,
useDoc,
useDocStore,
} from '@/features/docs/doc-management/';
import { MainLayout } from '@/layouts';
import { useBroadcastStore } from '@/stores';
import { NextPageWithLayout } from '@/types/next';
Expand Down Expand Up @@ -41,14 +46,25 @@ interface DocProps {

const DocPage = ({ id }: DocProps) => {
const { login } = useAuthStore();
const { data: docQuery, isError, error } = useDoc({ id });
const [doc, setDoc] = useState(docQuery);
const { setCurrentDoc, createProvider, providers } = useDocStore();
const { setBroadcastProvider, addTask } = useBroadcastStore();
const {
data: docQuery,
isError,
isFetching,
error,
} = useDoc(
{ id },
{
staleTime: 0,
queryKey: [KEY_DOC, { id }],
},
);

const [doc, setDoc] = useState<Doc>();
const { setCurrentDoc } = useDocStore();
const { addTask } = useBroadcastStore();
const queryClient = useQueryClient();
const { replace } = useRouter();
const provider = providers?.[id];
const collaborationUrl = useCollaborationUrl(doc?.id);
useCollaboration(doc?.id, doc?.content);

useEffect(() => {
if (doc?.title) {
Expand All @@ -59,26 +75,13 @@ const DocPage = ({ id }: DocProps) => {
}, [doc?.title]);

useEffect(() => {
if (!docQuery) {
if (!docQuery || isFetching) {
return;
}

setDoc(docQuery);
setCurrentDoc(docQuery);
}, [docQuery, setCurrentDoc]);

useEffect(() => {
if (!doc?.id || !collaborationUrl) {
return;
}

let newProvider = provider;
if (!provider || provider.document.guid !== doc.id) {
newProvider = createProvider(collaborationUrl, doc.id, doc.content);
}

setBroadcastProvider(newProvider);
}, [createProvider, doc, provider, setBroadcastProvider, collaborationUrl]);
}, [docQuery, setCurrentDoc, isFetching]);

/**
* We add a broadcast task to reset the query cache
Expand Down
50 changes: 42 additions & 8 deletions src/frontend/apps/impress/src/stores/useBroadcastStore.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,20 @@ interface BroadcastState {
getBroadcastProvider: () => HocuspocusProvider | undefined;
provider?: HocuspocusProvider;
setBroadcastProvider: (provider: HocuspocusProvider) => void;
tasks: { [taskLabel: string]: Y.Array<string> };
setTask: (
taskLabel: string,
task: Y.Array<string>,
action: () => void,
) => void;
tasks: {
[taskLabel: string]: {
task: Y.Array<string>;
observer: (
event: Y.YArrayEvent<string>,
transaction: Y.Transaction,
) => void;
};
};
}

export const useBroadcastStore = create<BroadcastState>((set, get) => ({
Expand All @@ -25,26 +38,47 @@ export const useBroadcastStore = create<BroadcastState>((set, get) => ({
return provider;
},
addTask: (taskLabel, action) => {
const taskExistAlready = get().tasks[taskLabel];
const provider = get().getBroadcastProvider();
if (taskExistAlready || !provider) {
if (!provider) {
return;
}

const existingTask = get().tasks[taskLabel];
if (existingTask) {
existingTask.task.unobserve(existingTask.observer);
get().setTask(taskLabel, existingTask.task, action);
return;
}

const task = provider.document.getArray<string>(taskLabel);
task.observe(() => {
action();
});
get().setTask(taskLabel, task, action);
},
setTask: (taskLabel: string, task: Y.Array<string>, action: () => void) => {
let isInitializing = true;
const observer = () => {
if (!isInitializing) {
action();
}
};

task.observe(observer);

setTimeout(() => {
isInitializing = false;
}, 1000);
Comment on lines +66 to +68
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


set((state) => ({
tasks: {
...state.tasks,
[taskLabel]: task,
[taskLabel]: {
task,
observer,
},
},
}));
},
broadcast: (taskLabel) => {
const task = get().tasks[taskLabel];
const { task } = get().tasks[taskLabel];
if (!task) {
console.warn(`Task ${taskLabel} is not defined`);
return;
Expand Down
Loading
Loading