Render actual images as thumbnails using blob URLs #12
Conversation
Added getThumbnailBlobUrl to fetch thumbnails as blobs and create object URLs. Modified FileBrowser.vue to use these blob URLs for media thumbnails, loading the first 5 eagerly and others on demand. Clean up blob URLs on unmount to prevent memory leaks. This improves thumbnail loading performance and caching behavior. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
WalkthroughImplemented blob URL-based lazy loading for thumbnails in the file browser. Added a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FileBrowser
participant Cache as thumbnailBlobUrls Map
participant AppUtils as getThumbnailBlobUrl
participant API
rect rgb(240, 248, 255)
Note over User,API: Initial Load (Preload Phase)
User->>FileBrowser: fetchFiles()
FileBrowser->>Cache: Clear thumbnailBlobUrls
FileBrowser->>AppUtils: loadThumbnailBlobUrl() x5 (background)
AppUtils->>API: Fetch thumbnail arraybuffer
API-->>AppUtils: arraybuffer + metadata
AppUtils->>AppUtils: Infer MIME type
AppUtils->>AppUtils: Create Blob object
AppUtils-->>FileBrowser: blob object URL
FileBrowser->>Cache: Store URL by item.key
end
rect rgb(240, 255, 240)
Note over User,API: On-Demand Load (Lazy Phase)
User->>FileBrowser: mouseenter on thumbnail
FileBrowser->>Cache: Check getThumbnailUrlForItem(item)
alt Cache Miss
FileBrowser->>AppUtils: loadThumbnailBlobUrl(item)
AppUtils->>API: Fetch thumbnail arraybuffer
API-->>AppUtils: arraybuffer + metadata
AppUtils-->>FileBrowser: blob object URL
FileBrowser->>Cache: Store URL
else Cache Hit
Cache-->>FileBrowser: Return cached URL
end
FileBrowser->>User: Render thumbnail
end
rect rgb(255, 240, 240)
Note over User,API: Cleanup
User->>FileBrowser: Component unmount
FileBrowser->>Cache: Revoke all blob URLs
FileBrowser->>Cache: Clear map
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/dashboard/src/components/files/FileBrowser.vue (1)
689-716: Race condition risk when rapidly navigating folders.There's a timing issue here: you clear
thumbnailBlobUrls.value = {}at line 692, but then immediately start loading thumbnails (lines 699-706) without awaiting the API calls. If the user navigates away before these loads complete, you could:
- Create blob URLs for the old folder
- Not revoke them because they're not in the map anymore (cleared on next navigation)
- Accumulate memory leaks over time
This is exacerbated by the lack of cancellation—there's no AbortController to cancel in-flight requests when navigating away.
Consider tracking in-flight requests and canceling them:
+ const abortController = ref(null); + const fetchFiles = async () => { + // Cancel any in-flight thumbnail requests + if (abortController.value) { + abortController.value.abort(); + } + abortController.value = new AbortController(); + loading.value = true; // Clear old thumbnail URLs when changing folders + Object.values(thumbnailBlobUrls.value).forEach((blobUrl) => { + if (blobUrl && blobUrl.startsWith("blob:")) { + URL.revokeObjectURL(blobUrl); + } + }); thumbnailBlobUrls.value = {}; try { items.value = await apiHandler.fetchFile( props.bucket, selectedFolder.value, "/", ); // Start loading thumbnails for media files in background const mediaFiles = items.value.filter( (item) => item.type === "file" && isMediaFile(item.name), ); // Load first 5 thumbnails immediately, rest on demand for (let i = 0; i < Math.min(5, mediaFiles.length); i++) { - loadThumbnailBlobUrl(mediaFiles[i]); + loadThumbnailBlobUrl(mediaFiles[i], abortController.value.signal); } } catch (error) { console.error("Failed to fetch files:", error); $q.notify({ type: "negative", message: "Failed to load files", }); } finally { loading.value = false; } };Then update
loadThumbnailBlobUrlto accept and pass the signal togetThumbnailBlobUrl(which also needs updating in appUtils.js to accept and use the signal).
🧹 Nitpick comments (3)
packages/dashboard/src/appUtils.js (1)
525-540: Add timeout to prevent hanging requests.The API call doesn't specify a timeout, which means a slow or unresponsive server could leave thumbnail requests hanging indefinitely. Given that you're preloading 5 thumbnails and then loading more on mouseenter, a single hanging request could degrade the entire user experience.
Consider adding a reasonable timeout:
export const getThumbnailBlobUrl = async (file, bucket) => { try { const response = await api.get(`/buckets/${bucket}/${encode(file.key)}`, { responseType: "arraybuffer", + timeout: 10000, // 10 second timeout }); const mimeType = file.httpMetadata?.contentType || inferMimeTypeFromFilename(file.name); const blob = new Blob([response.data], { type: mimeType }); return URL.createObjectURL(blob); } catch (error) { console.error("Failed to load thumbnail:", error); return null; } };packages/dashboard/src/components/files/FileBrowser.vue (2)
699-706: Preload strategy might be too aggressive or too conservative.You're preloading exactly 5 thumbnails, but:
- On mobile with slow connections, 5 might be too many and cause initial load lag
- On desktop with fast connections in grid view, 5 might not cover the initial viewport
- The "magic number" 5 has no comments explaining the rationale
Consider making the preload count dynamic based on viewport size and view mode:
// Load first 5 thumbnails immediately, rest on demand - for (let i = 0; i < Math.min(5, mediaFiles.length); i++) { + // Calculate how many thumbnails are likely visible in viewport + const preloadCount = viewMode.value === 'grid' + ? ($q.platform.is.mobile ? 4 : 10) + : 5; + for (let i = 0; i < Math.min(preloadCount, mediaFiles.length); i++) { loadThumbnailBlobUrl(mediaFiles[i]); }
986-994: Missing error feedback to user on thumbnail load failure.When
getThumbnailBlobUrlreturnsnull(line 990), you silently fail—the thumbnail just won't appear. Users have no indication that something went wrong, especially if it's a network issue affecting all thumbnails.Consider tracking failed loads and showing a subtle notification after multiple failures:
+ const failedThumbnailCount = ref(0); + const loadThumbnailBlobUrl = async (item) => { if (thumbnailBlobUrls.value[item.key]) { return; // Already loaded } const blobUrl = await getThumbnailBlobUrl(item, props.bucket); if (blobUrl) { thumbnailBlobUrls.value[item.key] = blobUrl; + failedThumbnailCount.value = 0; // Reset on success + } else { + failedThumbnailCount.value++; + if (failedThumbnailCount.value === 3) { + $q.notify({ + type: "warning", + message: "Having trouble loading thumbnails. Check your connection.", + timeout: 3000 + }); + } } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/dashboard/src/appUtils.js(1 hunks)packages/dashboard/src/components/files/FileBrowser.vue(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/dashboard/src/appUtils.js (2)
packages/dashboard/src/stores/main-store.js (1)
response(51-53)packages/dashboard/src/boot/axios.js (1)
api(14-14)
🪛 GitHub Actions: Build
packages/dashboard/src/components/files/FileBrowser.vue
[error] 1500-1502: lint/complexity/useOptionalChain: Change to an optional chain.
[error] 533-533: The imports and exports are not sorted. Safe fix: Organize Imports.
🔍 Remote MCP
Now let me search for information about Web Blob API and object URLs specifically:Perfect! I now have sufficient context to provide a comprehensive review summary. Let me compile the findings.
Summary of Additional Context for PR #12 Review
Memory Management & Blob URL Best Practices
The URL.revokeObjectURL() method releases an existing object URL which was previously created by calling URL.createObjectURL(). URL.revokeObjectURL(url) removes the reference from the internal mapping, thus allowing the Blob to be deleted (if there are no other references), and the memory to be freed.
Critical Implementation Consideration: URL.createObjectURL approximately creates a mapping of the ID in the created blob URL to the actual blob in a hidden Map associated with the current global object. From the perspective of the GC the Blob remains reachable until either the global itself becomes collectible (when you navigate away from the page or close a tab) or when the mapping is revoked. This means the PR's approach to cleaning up blob URLs on component unmount is correct and necessary to prevent memory leaks during navigation or folder changes.
Specific to Fetched Blobs: If you pass a Blob (or a File) that you did generate (or which has been fetched), then the Blob has to be stored in memory. Since the PR fetches thumbnails as ArrayBuffers from an API and converts them to Blobs, explicit revocation is critical to avoid memory accumulation when browsing multiple folders or files.
Vue.js Lifecycle & Cleanup Patterns
The onUnmounted() hook is essential for cleaning up manually created side effects, such as clearing timers, removing DOM event listeners, or closing server connections. The PR correctly implements this pattern for revoking blob URLs on component unmount.
Additionally, Vue's reactivity system supports using ref() for creating reactive state that can be accessed and modified via .value in JavaScript, with automatic unwrapping in templates, which aligns with the PR's use of ref({}) for thumbnailBlobUrls.
MIME Type Detection Considerations
Browsers don't read the bytestream of the file to determine its media type. Instead, they read the file's extension. The PR's fallback strategy—preferring file.httpMetadata.contentType but deriving from filename when unavailable—is pragmatic. However, browsers use the MIME type, not the file extension, to determine how to process a URL, so ensuring the correct MIME type is important for proper rendering in img elements.
Key Review Focus Areas
-
Memory Leak Prevention: Verify that all blob URLs created by
getThumbnailBlobUrl()are properly tracked in thethumbnailBlobUrlsmap and revoked:- On folder navigation (when
fetchFilesclears the map) - On component unmount
- Consider if there are edge cases (e.g., file deletion, rapid navigation)
- On folder navigation (when
-
Preload/On-Demand Balance: The PR preloads 5 thumbnails and loads on mouseenter. Validate that this doesn't cause excessive API calls or memory issues with large file lists.
-
Error Handling: Ensure that failed blob URL fetches don't leave broken references or prevent fallback thumbnail rendering.
-
MIME Type Reliability: Test the fallback from
file.httpMetadata.contentTypeto filename-based detection to ensure images render correctly.,,
🔇 Additional comments (4)
packages/dashboard/src/appUtils.js (1)
531-533: MIME type fallback looks solid.The fallback from
file.httpMetadata?.contentTypetoinferMimeTypeFromFilename(file.name)is pragmatic and should handle most cases correctly. Based on the MIME_TYPE_MAP (lines 21-47), you've got good coverage for common media types.packages/dashboard/src/components/files/FileBrowser.vue (3)
624-624: Good use of ref for reactive state.Using
ref({})to create a reactive map of blob URLs is the right approach for Vue 3. This will properly track additions and allow the template to react to changes.
981-994: Solid separation of concerns for thumbnail loading.The split between
getThumbnailUrlForItem(getter) andloadThumbnailBlobUrl(async loader) is clean and makes the template code readable. The deduplication check at line 987-989 prevents redundant fetches.
127-134: On-demand loading trigger is appropriate.Using
@mouseenterto triggerloadThumbnailBlobUrlis a good balance—it gives users a smooth experience without preloading everything. The lazy loading on the<img>tags (line 131, 257) is also correct.Also applies to: 253-260
| export const getThumbnailBlobUrl = async (file, bucket) => { | ||
| try { | ||
| const response = await api.get(`/buckets/${bucket}/${encode(file.key)}`, { | ||
| responseType: "arraybuffer", | ||
| }); | ||
|
|
||
| const mimeType = | ||
| file.httpMetadata?.contentType || | ||
| inferMimeTypeFromFilename(file.name); | ||
| const blob = new Blob([response.data], { type: mimeType }); | ||
| return URL.createObjectURL(blob); | ||
| } catch (error) { | ||
| console.error("Failed to load thumbnail:", error); | ||
| return null; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Critical: Missing cache-busting parameters creates stale thumbnail issue.
The new getThumbnailBlobUrl function doesn't include the disposition or cache version parameters that getThumbnailUrl uses (lines 515-523). This means:
- Thumbnails may be served from browser cache even after file replacement
- The cache version in
file.customMetadata["cache-version"]is completely ignored - Users won't see updated thumbnails after replacing files
This is especially problematic since the codebase has a whole "Refresh Cache" feature (see handleRefreshCache, handleBulkRefreshCache in FileBrowser.vue) that updates cache versions—but those updates won't affect blob URL thumbnails at all!
Apply this diff to include cache-busting parameters:
export const getThumbnailBlobUrl = async (file, bucket) => {
try {
+ const cacheVersion = file.customMetadata?.["cache-version"] || Date.now();
+ const params = new URLSearchParams({
+ v: cacheVersion.toString(),
+ disposition: "inline"
+ });
+
- const response = await api.get(`/buckets/${bucket}/${encode(file.key)}`, {
+ const response = await api.get(`/buckets/${bucket}/${encode(file.key)}?${params.toString()}`, {
responseType: "arraybuffer",
});
const mimeType =
file.httpMetadata?.contentType ||
inferMimeTypeFromFilename(file.name);
const blob = new Blob([response.data], { type: mimeType });
return URL.createObjectURL(blob);
} catch (error) {
console.error("Failed to load thumbnail:", error);
return null;
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const getThumbnailBlobUrl = async (file, bucket) => { | |
| try { | |
| const response = await api.get(`/buckets/${bucket}/${encode(file.key)}`, { | |
| responseType: "arraybuffer", | |
| }); | |
| const mimeType = | |
| file.httpMetadata?.contentType || | |
| inferMimeTypeFromFilename(file.name); | |
| const blob = new Blob([response.data], { type: mimeType }); | |
| return URL.createObjectURL(blob); | |
| } catch (error) { | |
| console.error("Failed to load thumbnail:", error); | |
| return null; | |
| } | |
| }; | |
| export const getThumbnailBlobUrl = async (file, bucket) => { | |
| try { | |
| const cacheVersion = file.customMetadata?.["cache-version"] || Date.now(); | |
| const params = new URLSearchParams({ | |
| v: cacheVersion.toString(), | |
| disposition: "inline" | |
| }); | |
| const response = await api.get(`/buckets/${bucket}/${encode(file.key)}?${params.toString()}`, { | |
| responseType: "arraybuffer", | |
| }); | |
| const mimeType = | |
| file.httpMetadata?.contentType || | |
| inferMimeTypeFromFilename(file.name); | |
| const blob = new Blob([response.data], { type: mimeType }); | |
| return URL.createObjectURL(blob); | |
| } catch (error) { | |
| console.error("Failed to load thumbnail:", error); | |
| return null; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In packages/dashboard/src/appUtils.js around lines 525 to 540,
getThumbnailBlobUrl is missing the cache-busting query params used by
getThumbnailUrl so thumbnails can be served from cached responses; modify the
api.get call to include the same query parameters (e.g. disposition=inline and
cache-version equal to file.customMetadata["cache-version"] or a sensible
default) when building the request URL/params (ensuring you still URL-encode
file.key), so the returned arraybuffer reflects the current cache-version and
avoids stale browser cache; keep the existing mime type inference and Blob
creation logic and return the object URL as before.
| decode, | ||
| encode, | ||
| getThumbnailUrl, | ||
| getThumbnailBlobUrl, |
There was a problem hiding this comment.
Pipeline failure: Import sorting required.
The linter is complaining that imports aren't sorted. You added getThumbnailBlobUrl to line 561, but the imports need to be organized.
Run the safe fix suggested by the linter to organize imports, or manually ensure alphabetical ordering.
🤖 Prompt for AI Agents
In packages/dashboard/src/components/files/FileBrowser.vue around line 561, the
import list is not alphabetically sorted after adding getThumbnailBlobUrl; run
the project's import-sort/linter safe-fix (e.g., eslint --fix or the
import-sorting tool configured) or manually reorder the import entries so they
follow the project's alphabetical ordering rules (place getThumbnailBlobUrl in
the correct position among other imports) and save to clear the import-sorting
lint error.
| // Clean up blob URLs to prevent memory leaks | ||
| Object.values(thumbnailBlobUrls.value).forEach((blobUrl) => { | ||
| if (blobUrl && blobUrl.startsWith("blob:")) { | ||
| URL.revokeObjectURL(blobUrl); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Cleanup logic is correct but incomplete.
You're properly revoking blob URLs on component unmount (lines 1498-1504), which is essential for memory management as per the Web Blob API docs. HOWEVER, you're not revoking the old blob URLs when clearing the map at line 692 during folder navigation. This means:
- User opens folder A → creates 5 blob URLs
- User navigates to folder B → line 692 clears the map BUT doesn't revoke the URLs
- Those 5 blob URLs from folder A are now orphaned in memory
- Repeat 10 times across folders → 50+ blob URLs leaked
Based on learnings about blob URL lifecycle management, this will accumulate memory until the page is closed or refreshed.
See the earlier comment on lines 689-716 for the fix—you need to revoke before clearing the map.
🧰 Tools
🪛 GitHub Actions: Build
[error] 1500-1502: lint/complexity/useOptionalChain: Change to an optional chain.
🤖 Prompt for AI Agents
In packages/dashboard/src/components/files/FileBrowser.vue around lines 692 and
1498-1504, the cleanup on unmount revokes blob URLs but the map-clear at ~line
692 does not — causing orphaned blob URLs when navigating folders. Before
clearing or replacing thumbnailBlobUrls at line 692, iterate its values and call
URL.revokeObjectURL on any non-empty value that startsWith("blob:"), then clear
the map; keep the existing unmount cleanup as-is. Ensure this
revoke-before-clear behavior is applied wherever the map is reset during
navigation.
Pipeline failure: Use optional chain instead of manual checks.
The linter is complaining about lines 1500-1502. You're manually checking blobUrl && blobUrl.startsWith("blob:") when you could use optional chaining.
Apply this diff:
// Clean up blob URLs to prevent memory leaks
Object.values(thumbnailBlobUrls.value).forEach((blobUrl) => {
- if (blobUrl && blobUrl.startsWith("blob:")) {
+ if (blobUrl?.startsWith("blob:")) {
URL.revokeObjectURL(blobUrl);
}
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Clean up blob URLs to prevent memory leaks | |
| Object.values(thumbnailBlobUrls.value).forEach((blobUrl) => { | |
| if (blobUrl && blobUrl.startsWith("blob:")) { | |
| URL.revokeObjectURL(blobUrl); | |
| } | |
| }); | |
| }); | |
| // Clean up blob URLs to prevent memory leaks | |
| Object.values(thumbnailBlobUrls.value).forEach((blobUrl) => { | |
| if (blobUrl?.startsWith("blob:")) { | |
| URL.revokeObjectURL(blobUrl); | |
| } | |
| }); |
🧰 Tools
🪛 GitHub Actions: Build
[error] 1500-1502: lint/complexity/useOptionalChain: Change to an optional chain.
🤖 Prompt for AI Agents
In packages/dashboard/src/components/files/FileBrowser.vue around lines 1498 to
1504, the cleanup loop manually checks blobUrl with "blobUrl &&
blobUrl.startsWith('blob:')"; replace that manual null/undefined check with
optional chaining so the condition becomes blobUrl?.startsWith('blob:'), keeping
the URL.revokeObjectURL(blobUrl) call unchanged and preserving the same
behavior.
Summary
Changes
Core Utils
File Browser UI
State and Data Flow
Imports and Usage
Tests / Validation Plan
Notes
🌿 Generated by Terry
ℹ️ Tag @terragon-labs to ask questions and address PR feedback
📎 Task: https://www.terragonlabs.com/task/8aa2f0b8-9fba-4b5c-beba-eb65de79cee7
Render Real Image Thumbnails via Blob URLs
Changes:
getThumbnailBlobUrl(file, bucket)async utility that fetches image data as ArrayBuffer from the API, detects MIME type from file metadata or filename, creates a Blob URL, and returns it with error handling that logs failures and returns null.thumbnailBlobUrlsreactive map caching blob URLs by item.keygetThumbnailUrlForItem(item)to retrieve cached blob URL;loadThumbnailBlobUrl(item)to fetch and cache asyncImpact: Enables actual image thumbnails using locally-created blob URLs instead of direct remote URLs, improving load reliability while reducing external dependencies. Maintains graceful fallback to existing thumbnail behavior and backward compatibility.