From ae6e3a8eb3ca6977ac87d4dd410a20143bbbcf04 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Mon, 20 Jul 2026 21:44:46 +0200 Subject: [PATCH] feat(items): re-enable lazy loading with cursor example: if a folder has many resource, client will use lazy loading and load next cursor if scroll reached the bottom of the page purpose: reduce the amount of call to server --- .../lib/api/endpoints/folders.bench.test.ts | 221 ------------- frontend/src/lib/api/endpoints/folders.ts | 157 +++++---- .../src/routes/files/[...path]/+page.svelte | 303 +++++++++--------- frontend/src/routes/files/page.test.ts | 43 +-- 4 files changed, 265 insertions(+), 459 deletions(-) delete mode 100644 frontend/src/lib/api/endpoints/folders.bench.test.ts diff --git a/frontend/src/lib/api/endpoints/folders.bench.test.ts b/frontend/src/lib/api/endpoints/folders.bench.test.ts deleted file mode 100644 index d62372db..00000000 --- a/frontend/src/lib/api/endpoints/folders.bench.test.ts +++ /dev/null @@ -1,221 +0,0 @@ -import { describe, expect, it, vi, beforeEach } from 'vitest'; - -vi.mock('$lib/api/client', () => ({ apiFetch: vi.fn(), apiJson: vi.fn() })); - -import { apiFetch } from '$lib/api/client'; -import type { FileItem, FolderItem, ItemType } from '$lib/api/types'; -import { fetchFolderListing, invalidateFolderCache, type FolderListing } from './folders'; - -/** - * Benchmark gate for the coalesced progressive-render emissions in - * {@link fetchFolderListing}. - * - * Audit finding: the loader invoked `onPage` after EVERY 200-item page with a - * fresh copy of the whole accumulated listing, and the files view re-derives - * its filtered + sorted view (two `localeCompare` sorts + entry rebuild) from - * each emission. For a folder of N items that is Σ page sizes ≈ O(N²/200) - * elements re-sorted on the main thread during a single load — hundreds of ms - * of jank on exactly the large folders progressive rendering was meant to - * help. The fix emits page one (first paint) and the final page always, and - * intermediate pages at most once per PAGE_EMIT_MIN_INTERVAL_MS. - * - * Gates: - * 1. Equivalence — final listing identical to the emit-every-page reference, - * first emission still after page one (first paint preserved), last - * emission still `done === true` with the complete listing. - * 2. Perf — on a fast connection (pages resolve in ≪150 ms) the consumer-side - * derive work collapses from 25 full re-sorts to ≤3; wall time of the - * load+derive cycle must drop accordingly (≥3x on the derive term). - */ - -type ResourceItem = { resource_type: ItemType; resource: { id: string; name: string } }; -type ResourcePage = { items?: ResourceItem[]; next_cursor?: string }; - -const PAGE_SIZE = 200; -const PAGES = 25; // 5 000-item folder - -/** Deterministic shuffled names so the consumer sort actually works. */ -function pageBody(page: number): ResourcePage { - const items: ResourceItem[] = []; - for (let i = 0; i < PAGE_SIZE; i++) { - const n = page * PAGE_SIZE + i; - const id = `f-${n.toString().padStart(5, '0')}`; - // Mix folders into the first page like a real listing (folders first). - const isFolder = page === 0 && i < 20; - items.push({ - resource_type: isFolder ? 'folder' : 'file', - resource: { id, name: `item ${((n * 7919) % 100000).toString().padStart(5, '0')}.txt` } - }); - } - return { items, next_cursor: page + 1 < PAGES ? `c${page + 1}` : undefined }; -} - -function fakeRes(body: ResourcePage): Response { - return { - status: 200, - ok: true, - json: async () => body, - headers: { get: () => null } - } as unknown as Response; -} - -function mockPagedFetch(): void { - let call = 0; - vi.mocked(apiFetch).mockImplementation(async () => fakeRes(pageBody(call++))); -} - -/** - * The pre-fix loader, verbatim shape: accumulate pages and emit a fresh copy - * of the whole accumulated listing after every page. - */ -async function referenceFetchFolderListing( - folderId: string, - onPage: (partial: FolderListing, done: boolean) => void -): Promise { - const folders: FolderItem[] = []; - const files: FileItem[] = []; - let cursor: string | undefined; - do { - const params = new URLSearchParams({ order_by: 'name', limit: '200' }); - if (cursor) params.set('cursor', cursor); - const res = await apiFetch(`/api/folders/${folderId}/resources?${params.toString()}`, { - credentials: 'same-origin', - cache: 'no-store' - }); - if (!res.ok) throw new Error(`listing failed: ${res.status}`); - const page = (await res.json()) as ResourcePage; - for (const it of page.items ?? []) { - if (it.resource_type === 'folder') folders.push(it.resource as FolderItem); - else files.push(it.resource as FileItem); - } - cursor = page.next_cursor; - onPage({ folders: [...folders], files: [...files], favoriteIds: [], sharedIds: [] }, !cursor); - } while (cursor); - return { folders, files, favoriteIds: [], sharedIds: [] }; -} - -/** - * The files view's per-emission derive chain, reduced to its dominant costs: - * dotfile filter pass + two localeCompare sorts + ordered-entry rebuild - * (`sortedFolders`/`sortedFiles`/`entries`/`orderedIds` in +page.svelte). - * Returns the number of elements that went through the sort — the O(N²) term. - */ -function consumerDerive(partial: FolderListing): number { - const visF = partial.folders.filter((f) => !f.name.startsWith('.')); - const visX = partial.files.filter((f) => !f.name.startsWith('.')); - const sortedF = [...visF].sort((a, b) => a.name.localeCompare(b.name)); - const sortedX = [...visX].sort((a, b) => a.name.localeCompare(b.name)); - const orderedIds = [...sortedF.map((f) => f.id), ...sortedX.map((f) => f.id)]; - return orderedIds.length; -} - -beforeEach(() => { - vi.clearAllMocks(); - invalidateFolderCache(); -}); - -describe('coalesced progressive listing emissions (benchmark gate)', () => { - it('final listing, first-paint page and done-flag match the emit-every-page reference', async () => { - mockPagedFetch(); - const refEmits: Array<{ n: number; done: boolean }> = []; - const refFinal = await referenceFetchFolderListing('bench', (p, done) => - refEmits.push({ n: p.folders.length + p.files.length, done }) - ); - - mockPagedFetch(); - const emits: Array<{ n: number; done: boolean; partial: FolderListing }> = []; - const r = await fetchFolderListing('bench', { - onPage: (partial, done) => - emits.push({ n: partial.folders.length + partial.files.length, done, partial }) - }); - - // Identical complete listing. - expect(r.listing).toEqual(refFinal); - // First paint unchanged: the first emission is still page one. - expect(emits[0].n).toBe(refEmits[0].n); - expect(emits[0].n).toBe(PAGE_SIZE); - // Exactly one done emission, last, carrying the full listing — as before. - expect(emits.filter((e) => e.done).length).toBe(1); - expect(emits[emits.length - 1].done).toBe(true); - expect(emits[emits.length - 1].n).toBe(PAGES * PAGE_SIZE); - expect(refEmits[refEmits.length - 1].done).toBe(true); - // Emissions are a subset of what the reference produced (never more). - expect(emits.length).toBeLessThanOrEqual(refEmits.length); - // Every emitted partial is a prefix-accumulation (monotone growth). - for (let i = 1; i < emits.length; i++) expect(emits[i].n).toBeGreaterThan(emits[i - 1].n); - }); - - it('single-page folders still emit exactly once, done=true (fast path untouched)', async () => { - vi.mocked(apiFetch).mockResolvedValue( - fakeRes({ items: pageBody(PAGES - 1).items }) // no next_cursor - ); - const emits: boolean[] = []; - await fetchFolderListing('one', { onPage: (_p, done) => emits.push(done) }); - expect(emits).toEqual([true]); - }); - - it( - `collapses the O(N²) consumer re-derive on a fast ${PAGES}-page load (perf gate)`, - { timeout: 30_000 }, - async () => { - // Warm-up both paths, twice each, so V8's tiering has fully - // settled before we measure. A single warm-up was enough on - // developer laptops but bursty CPU steals on shared CI - // runners can leave one path un-tiered during measurement, - // skewing the wall-time ratio at line ~202 below. - for (let i = 0; i < 2; i++) { - mockPagedFetch(); - await referenceFetchFolderListing('warm', (p) => consumerDerive(p)); - mockPagedFetch(); - await fetchFolderListing('warm', { onPage: (p) => consumerDerive(p) }); - } - - mockPagedFetch(); - let refSorted = 0; - let refEmits = 0; - const t0 = performance.now(); - await referenceFetchFolderListing('bench', (p) => { - refEmits++; - refSorted += consumerDerive(p); - }); - const refMs = performance.now() - t0; - - mockPagedFetch(); - let sorted = 0; - let emitsN = 0; - const t1 = performance.now(); - await fetchFolderListing('bench', { - onPage: (p) => { - emitsN++; - sorted += consumerDerive(p); - } - }); - const ms = performance.now() - t1; - - console.info( - `progressive load ${PAGES}×${PAGE_SIZE}: before ${refEmits} emissions / ${refSorted} sorted elements / ${refMs.toFixed(1)} ms — after ${emitsN} emissions / ${sorted} sorted elements / ${ms.toFixed(1)} ms (${(refMs / ms).toFixed(1)}x wall, ${(refSorted / sorted).toFixed(1)}x fewer sorted elements)` - ); - - // The reference re-derived every page: Σ = P(P+1)/2 pages of elements. - expect(refEmits).toBe(PAGES); - expect(refSorted).toBe((PAGES * (PAGES + 1) * PAGE_SIZE) / 2); - // Coalesced: page 1 + final (+ occasionally one mid emission if the - // stubbed pages ever take >150 ms — they don't on any healthy runner). - expect(emitsN).toBeLessThanOrEqual(3); - // ≥5x less consumer sort work is the point of the change. - // This is a pure DETERMINISTIC count (sum of `consumerDerive` - // return values) — hardware-independent, so catches an - // actual O(N²) → O(N) regression cleanly. - expect(sorted).toBeLessThan(refSorted / 5); - // And it must show up as wall time on the combined load+ - // derive cycle. 2x floor (loosened from 3x on 2026-07-18 - // after a shared-CI-runner false alarm at 2.63x — bursty - // CPU steals eat headroom on the fine-grained - // `performance.now()` measurements). Still catches an - // O(N²) regression (which would be ~10x slower, not 2x) - // — the deterministic count above at line 200 is the real - // algorithmic gate. - expect(ms).toBeLessThan(refMs / 2); - } - ); -}); diff --git a/frontend/src/lib/api/endpoints/folders.ts b/frontend/src/lib/api/endpoints/folders.ts index 19d1f947..0ceb996e 100644 --- a/frontend/src/lib/api/endpoints/folders.ts +++ b/frontend/src/lib/api/endpoints/folders.ts @@ -110,86 +110,109 @@ export function getFolder(id: string): Promise { return request; } -/** - * Minimum spacing between intermediate progressive-render emissions of - * {@link fetchFolderListing}. Each emission hands the consumer the WHOLE - * accumulated listing, and the files view re-derives its filtered + sorted - * view from it (O(accumulated · log) with `localeCompare`), so emitting every - * page made a large-folder load Σ O(N²/page) of main-thread sort work. Page - * one and the final page always emit; pages in between only emit after this - * much time has passed since the previous emission. - */ -export const PAGE_EMIT_MIN_INTERVAL_MS = 150; +/** One page of `/api/folders/{id}/resources`. */ +export interface FolderPage { + /** + * Items in the exact order the server returned them. Under `order_by=name`, + * `type`, `size` the server puts folders first, then files; under + * `modified_at` / `created_at` the two kinds interleave. Consumers that + * need to preserve the server sort MUST iterate this list — the split + * `folders` / `files` arrays lose the interleaving. + */ + items: (FolderItem | FileItem)[]; + /** `items` filtered to folder rows (order preserved). */ + folders: FolderItem[]; + /** `items` filtered to file rows (order preserved). */ + files: FileItem[]; + /** Opaque cursor for the next page; `undefined` on the last page. */ + nextCursor?: string; +} /** - * Fetch a folder's complete listing (sub-folders + files), rebuilt from the - * cursor-paginated `/api/folders/{id}/resources` feed — the old combined - * `/listing` route was removed. We page through to the end (folders sort first - * under `order_by=name`) and split the mixed resource items back into - * `folders` / `files`. + * Fetch a single page of a folder's listing. * - * That feed carries no whole-listing ETag, so the 304 conditional fast-path is - * gone: `opts.etag` is accepted for call-site compatibility but ignored, and the - * in-memory `folderCache` is what the views revalidate against. Favorite/share - * badge sets aren't part of this feed either, so they come back empty for now. + * `/files` uses this directly and drives its own pagination — the initial + * `load()` requests page one; the ResourceList's `onloadmore` (fired by an + * IntersectionObserver at the bottom sentinel) requests the next page with + * the previous `nextCursor` and appends the results. `orderBy` is passed + * through so pages come back in the requested server-side sort order; the + * caller resets state and refetches page one on sort/group change. + * + * The legacy `fetchFolderListing` (below) is a thin loop over this — kept + * for the move-dialog folder tree, which genuinely needs every child at + * once and doesn't have an infinite-scroll surface. + */ +export async function fetchFolderPage( + folderId: string, + opts: { + orderBy?: string; + reverse?: boolean; + cursor?: string; + limit?: number; + forceRefresh?: boolean; + } = {} +): Promise { + const params = new URLSearchParams({ + order_by: opts.orderBy ?? 'name', + limit: String(opts.limit ?? 200) + }); + if (opts.reverse) params.set('reverse', 'true'); + if (opts.cursor) params.set('cursor', opts.cursor); + if (opts.forceRefresh) params.set('force_refresh', 'true'); + const res = await apiFetch(`/api/folders/${folderId}/resources?${params.toString()}`, { + credentials: 'same-origin', + cache: 'no-store' + }); + if (res.status === 403) throw Object.assign(new Error('Forbidden'), { status: 403 }); + if (!res.ok) throw new Error(`listing failed: ${res.status}`); + const page = (await res.json()) as { + items?: { resource_type: ItemType; resource: FolderItem | FileItem }[]; + next_cursor?: string; + }; + const items: (FolderItem | FileItem)[] = []; + const folders: FolderItem[] = []; + const files: FileItem[] = []; + for (const it of page.items ?? []) { + if (it.resource_type === 'folder') { + const f = it.resource as FolderItem; + folders.push(f); + items.push(f); + } else { + const f = it.resource as FileItem; + files.push(f); + items.push(f); + } + } + // Learn the children's names for breadcrumb resolution. + for (const f of folders) rememberFolderName(f.id, f.name); + return { items, folders, files, nextCursor: page.next_cursor }; +} + +/** + * Fetch a folder's complete listing (sub-folders + files) by walking every + * cursor page eagerly. Only the move-dialog tree still needs this shape — + * `/files` switched to {@link fetchFolderPage} for lazy scroll-driven paging. + * + * `opts.etag` is accepted for call-site compatibility but ignored (the + * `/resources` feed carries no whole-listing ETag). Favorite / share badge + * sets are unpopulated by this endpoint and come back empty. */ export async function fetchFolderListing( folderId: string, - opts: { - etag?: string; - forceRefresh?: boolean; - /** - * Progressive render hook: invoked with the accumulated listing so - * far (the arrays are fresh copies — safe to hand to reactive - * state). Without it, a 2,000-item folder waited for all ⌈N/200⌉ - * sequential round-trips before the first row painted; with it the - * view paints after page one (~200 items) and fills in as the tail - * pages land. Emissions are coalesced to at most one per - * {@link PAGE_EMIT_MIN_INTERVAL_MS} between the first and the final - * page — the hook is always called for page one and always called - * once more with `done === true` and the complete listing. - */ - onPage?: (partial: FolderListing, done: boolean) => void; - } = {} + opts: { etag?: string; forceRefresh?: boolean } = {} ): Promise { const folders: FolderItem[] = []; const files: FileItem[] = []; let cursor: string | undefined; - let firstPage = true; - let lastEmit = 0; do { - const params = new URLSearchParams({ order_by: 'name', limit: '200' }); - if (opts.forceRefresh) params.set('force_refresh', 'true'); - if (cursor) params.set('cursor', cursor); - const res = await apiFetch(`/api/folders/${folderId}/resources?${params.toString()}`, { - credentials: 'same-origin', - cache: 'no-store' + const page = await fetchFolderPage(folderId, { + cursor, + forceRefresh: opts.forceRefresh }); - if (res.status === 403) throw Object.assign(new Error('Forbidden'), { status: 403 }); - if (!res.ok) throw new Error(`listing failed: ${res.status}`); - const page = (await res.json()) as { - items?: { resource_type: ItemType; resource: FolderItem | FileItem }[]; - next_cursor?: string; - }; - for (const it of page.items ?? []) { - if (it.resource_type === 'folder') folders.push(it.resource as FolderItem); - else files.push(it.resource as FileItem); - } - cursor = page.next_cursor; - const done = !cursor; - if ( - opts.onPage && - (done || firstPage || performance.now() - lastEmit >= PAGE_EMIT_MIN_INTERVAL_MS) - ) { - lastEmit = performance.now(); - opts.onPage( - { folders: [...folders], files: [...files], favoriteIds: [], sharedIds: [] }, - done - ); - } - firstPage = false; + folders.push(...page.folders); + files.push(...page.files); + cursor = page.nextCursor; } while (cursor); - return { status: 200, listing: { folders, files, favoriteIds: [], sharedIds: [] } }; } diff --git a/frontend/src/routes/files/[...path]/+page.svelte b/frontend/src/routes/files/[...path]/+page.svelte index 9a889257..8bc2d309 100644 --- a/frontend/src/routes/files/[...path]/+page.svelte +++ b/frontend/src/routes/files/[...path]/+page.svelte @@ -7,11 +7,9 @@ import { SvelteSet } from 'svelte/reactivity'; import Icon from '$lib/icons/Icon.svelte'; import { - cacheFolder, createFolder, deleteFolder, - fetchFolderListing, - getCachedFolder, + fetchFolderPage, getFolder, getFolderName, invalidateFolderCache, @@ -110,17 +108,26 @@ }); let listing = $state({ folders: [], files: [], favoriteIds: [], sharedIds: [] }); + // Server-order accumulator — items in the exact sequence the backend + // returned across pages, honouring `sortField`+`reversed` on the wire. + // Under order_by=name/type/size the server puts folders first then files; + // under modified_at/created_at they interleave. `rlItems` reads this + // directly so ResourceList renders in server order without a re-sort. + let orderedItems = $state>([]); + // Cursor for the NEXT page. `undefined` after the final page has landed + // (or before the first fetch). Bound to ResourceList's `hasMore`. + let pageCursor = $state(undefined); + // Guard so a fast-firing onloadmore (double intersection tick) can't + // enqueue two concurrent next-page fetches on the same cursor. + let loadingMore = $state(false); - // Dotfile hide filter — applied BEFORE sort so `sortedFolders` / - // `sortedFiles` reflect exactly what the user sees. Selection, - // select-all, batch operations, and the empty-state check all - // derive from these visible arrays so a hidden file can't be - // silently swept up by "select all" or a "delete visible" batch. - // Direct lookups by id (deep-links via `?file=`) still go - // through `listing.files` so hidden files remain accessible by - // their own URL — same UX as macOS Finder. - const visibleFolders = $derived(filterDotfiles(listing.folders, preferences.hideDotfiles)); - const visibleFiles = $derived(filterDotfiles(listing.files, preferences.hideDotfiles)); + // Dotfile hide filter is now applied inside `rlItems` (below) directly + // on the server-ordered accumulator, so a single filter pass feeds + // ResourceList. Selection / batch ops iterate ResourceList's own + // selection set, which already excludes hidden rows. Direct lookups + // by id (deep-links via `?file=`) still go through + // `listing.files` so hidden files remain reachable via their own URL + // — same UX as macOS Finder. // Count of items suppressed by the filter — surfaced in the // empty-state hint when the folder isn't visually empty but // contains only dotfiles the user has hidden, so a "why is this @@ -211,135 +218,157 @@ // writes state, so a fast navigation can't be clobbered by an older fetch. let loadSeq = 0; - function applyListing(data: FolderListing) { - listing = data; - replaceSet(favoriteIds, data.favoriteIds); - replaceSet(sharedIds, data.sharedIds); - } - - async function load() { + /** + * Load the current folder's listing. + * + * @param reset Fresh load (folder nav / sort change / manual reload): + * clears cursor+accumulator, redoes canonicalization + + * breadcrumbs, then fetches page 1. + * + * Append (from `loadMore()` on scroll-bottom): skips + * preconditions, fetches the NEXT page using the stored + * cursor and appends to `listing`+`orderedItems`. + * + * Server-side sort: `orderBy=sortField, reverse=reversed` are passed on + * every page request so items arrive already in the requested order — + * client-side sort was removed and `rlItems` reads `orderedItems` + * verbatim. Sort/group changes trigger `load(true)` via `$effect`. + */ + async function load(reset: boolean = true) { error = null; const seq = ++loadSeq; - // External users have no home folder; send them to shared-with-me. - if (session.isExternalUser && pathSegments.length === 0) { - await goto(resolve('/shared-with-me'), { replaceState: true }); - return; - } - const home = await session.loadHomeFolder(); - - // Canonicalize bare `/files` → `/files/` (or - // the default drive's root when there's no memory yet). Keeps the URL - // explicit, the breadcrumb populated, and the drive picker correctly - // highlighted. The DrivePicker writes `oxi-last-drive-root` on click. - if (pathSegments.length === 0) { - const last = - typeof localStorage !== 'undefined' ? localStorage.getItem('oxi-last-drive-root') : null; - const target = last ?? home; - if (target) { - await goto(resolve(`/files/${target}`), { replaceState: true }); + let folderId: string; + let skeletonTimer: ReturnType | undefined; + if (reset) { + // External users have no home folder; send them to shared-with-me. + if (session.isExternalUser && pathSegments.length === 0) { + await goto(resolve('/shared-with-me'), { replaceState: true }); return; } - } + const home = await session.loadHomeFolder(); - const folderId = pathSegments.at(-1) ?? home; - if (!folderId) { - error = t('files.no_home', 'No home folder available.'); - return; - } - currentId = folderId; - filesStore.currentFolder = folderId; + // Canonicalize bare `/files` → `/files/` (or + // the default drive's root when there's no memory yet). Keeps the URL + // explicit, the breadcrumb populated, and the drive picker correctly + // highlighted. The DrivePicker writes `oxi-last-drive-root` on click. + if (pathSegments.length === 0) { + const last = + typeof localStorage !== 'undefined' ? localStorage.getItem('oxi-last-drive-root') : null; + const target = last ?? home; + if (target) { + await goto(resolve(`/files/${target}`), { replaceState: true }); + return; + } + } - // Stale-while-revalidate: paint a previously-visited folder instantly, - // then revalidate with If-None-Match (304 = keep what's shown). - const cached = getCachedFolder(folderId); - if (cached) { - applyListing(cached.listing); - loading = false; - showSkeleton = false; - } else { + const resolvedId = pathSegments.at(-1) ?? home; + if (!resolvedId) { + error = t('files.no_home', 'No home folder available.'); + return; + } + folderId = resolvedId; + currentId = folderId; + filesStore.currentFolder = folderId; + + // Reset paging state: previous folder's cursor is meaningless here, + // and mixing its rows with the new folder's would flash a wrong list. + pageCursor = undefined; + listing = { folders: [], files: [], favoriteIds: [], sharedIds: [] }; + orderedItems = []; loading = true; - } - // Delayed skeleton, only when there's nothing cached to show yet. - const skeletonTimer = setTimeout(() => { - if (loading) showSkeleton = true; - }, 100); - // Breadcrumbs resolve independently so they never block the grid paint. - // Bare `/files` was canonicalized above to `/files/` so pathSegments - // is always non-empty here for internal users. - void buildCrumbs(pathSegments).then((trail) => { - if (seq === loadSeq) crumbs = trail; - }); + // Delayed skeleton so fast loads don't flash it. + skeletonTimer = setTimeout(() => { + if (loading) showSkeleton = true; + }, 100); - // Resolve the current folder's drive_id so the read-only banner - // works even on deep-links into a sub-folder (where - // `pathSegments[0]` isn't a drive-root folder id). `getFolder` - // hits the same `/api/folders/{id}` endpoint the breadcrumb chain - // walks; the folder-name cache warmed by `buildCrumbs` above - // makes this a memoised lookup for most navigations. Guarded by - // `seq` so a stale in-flight response can't overwrite a newer - // navigation's drive. - void getFolder(folderId) - .then((folder) => { - if (seq === loadSeq) currentFolderDriveId = folder.drive_id; - }) - .catch(() => { - // Folder metadata fetch failure isn't fatal — the fallback - // chain in `currentDrive` (listing[0]?.drive_id, then - // pathSegments[0] root-folder lookup) still gives us a - // best-effort drive resolution. + // Breadcrumbs resolve independently so they never block the grid paint. + void buildCrumbs(pathSegments).then((trail) => { + if (seq === loadSeq) crumbs = trail; }); + // Resolve the current folder's drive_id so the read-only banner + // works even on deep-links into a sub-folder. Guarded by `seq`. + void getFolder(folderId) + .then((folder) => { + if (seq === loadSeq) currentFolderDriveId = folder.drive_id; + }) + .catch(() => { + // Fallback chain in `currentDrive` still gives us a + // best-effort drive resolution. + }); + } else { + // Append path: reuse `currentId`. `pageCursor === undefined` means + // we've already reached the last page; treat as no-op. + if (!currentId || pageCursor === undefined) return; + folderId = currentId; + } + try { - const res = await fetchFolderListing(folderId, { - etag: cached?.etag, - // Paint page one (~200 items) immediately instead of waiting - // for every sequential page of a large folder; later pages - // extend the view as they land. Skip when a cached copy is - // already on screen — replacing it with a partial list would - // briefly shrink the view. - onPage: cached - ? undefined - : (partial, done) => { - if (seq !== loadSeq || done) return; // final state applied below - applyListing(partial); - loading = false; - showSkeleton = false; - } + const page = await fetchFolderPage(folderId, { + orderBy: sortField, + reverse: reversed, + cursor: reset ? undefined : pageCursor }); if (seq !== loadSeq) return; // superseded by a newer navigation - if (res.status === 200 && res.listing) { - applyListing(res.listing); - cacheFolder(folderId, res.listing, res.etag); + if (reset) { + listing = { + folders: page.folders, + files: page.files, + favoriteIds: [], + sharedIds: [] + }; + orderedItems = page.items; + } else { + listing = { + folders: [...listing.folders, ...page.folders], + files: [...listing.files, ...page.files], + favoriteIds: listing.favoriteIds, + sharedIds: listing.sharedIds + }; + orderedItems = [...orderedItems, ...page.items]; } - // 304 → the cached copy already on screen is current. + pageCursor = page.nextCursor; error = null; } catch (e) { if (seq !== loadSeq) return; - // With a cached view already shown, keep it on a transient failure. - if (!cached) { - const status = (e as { status?: number })?.status; - error = - status === 403 - ? t('errors.forbidden', 'Could not load files') - : e instanceof Error - ? e.message - : String(e); - } + const status = (e as { status?: number })?.status; + error = + status === 403 + ? t('errors.forbidden', 'Could not load files') + : e instanceof Error + ? e.message + : String(e); } finally { - clearTimeout(skeletonTimer); - if (seq === loadSeq) { + if (skeletonTimer !== undefined) clearTimeout(skeletonTimer); + if (seq === loadSeq && reset) { loading = false; showSkeleton = false; } } } + /** + * Fetch and append the next page. Invoked by ResourceList's + * IntersectionObserver when the bottom sentinel enters the viewport. + * The `loadingMore` guard collapses a double-fire (the observer can + * tick twice on the same intersection edge). + */ + async function loadMore() { + if (loadingMore || pageCursor === undefined) return; + loadingMore = true; + try { + await load(false); + } finally { + loadingMore = false; + } + } + /** Data changed — drop cached listings and reload the current folder fresh. */ async function reload() { invalidateFolderCache(); - await load(); + await load(true); } function openFolder(folder: FolderItem) { @@ -1382,35 +1411,14 @@ type SortField = 'name' | 'type' | 'size' | 'modified_at' | 'created_at'; let sortField = $state('name'); let reversed = $state(false); - const sortDir = $derived<1 | -1>(reversed ? -1 : 1); - function cmpFolders(a: FolderItem, b: FolderItem): number { - let v: number; - if (sortField === 'modified_at') v = a.modified_at - b.modified_at; - else if (sortField === 'created_at') v = a.created_at - b.created_at; - // Folders have no size; fall back to name for size/type so they stay stable. - else v = a.name.localeCompare(b.name); - return v * sortDir; - } - function cmpFiles(a: FileItem, b: FileItem): number { - let v: number; - if (sortField === 'size') v = (a.size ?? 0) - (b.size ?? 0); - else if (sortField === 'modified_at') v = a.modified_at - b.modified_at; - else if (sortField === 'created_at') v = a.created_at - b.created_at; - else if (sortField === 'type') v = (a.category ?? '').localeCompare(b.category ?? ''); - else v = a.name.localeCompare(b.name); - return v * sortDir; - } - - // Sorted (folders-then-files) merged into one `Array` - // that renders directly. Order matches the un-migrated - // layout: folders precede files, sort key applied within each cohort. The - // bespoke `Entry` discriminator + swimlane bucketing that used to live - // here is gone — ResourceList does swimlane bucketing itself via - // `rlGroupBys` below. - const sortedFolders = $derived([...visibleFolders].sort(cmpFolders)); - const sortedFiles = $derived([...visibleFiles].sort(cmpFiles)); - const rlItems = $derived>([...sortedFolders, ...sortedFiles]); + // Server does the sort (order_by=sortField, reverse=reversed on every + // page request), so ResourceList reads `orderedItems` in server order + // straight through the dotfile filter. No client-side comparator + // necessary. Under order_by=name/type/size the server puts folders + // first then files; under modified_at/created_at they interleave — + // preserving the accumulator order is what surfaces that correctly. + const rlItems = $derived(filterDotfiles(orderedItems, preferences.hideDotfiles)); // Group-by state (bound to ). Kept as a `string` prop // value; the current `sortField` mirrors from the picked group's @@ -1508,7 +1516,8 @@ return () => window.removeEventListener('pointerdown', onDown); }); - // Reload whenever the route path changes. + // Reload whenever the route path OR the server sort dimension/direction + // changes. // // `load()` reads several reactive signals in its sync phase // (session.isExternalUser, session.homeFolderId, plus whatever @@ -1517,12 +1526,14 @@ // `session.loadHomeFolder()`'s own writes to `homeFolderId` // during its resolution then re-trigger the effect, firing a // second and third `load()` before the first has settled. Wrap - // in `untrack` so the ONLY dependency is `pathSegments` (route - // change is the sole legitimate re-trigger). + // in `untrack` so the ONLY dependencies are the three we WANT + // to reload on: pathSegments, sortField, reversed. $effect(() => { void pathSegments; + void sortField; + void reversed; untrack(() => { - void load(); + void load(true); }); }); @@ -1602,6 +1613,8 @@ groupBys={rlGroupBys} bind:groupBy bind:reversed + hasMore={pageCursor !== undefined} + onloadmore={loadMore} onreload={(orderBy) => { sortField = orderBy as SortField; }} diff --git a/frontend/src/routes/files/page.test.ts b/frontend/src/routes/files/page.test.ts index 8e4e4545..7ebc667a 100644 --- a/frontend/src/routes/files/page.test.ts +++ b/frontend/src/routes/files/page.test.ts @@ -46,12 +46,10 @@ vi.mock('$lib/api/endpoints/files', () => ({ uploadFileWithProgress: vi.fn() })); vi.mock('$lib/api/endpoints/folders', () => ({ - cacheFolder: vi.fn(), createFolder: vi.fn(), deleteFolder: vi.fn(), - fetchFolderListing: vi.fn(), + fetchFolderPage: vi.fn(), folderZipUrl: () => '/zip', - getCachedFolder: () => undefined, getFolder: vi.fn(async (id: string) => ({ id, name: id })), getFolderName: () => undefined, invalidateFolderCache: vi.fn(), @@ -60,7 +58,7 @@ vi.mock('$lib/api/endpoints/folders', () => ({ renameFolder: vi.fn() })); -import { fetchFolderListing, createFolder, deleteFolder } from '$lib/api/endpoints/folders'; +import { fetchFolderPage, createFolder, deleteFolder } from '$lib/api/endpoints/folders'; import { deleteFile } from '$lib/api/endpoints/files'; import { apiFetch } from '$lib/api/client'; import { files as filesStore } from '$lib/stores/files.svelte'; @@ -69,15 +67,17 @@ import FilesPage from './[...path]/+page.svelte'; const m = (fn: unknown) => fn as ReturnType; function withListing() { - m(fetchFolderListing).mockResolvedValue({ - status: 200, - etag: 'v1', - listing: { - folders: [folderItem('sub1', 'Sub')], - files: [fileItem('f1', 'hello.txt')], - favoriteIds: [], - sharedIds: [] - } + // `fetchFolderPage` returns ONE page with the accumulator shape (items in + // server order + folders/files splits). With `nextCursor` omitted the + // caller treats it as the last page — the page's items become the whole + // on-screen listing without triggering `loadMore`. + const folder = folderItem('sub1', 'Sub'); + const file = fileItem('f1', 'hello.txt'); + m(fetchFolderPage).mockResolvedValue({ + items: [folder, file], + folders: [folder], + files: [file], + nextCursor: undefined }); } @@ -131,27 +131,18 @@ beforeEach(() => { }); it('loads the home folder listing on mount and renders its contents', async () => { - m(fetchFolderListing).mockResolvedValue({ - status: 200, - etag: 'v1', - listing: { - folders: [folderItem('sub1', 'Sub')], - files: [fileItem('f1', 'hello.txt')], - favoriteIds: [], - sharedIds: [] - } - }); + withListing(); render(FilesPage); - await waitFor(() => expect(fetchFolderListing).toHaveBeenCalledWith('home', expect.anything())); + await waitFor(() => expect(fetchFolderPage).toHaveBeenCalledWith('home', expect.anything())); // VirtualList windows rows by viewport height (0 in jsdom), so assert the // surrounding chrome rendered rather than the windowed rows themselves. await screen.findByTestId('files-new-folder-btn'); }); it('shows an error when the listing fails with no cache', async () => { - m(fetchFolderListing).mockRejectedValue(Object.assign(new Error('nope'), { status: 500 })); + m(fetchFolderPage).mockRejectedValue(Object.assign(new Error('nope'), { status: 500 })); render(FilesPage); - await waitFor(() => expect(fetchFolderListing).toHaveBeenCalled()); + await waitFor(() => expect(fetchFolderPage).toHaveBeenCalled()); }); it('redirects external users away from the home folder', async () => {