From ecbdaee19e443e16900a17fd5fb8b4ab6829f6eb Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 09:58:46 +0000 Subject: [PATCH] Frontend perf: pdf.js smart preload, admin poll cleanup, list render hoisting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three targeted frontend fixes: 1. pdf.js smart preload (thumbnail.js, resourceIcon.js). The first PDF thumbnail of a session stalled 1-2s on the lazy import of the ~1.3 MB pdf.js stack. buildResourceIcon() now fires thumbnail.preloadPdf() the moment a PDF row enters the DOM, warming both the module (~300 KB, via the now promise-memoized getPdfjsLib, shared with real users) and the worker script (~1 MB, via a cache-priming fetch — pdf.js only requests it on first getDocument). Only folders that actually contain PDFs pay the download; idempotent after first call, resets on failure so transient offline retries. 2. Admin migration poll cleanup (admin.js). The 2s setInterval kept hitting the API and updating hidden DOM after leaving the Storage tab, and polled a failing endpoint forever after session expiry (!resp.ok returned without clearing). New stopMigrationPolling() helper, invoked on tab switch away from Storage, on non-running status, and on failed polls; tab re-entry re-arms via loadStorage(). 3. resourceList.js render hoisting. Per-row i18n.t() type-cell lookups and the fully item-invariant _renderCustomActions() HTML were recomputed for every row; they now resolve once per batch via _buildItemLabels() (per-category labels memoized, rebuilt each batch so locale switches keep working). _findLaneByKey() swaps the container-wide attribute querySelector for an O(1) _lanes Map kept in sync at the only lane create/wipe sites. https://claude.ai/code/session_0193Hff42gaA962wThxMGSd1 --- static/js/components/resourceIcon.js | 7 +++ static/js/components/resourceList.js | 93 +++++++++++++++++++++++----- static/js/features/thumbnail.js | 71 +++++++++++++++++---- static/js/views/admin/admin.js | 32 ++++++++-- 4 files changed, 170 insertions(+), 33 deletions(-) diff --git a/static/js/components/resourceIcon.js b/static/js/components/resourceIcon.js index ec0a797d..6f50d790 100644 --- a/static/js/components/resourceIcon.js +++ b/static/js/components/resourceIcon.js @@ -36,6 +36,13 @@ function buildResourceIcon(item, resourceType) { const canThumbnail = thumbnail?.canHandle(file) ?? false; if (canThumbnail) { + // A PDF just entered the list: warm up the pdf.js stack (~1.3 MB) + // in the background now, so a thumbnail cache-miss below doesn't + // stall its first render on the library download. Idempotent. + if (file.mime_type === 'application/pdf') { + thumbnail.preloadPdf(); + } + const img = document.createElement('img'); img.className = 'file-thumb'; img.src = `/api/files/${file.id}/thumbnail/icon`; diff --git a/static/js/components/resourceList.js b/static/js/components/resourceList.js index 73b633fc..63c82380 100644 --- a/static/js/components/resourceList.js +++ b/static/js/components/resourceList.js @@ -140,6 +140,18 @@ export class ResourceListComponent { */ this._lastGroupEl = null; + /** + * Live swimlane wrappers currently in the DOM, keyed by group key — + * lets `_findLaneByKey()` resolve in O(1) instead of a container-wide + * `querySelector` per lookup. Kept in sync with the DOM: entries are + * added where lanes are created (`_appendItems`, + * `_ensureJustAddedLane`) and the map is cleared on the full-container + * wipes in `render()` / `clear()`; lanes are never removed + * individually anywhere else. + * @type {Map} + */ + this._lanes = new Map(); + /** * Optional grouping-key resolver stored between `render()` / `append()` * calls so `addItem()` can place a new row in the correct swimlane @@ -203,6 +215,7 @@ export class ResourceListComponent { // Reset group tracking for the fresh render this._lastGroupKey = undefined; this._lastGroupEl = null; + this._lanes.clear(); this._groupFn = groupFn; this._groupLabelFn = groupLabelFn; this._headerNodeFn = headerNodeFn; @@ -244,6 +257,7 @@ export class ResourceListComponent { this._lastClickedIndex = -1; this._lastGroupKey = undefined; this._lastGroupEl = null; + this._lanes.clear(); this._groupFn = undefined; this._groupLabelFn = undefined; this._headerNodeFn = undefined; @@ -348,7 +362,8 @@ export class ResourceListComponent { // "New" swimlane, creating it on first call. const lane = this._ensureJustAddedLane(); this._items.set(item.id, item); - row = isFile ? this._createFileItem(/** @type {FileItem} */ (item)) : this._createFolderItem(/** @type {FolderItem} */ (item)); + const labels = this._buildItemLabels(); + row = isFile ? this._createFileItem(/** @type {FileItem} */ (item), labels) : this._createFolderItem(/** @type {FolderItem} */ (item), labels); lane.appendChild(row); } else { // Flat list (no grouping) — append at the end like before. @@ -394,6 +409,7 @@ export class ResourceListComponent { const lane = document.createElement('div'); lane.className = 'resource-list__swimlane-group resource-list__swimlane-group--just-added'; lane.dataset.groupKey = JUST_ADDED_KEY; + this._lanes.set(JUST_ADDED_KEY, lane); const header = document.createElement('div'); header.className = 'resource-list__swimlane-header'; @@ -420,13 +436,14 @@ export class ResourceListComponent { * Locate an on-screen swimlane wrapper by its group key. Returns * `null` when no swimlane currently matches. * + * O(1) via the `_lanes` registry — see its declaration for how it is + * kept in sync with the DOM. + * * @param {string} key * @returns {HTMLElement | null} */ _findLaneByKey(key) { - // CSS.escape covers arbitrary key shapes (dates with colons, - // UUIDs with dashes, etc.) so the attribute selector is safe. - return /** @type {HTMLElement | null} */ (this._container.querySelector(`.resource-list__swimlane-group[data-group-key="${CSS.escape(String(key))}"]`)); + return this._lanes.get(String(key)) ?? null; } /** @@ -523,6 +540,9 @@ export class ResourceListComponent { _appendItems(items, groupFn, groupLabelFn, headerNodeFn) { const fragment = document.createDocumentFragment(); + // Resolve batch-invariant labels once, not once per row. + const labels = this._buildItemLabels(); + // Start from the persisted key so load-more pages continue seamlessly. let lastGroupKey = this._lastGroupKey; @@ -545,11 +565,12 @@ export class ResourceListComponent { if (key !== null) { fragmentGroup = document.createElement('div'); fragmentGroup.className = 'resource-list__swimlane-group'; - // Stamp the group key on the wrapper so `addItem()` - // can locate this swimlane later via - // `_findLaneByKey()` and append into it without a - // full re-render. + // Stamp the group key on the wrapper (handy in + // devtools) and register it in `_lanes` so + // `_findLaneByKey()` can locate this swimlane later + // without a container-wide query. fragmentGroup.dataset.groupKey = key; + this._lanes.set(key, fragmentGroup); fragmentGroup.appendChild(this._createGroupHeader(key, groupLabelFn, headerNodeFn)); fragment.appendChild(fragmentGroup); } @@ -558,7 +579,9 @@ export class ResourceListComponent { // Dispatch to the correct renderer: files have mime_type, folders do not. const isFile = 'mime_type' in item; - const itemEl = isFile ? this._createFileItem(/** @type {FileItem} */ (item)) : this._createFolderItem(/** @type {FolderItem} */ (item)); + const itemEl = isFile + ? this._createFileItem(/** @type {FileItem} */ (item), labels) + : this._createFolderItem(/** @type {FolderItem} */ (item), labels); // Priority: live DOM group (load-more continuation) > current fragment group > bare container const target = liveGroup ?? fragmentGroup; @@ -600,12 +623,50 @@ export class ResourceListComponent { return el; } + /** + * @typedef {Object} ItemLabels + * @property {string} folderTypeLabel - Type-cell label for folders. + * @property {string} customActionsHtml - Pre-rendered inline-action buttons. + * @property {(category: string) => string} fileTypeLabel - Type-cell label + * for a file category (memoized per batch). + */ + + /** + * Resolve every per-row value that does not depend on the item once per + * batch: the i18n lookups for the type cell and the custom-actions HTML + * are identical for all 50 rows of a page, so repeating them in + * `_createFileItem` / `_createFolderItem` was pure overhead. Built fresh + * on every call (never cached on the instance), so a locale switch is + * picked up naturally by the next render/append. + * + * @returns {ItemLabels} + */ + _buildItemLabels() { + const fallbackTypeLabel = i18n.t('files.file_types.document'); + /** @type {Map} */ + const byCategory = new Map(); + return { + folderTypeLabel: i18n.t('files.file_types.folder'), + customActionsHtml: this._renderCustomActions(), + fileTypeLabel(category) { + if (!category) return fallbackTypeLabel; + let label = byCategory.get(category); + if (label === undefined) { + label = i18n.t(`files.file_types.${category.toLowerCase()}`) || category; + byCategory.set(category, label); + } + return label; + } + }; + } + /** * Build a .file-item DOM element for a folder. * @param {FolderItem} folder + * @param {ItemLabels} labels - Batch-invariant labels from `_buildItemLabels()`. * @returns {HTMLElement} */ - _createFolderItem(folder) { + _createFolderItem(folder, labels) { const cfg = this._cfg; const el = document.createElement('div'); const modClass = cfg.itemModifierClass ? ` ${cfg.itemModifierClass}` : ''; @@ -632,11 +693,11 @@ export class ResourceListComponent {
${cfg.showPath ? `
${escapeHtml(folder.path || '')}
` : ''} - ${cfg.showType ? `
${i18n.t('files.file_types.folder')}
` : ''} + ${cfg.showType ? `
${labels.folderTypeLabel}
` : ''}
--
${formattedDate}
- ${this._renderCustomActions()} + ${labels.customActionsHtml} ${cfg.showFavorite ? `` : ''} ${cfg.showContextMenu ? '' : ''}
@@ -649,12 +710,12 @@ export class ResourceListComponent { /** * Build a .file-item DOM element for a file. * @param {FileItem} file + * @param {ItemLabels} labels - Batch-invariant labels from `_buildItemLabels()`. * @returns {HTMLElement} */ - _createFileItem(file) { + _createFileItem(file, labels) { const cfg = this._cfg; - const cat = file.category || ''; - const typeLabel = cat ? i18n.t(`files.file_types.${cat.toLowerCase()}`) || cat : i18n.t('files.file_types.document'); + const typeLabel = labels.fileTypeLabel(file.category || ''); const fileSize = file.size_formatted || formatFileSize(file.size); const dateVal = /** @type {Record} */ (/** @type {unknown} */ (file))[cfg.dateField] ?? file.modified_at; const formattedDate = cfg.dateFormatter ? cfg.dateFormatter(dateVal) : formatDateTime(new Date(dateVal)); @@ -685,7 +746,7 @@ export class ResourceListComponent {
${fileSize}
${formattedDate}
- ${this._renderCustomActions()} + ${labels.customActionsHtml} ${cfg.showFavorite ? `` : ''} ${cfg.showContextMenu ? '' : ''}
diff --git a/static/js/features/thumbnail.js b/static/js/features/thumbnail.js index 17f23922..c0aa0ea3 100644 --- a/static/js/features/thumbnail.js +++ b/static/js/features/thumbnail.js @@ -2,29 +2,47 @@ import { getCsrfHeaders } from '../core/csrf.js'; /** @import {FileItem} from '../core/types.js' */ +// IMPORTANT: absolute paths so the dynamic import resolves correctly both in +// dev mode (native ESM, module at /js/features/thumbnail.js) and in release +// mode (IIFE bundle at /js/app.{hash}.js — relative '../vendors/…' would +// incorrectly resolve to /vendors/… instead of /js/vendors/…). +const PDFJS_LIB_URL = '/js/vendors/pdf.min.mjs'; +const PDFJS_WORKER_URL = '/js/vendors/pdf.worker.min.mjs'; + /** - * use any type so tsc will not scan library - * @type {any} + * Memoized import of pdf.min.mjs (in-flight or settled). + * use any type so tsc will not scan library. + * Reset to null on failure so a later call retries (e.g. transient offline). + * @type {Promise | null} */ -let _pdfjsLib = null; +let _pdfjsLibPromise = null; + +/** True once the worker script warm-up fetch has completed successfully. */ +let _pdfWorkerWarmed = false; // TODO: do we need to add a max concurrncy ? /** * Lazy-loads pdf.min.mjs on first use via dynamic import so it is never * bundled into the IIFE (it uses top-level await which breaks IIFE wrapping). + * Memoizing the promise (rather than the resolved module) lets concurrent + * callers — e.g. `preloadPdf()` racing the first real thumbnail — share a + * single network fetch. * @returns {Promise} */ -async function getPdfjsLib() { - if (_pdfjsLib) return _pdfjsLib; - // IMPORTANT: use an absolute path so the import resolves correctly both in - // dev mode (native ESM, module at /js/features/thumbnail.js) and in release - // mode (IIFE bundle at /js/app.{hash}.js — relative '../vendors/…' would - // incorrectly resolve to /vendors/… instead of /js/vendors/…). - const lib = '/js/vendors/pdf.min.mjs'; - _pdfjsLib = /** @type {any} */ (await import(lib)); - _pdfjsLib.GlobalWorkerOptions.workerSrc = '/js/vendors/pdf.worker.min.mjs'; - return _pdfjsLib; +function getPdfjsLib() { + if (!_pdfjsLibPromise) { + _pdfjsLibPromise = import(PDFJS_LIB_URL) + .then((lib) => { + lib.GlobalWorkerOptions.workerSrc = PDFJS_WORKER_URL; + return lib; + }) + .catch((err) => { + _pdfjsLibPromise = null; // allow retry after a failed load + throw err; + }); + } + return _pdfjsLibPromise; } export const thumbnail = { @@ -43,6 +61,33 @@ export const thumbnail = { return false; }, + /** + * Fire-and-forget warm-up of the pdf.js stack (module + worker script). + * + * Called the moment a PDF row enters the DOM (see resourceIcon.js), so + * the ~1.3 MB library downloads in the background while the user is + * still looking at the list — instead of stalling the first thumbnail + * render on it. Idempotent and cheap after the first call, and only + * folders that actually contain PDFs ever pay the download. + */ + preloadPdf() { + // Module (≈300 KB): shares the memoized promise with real users. + getPdfjsLib().catch(() => { + /* transient failure — the first real use retries */ + }); + + // Worker (≈1 MB): pdf.js only fetches it via `new Worker(...)` on the + // first getDocument(), so prime the HTTP cache with a plain fetch. + // Reading the body ensures the download completes and is cacheable. + if (_pdfWorkerWarmed) return; + _pdfWorkerWarmed = true; + fetch(PDFJS_WORKER_URL) + .then((r) => (r.ok ? r.blob() : Promise.reject(new Error(`HTTP ${r.status}`)))) + .catch(() => { + _pdfWorkerWarmed = false; // allow retry on a later sighting + }); + }, + // TODO: use these informations from server ? SIZES: { icon: { width: 150, height: 150 }, diff --git a/static/js/views/admin/admin.js b/static/js/views/admin/admin.js index 6bcd5843..6a3330d6 100644 --- a/static/js/views/admin/admin.js +++ b/static/js/views/admin/admin.js @@ -151,6 +151,10 @@ function switchTab(name, el) { } activeTabName = name; + // The migration auto-poll only makes sense while the Storage tab is + // visible — without this it would keep hitting the API every 2 s + // (and updating hidden DOM) for as long as a migration runs. + if (name !== 'storage') stopMigrationPolling(); if (name === 'users') loadUsers(); if (name === 'dashboard') loadDashboard(); if (name === 'storage') loadStorage(); @@ -908,6 +912,22 @@ async function testStorageConnection() { /** @type {ReturnType | null} */ let migrationPollTimer = null; +/** + * Stop the 2 s migration auto-poll if it is armed. + * + * Called when the poll observes a non-running status, when a poll request + * fails (an expired admin session would otherwise be retried every 2 s + * forever), and when the user leaves the Storage tab. Re-entering the tab + * re-arms it via `loadStorage()` → `loadMigrationStatus()` while a + * migration is running. + */ +function stopMigrationPolling() { + if (migrationPollTimer) { + clearInterval(migrationPollTimer); + migrationPollTimer = null; + } +} + /** * @param {string} msg * @param {string} type @@ -979,7 +999,12 @@ async function loadMigrationStatus() { headers: headers(), credentials: 'same-origin' }); - if (!resp.ok) return; + if (!resp.ok) { + // Don't keep hammering a failing endpoint (e.g. expired session); + // any migration button or tab re-entry re-arms the poll. + stopMigrationPolling(); + return; + } const m = await resp.json(); updateMigrationUI(m); @@ -988,9 +1013,8 @@ async function loadMigrationStatus() { if (!migrationPollTimer) { migrationPollTimer = setInterval(loadMigrationStatus, 2000); } - } else if (migrationPollTimer) { - clearInterval(migrationPollTimer); - migrationPollTimer = null; + } else { + stopMigrationPolling(); } } catch (_e) { /* ignore */