Skip to content

Commit 8cb92fa

Browse files
committed
⚡️(frontend) improve UploadFile process
We notices that `context.getChanges` was very greedy, on a large document with multiple users collaborating, it caused performance issues. We change the way that we track a upload by listening onUploadEnd event instead of tracking all changes in the document. When a doc opens, we check if there are any ongoing uploads and resume them. We fix as well a race condition that could happen when multiple collaborators were on a document during an upload.
1 parent 70d6d7e commit 8cb92fa

File tree

2 files changed

+93
-55
lines changed

2 files changed

+93
-55
lines changed

src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-blocks/UploadLoaderBlock.tsx

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ type UploadLoaderPropSchema = {
2828
readonly default: '';
2929
};
3030
readonly blockUploadUrl: { readonly default: '' };
31+
readonly isProcessing: { readonly default: false };
3132
};
3233

3334
type UploadLoaderBlockConfig = BlockConfig<
@@ -65,12 +66,27 @@ const UploadLoaderBlockComponent = ({
6566
const shouldCheckStatus =
6667
block.props.blockUploadUrl &&
6768
block.props.type === 'loading' &&
68-
isEditable;
69+
isEditable &&
70+
!block.props.isProcessing;
6971

7072
if (!shouldCheckStatus) {
7173
return;
7274
}
7375

76+
// Mark as processing to prevent other users from processing the same block
77+
try {
78+
editor.updateBlock(block.id, {
79+
type: 'uploadLoader',
80+
props: {
81+
...block.props,
82+
isProcessing: true,
83+
},
84+
});
85+
} catch {
86+
// Block was already updated or deleted by another user
87+
return;
88+
}
89+
7490
const url = block.props.blockUploadUrl;
7591

7692
loopCheckDocMediaStatus(url)
@@ -143,6 +159,7 @@ export const UploadLoaderBlock = createReactBlockSpec(
143159
default: '',
144160
},
145161
blockUploadUrl: { default: '' },
162+
isProcessing: { default: false },
146163
},
147164
content: 'none',
148165
},
Lines changed: 75 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { Block } from '@blocknote/core';
12
import { captureException } from '@sentry/nextjs';
23
import { useCallback, useEffect } from 'react';
34
import { useTranslation } from 'react-i18next';
@@ -36,73 +37,93 @@ export const useUploadFile = (docId: string) => {
3637
};
3738
};
3839

40+
/**
41+
* When we upload a file it can takes some time to analyze it (e.g. virus scan).
42+
* This hook listen to upload end and replace the uploaded block by a uploadLoader
43+
* block to show analyzing status.
44+
* The uploadLoader block will then handle the status display until the analysis is done
45+
* then replaced by the final block (e.g. image, pdf, etc.).
46+
* @param editor
47+
*/
3948
export const useUploadStatus = (editor: DocsBlockNoteEditor) => {
4049
const ANALYZE_URL = 'media-check';
4150
const { t } = useTranslation();
4251

43-
useEffect(() => {
44-
const unsubscribe = editor.onChange((_, context) => {
45-
const blocksChanges = context.getChanges();
46-
47-
if (!blocksChanges.length) {
48-
return;
49-
}
50-
51-
const blockChanges = blocksChanges[0];
52-
52+
/**
53+
* Replace the resource block by a uploadLoader block to show analyzing status
54+
*/
55+
const replaceBlockWithUploadLoader = useCallback(
56+
(block: Block) => {
5357
if (
54-
blockChanges.source.type !== 'local' ||
55-
blockChanges.type !== 'update' ||
56-
!('url' in blockChanges.block.props) ||
57-
('url' in blockChanges.block.props &&
58-
!blockChanges.block.props.url.includes(ANALYZE_URL))
58+
!block ||
59+
!('url' in block.props) ||
60+
('url' in block.props && !block.props.url.includes(ANALYZE_URL))
5961
) {
6062
return;
6163
}
6264

63-
const blockUploadUrl = blockChanges.block.props.url;
64-
const blockUploadType = blockChanges.block.type;
65-
const blockUploadName = blockChanges.block.props.name;
65+
const blockUploadUrl = block.props.url;
66+
const blockUploadType = block.type;
67+
const blockUploadName = block.props.name;
6668
const blockUploadShowPreview =
67-
('showPreview' in blockChanges.block.props &&
68-
blockChanges.block.props.showPreview) ||
69-
false;
70-
71-
const timeoutId = setTimeout(() => {
72-
// Replace the resource block by a uploadLoader block
73-
// to show analyzing status
74-
try {
75-
editor.replaceBlocks(
76-
[blockChanges.block.id],
77-
[
78-
{
79-
type: 'uploadLoader',
80-
props: {
81-
information: t('Analyzing file...'),
82-
type: 'loading',
83-
blockUploadName,
84-
blockUploadType,
85-
blockUploadUrl,
86-
blockUploadShowPreview,
87-
},
69+
('showPreview' in block.props && block.props.showPreview) || false;
70+
71+
try {
72+
editor.replaceBlocks(
73+
[block.id],
74+
[
75+
{
76+
type: 'uploadLoader',
77+
props: {
78+
information: t('Analyzing file...'),
79+
type: 'loading',
80+
blockUploadName,
81+
blockUploadType,
82+
blockUploadUrl,
83+
blockUploadShowPreview,
8884
},
89-
],
90-
);
91-
} catch (error) {
92-
captureException(error, {
93-
extra: { info: 'Error replacing block for upload loader' },
94-
});
95-
}
96-
}, 250);
85+
},
86+
],
87+
);
88+
} catch (error) {
89+
captureException(error, {
90+
extra: { info: 'Error replacing block for upload loader' },
91+
});
92+
}
93+
},
94+
[editor, t],
95+
);
96+
97+
useEffect(() => {
98+
const imagesBlocks = editor?.document.filter(
99+
(block) =>
100+
block.type === 'image' && block.props.url.includes(ANALYZE_URL),
101+
);
102+
103+
imagesBlocks.forEach((block) => {
104+
replaceBlockWithUploadLoader(block as Block);
105+
});
106+
}, [editor, replaceBlockWithUploadLoader]);
107+
108+
/**
109+
* Handle upload end to replace the upload block by a uploadLoader
110+
* block to show analyzing status
111+
*/
112+
useEffect(() => {
113+
editor.onUploadEnd((blockId) => {
114+
if (!blockId) {
115+
return;
116+
}
117+
118+
const innerTimeoutId = setTimeout(() => {
119+
const block = editor.getBlock({ id: blockId });
120+
121+
replaceBlockWithUploadLoader(block as Block);
122+
}, 300);
97123

98124
return () => {
99-
clearTimeout(timeoutId);
100-
unsubscribe();
125+
clearTimeout(innerTimeoutId);
101126
};
102127
});
103-
104-
return () => {
105-
unsubscribe();
106-
};
107-
}, [editor, t]);
128+
}, [editor, replaceBlockWithUploadLoader]);
108129
};

0 commit comments

Comments
 (0)