From b12b3b5dd6dbfafaaf2f72396727ffa83e9d3d41 Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Tue, 1 Sep 2026 21:53:14 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(frontend)=20save=20the=20doc=20wit?= =?UTF-8?q?h=20a=20keepalive=20request=20when=20leaving=20the=20page?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since we fix the error in the backend application coming from middleware not managing async request lifecycle, the e2e tests were failing. This is because the browser cancel the request when made in the beforeunload event. To fix it, we set the keepalive property on the fetch method to True to not abort the request when the page is unloaded. --- CHANGELOG.md | 1 + .../hook/__tests__/useSaveDoc.test.tsx | 105 +++++++++++++++++- .../docs/doc-editor/hook/useSaveDoc.tsx | 57 +++++++--- .../api/useDocContentUpdate.tsx | 35 +++++- 4 files changed, 176 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d0a3adb0..9979420b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ and this project adheres to - 🐛(backend) fix duplicating a document that has no content - 📄(frontend) allowed partially export when MIT #2551 - 🐛(backend) manage async support for Docs custom middleware +- 🐛(frontend) save the doc with a keepalive request when leaving the page ### Removed diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/hook/__tests__/useSaveDoc.test.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/hook/__tests__/useSaveDoc.test.tsx index 8ed670d6b..8b055d9a6 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/hook/__tests__/useSaveDoc.test.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/hook/__tests__/useSaveDoc.test.tsx @@ -1,7 +1,7 @@ import { act, renderHook, waitFor } from '@testing-library/react'; import fetchMock from 'fetch-mock'; import { useRouter } from 'next/router'; -import { Mock, beforeEach, describe, expect, it, vi } from 'vitest'; +import { Mock, afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import * as Y from 'yjs'; import { AppWrapper } from '@/tests/utils'; @@ -38,6 +38,10 @@ describe('useSaveDoc', () => { }); }); + afterEach(() => { + vi.restoreAllMocks(); + }); + it('should setup event listeners on mount', () => { const yDoc = new Y.Doc(); const docId = 'test-doc-id'; @@ -129,6 +133,105 @@ describe('useSaveDoc', () => { vi.useRealTimers(); }); + const setupSavedDoc = async (yDoc: Y.Doc, docId: string) => { + fetchMock.patch(`http://test.jest/api/v1.0/documents/${docId}/content/`, { + body: JSON.stringify({ id: docId, content: 'test-content' }), + }); + + renderHook(() => useSaveDoc(docId, yDoc), { + wrapper: AppWrapper, + }); + + act(() => { + // Trigger a local update so there is something to save + yDoc.getMap('test').set('key', 'value'); + }); + }; + + const dispatchBeforeUnload = () => { + const event = new Event('beforeunload', { cancelable: true }); + act(() => { + window.dispatchEvent(event); + }); + return event; + }; + + it('should save with keepalive when the page is unloading', async () => { + const yDoc = new Y.Doc(); + const docId = self.crypto.randomUUID(); + + await setupSavedDoc(yDoc, docId); + + const event = dispatchBeforeUnload(); + + await waitFor(() => { + expect(fetchMock.callHistory.lastCall()?.url).toBe( + `http://test.jest/api/v1.0/documents/${docId}/content/`, + ); + }); + + expect(fetchMock.callHistory.lastCall()?.options.keepalive).toBe(true); + // The browser owns the request, no need to hold the unload back + expect(event.defaultPrevented).toBe(false); + }); + + it('should not use keepalive when saving without unloading', async () => { + vi.useFakeTimers(); + const yDoc = new Y.Doc(); + const docId = self.crypto.randomUUID(); + + await setupSavedDoc(yDoc, docId); + + act(() => { + vi.advanceTimersByTime(61000); + }); + + vi.useRealTimers(); + + await waitFor(() => { + expect(fetchMock.callHistory.lastCall()?.url).toBe( + `http://test.jest/api/v1.0/documents/${docId}/content/`, + ); + }); + + expect(fetchMock.callHistory.lastCall()?.options.keepalive).toBeFalsy(); + }); + + it('should hold the unload back when the doc is too big for keepalive with firefox', async () => { + const yDoc = new Y.Doc(); + const docId = self.crypto.randomUUID(); + + // Mock Firefox user agent to simulate Firefox behavior + vi.spyOn(navigator, 'userAgent', 'get').mockReturnValue( + 'Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/117.0', + ); + + fetchMock.patch(`http://test.jest/api/v1.0/documents/${docId}/content/`, { + body: JSON.stringify({ id: docId, content: 'test-content' }), + }); + + renderHook(() => useSaveDoc(docId, yDoc), { + wrapper: AppWrapper, + }); + + act(() => { + // Over the 64 KiB keepalive cap once base64 encoded + yDoc.getText('big').insert(0, 'a'.repeat(70 * 1024)); + }); + + const event = dispatchBeforeUnload(); + + await waitFor(() => { + expect(fetchMock.callHistory.lastCall()?.url).toBe( + `http://test.jest/api/v1.0/documents/${docId}/content/`, + ); + }); + + expect(fetchMock.callHistory.lastCall()?.options.keepalive).toBe(false); + // Regular fetch: the unload is held back so the request has time to go out + expect(event.defaultPrevented).toBe(true); + }); + it('should cleanup event listeners on unmount', () => { const yDoc = new Y.Doc(); const docId = 'test-doc-id'; diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/hook/useSaveDoc.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/hook/useSaveDoc.tsx index b6ceb0230..e0fbe7496 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/hook/useSaveDoc.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/hook/useSaveDoc.tsx @@ -2,7 +2,10 @@ import { useRouter } from 'next/router'; import { useCallback, useEffect, useRef, useState } from 'react'; import * as Y from 'yjs'; -import { useDocContentUpdate } from '@/docs/doc-management/api/useDocContentUpdate'; +import { + canKeepaliveContent, + useDocContentUpdate, +} from '@/docs/doc-management/api/useDocContentUpdate'; import { useProviderStore } from '@/docs/doc-management/stores/useProviderStore'; import { KEY_LIST_DOC_VERSIONS } from '@/docs/doc-versioning/api/useDocVersions'; import { COMMENT_UPDATE_ORIGIN } from '@/features/docs/doc-comments/api/DocsThreadStore'; @@ -86,26 +89,41 @@ export const useSaveDoc = (docId: string, yDoc: Y.Doc) => { }; }, [yDoc]); - const saveDoc = useCallback(() => { - if (!isLocalChange || isSavingRef.current) { - return false; - } + /** + * `isSaving` tells whether a request was actually sent, `isKeptAlive` + * whether it was handed over to the browser process (see `keepalive`) and + * will therefore outlive the page. + */ + const saveDoc = useCallback( + ({ isUnloading = false }: { isUnloading?: boolean } = {}) => { + if (!isLocalChange || isSavingRef.current) { + return { isSaving: false, isKeptAlive: false }; + } - isSavingRef.current = true; - updateDocContent({ - id: docId, - content: toBase64(Y.encodeStateAsUpdate(yDoc)), - websocket: isConnectedToCollabServer, - }); + isSavingRef.current = true; + const content = toBase64(Y.encodeStateAsUpdate(yDoc)); + const websocket = isConnectedToCollabServer; + updateDocContent({ + id: docId, + content, + websocket, + keepalive: isUnloading, + }); - return true; - }, [isLocalChange, updateDocContent, docId, yDoc, isConnectedToCollabServer]); + return { + isSaving: true, + isKeptAlive: isUnloading && canKeepaliveContent({ content, websocket }), + }; + }, + [isLocalChange, updateDocContent, docId, yDoc, isConnectedToCollabServer], + ); const router = useRouter(); useEffect(() => { const onSave = (e?: Event) => { - const isSaving = saveDoc(); + const isUnloading = typeof e !== 'undefined' && e.type === 'beforeunload'; + const { isSaving, isKeptAlive } = saveDoc({ isUnloading }); /** * Firefox does not trigger the request every time the user leaves the page. @@ -113,19 +131,24 @@ export const useSaveDoc = (docId: string, yDoc: Y.Doc) => { * So we prevent the default behavior to have the popup asking the user * if he wants to leave the page, by adding the popup, we let the time to the * request to be sent, and intercepted by the service worker (for the offline part). + * + * We do the same for documents too big to be sent with `keepalive`: the + * request is a regular fetch, so it dies with the page unless we hold + * the unload back. */ if ( isSaving && - typeof e !== 'undefined' && + isUnloading && e.preventDefault && - isFirefox() + isFirefox() && + !isKeptAlive ) { e.preventDefault(); } }; // Save every minute - const timeout = setInterval(onSave, SAVE_INTERVAL); + const timeout = setInterval(() => onSave(), SAVE_INTERVAL); // Save when the user leaves the page addEventListener('beforeunload', onSave); // Save when the user navigates to another page diff --git a/src/frontend/apps/impress/src/features/docs/doc-management/api/useDocContentUpdate.tsx b/src/frontend/apps/impress/src/features/docs/doc-management/api/useDocContentUpdate.tsx index 23cb7402e..0cb0e4576 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-management/api/useDocContentUpdate.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-management/api/useDocContentUpdate.tsx @@ -16,23 +16,50 @@ export interface UpdateDocContentParams { id: Doc['id']; content: string; // Base64 encoded content websocket?: boolean; + /** + * Hand the request over to the browser process so it survives the page + * being torn down. Needed when saving from "beforeunload": a regular fetch + * is aborted with the document, and the server never sees the request. + */ + keepalive?: boolean; } +/** + * The fetch spec caps the body of a keepalive request. Over that limit the + * browser rejects the request outright, so we fall back to a regular fetch. + */ +const KEEPALIVE_MAX_BODY_SIZE = 64 * 1024; + +const buildContentBody = ({ + content, + websocket, +}: Pick) => + JSON.stringify({ + content, + websocket, + }); + +/** Whether the doc is small enough to be saved with a keepalive request. */ +export const canKeepaliveContent = ( + params: Pick, +) => buildContentBody(params).length <= KEEPALIVE_MAX_BODY_SIZE; + export const updateDocContent = async ({ id, content, websocket, + keepalive, }: UpdateDocContentParams): Promise => { if (!uuidValidate(id)) { throw new Error(`Invalid doc id in updateDocContent: ${id}`); } + const body = buildContentBody({ content, websocket }); + const response = await fetchAPI(`documents/${id}/content/`, { method: 'PATCH', - body: JSON.stringify({ - content, - websocket, - }), + body, + keepalive: keepalive && body.length <= KEEPALIVE_MAX_BODY_SIZE, }); if (!response.ok) {