Refactor: optimize notes covers rendering#356
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
neSpecc
left a comment
neSpecc
left a comment
There was a problem hiding this comment.
can we show cover skeleton while image is still loading?
There was a problem hiding this comment.
Pull request overview
This PR refactors homepage note cover loading to improve perceived performance by fetching note metadata first and then downloading cover images asynchronously, allowing covers to render progressively as they arrive.
Changes:
- Refactors
NoteListService.getNoteList()to return metadata only and introducesloadCover()for per-note cover fetching. - Adds background parallel cover downloading in
useNoteListand updates notes reactively as covers complete. - Updates
NoteList.vueto only pass blob URLs to theCardimage source.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/presentation/components/note-list/NoteList.vue | Only renders a cover when it’s a blob: URL (progressive rendering support). |
| src/domain/noteList.service.ts | Splits cover downloading into a dedicated loadCover() method and returns metadata-only lists. |
| src/application/services/useNoteList.ts | Adds background parallel cover loading and reactive per-item updates; updates lifecycle cleanup implications. |
Comments suppressed due to low confidence (1)
src/application/services/useNoteList.ts:155
covernow holds a server key until it’s replaced with ablob:URL. The unmount cleanup currently callsURL.revokeObjectURLfor any non-nullcover, which will include non-blob keys. Restrict revocation to actualblob:URLs so the cleanup only targets object URLs created byURL.createObjectURL.
loadCovers().catch(console.error);
};
/**
* Load first notes
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const loadCovers = async (): Promise<void> => { | ||
| if (isEmpty(noteList.value)) { | ||
| return; | ||
| } | ||
|
|
||
| const list = await noteListService.getNoteList(page, onlyCreatedByUser); | ||
| const list = noteList.value; | ||
| const items = list.items; | ||
|
|
||
| isLoading.value = false; | ||
| await Promise.all(items.map(async (item, index) => { | ||
| /** | ||
| * If cover is null, the note has no cover image | ||
| */ | ||
| if (item.cover === null) { | ||
| return; | ||
| } | ||
|
|
||
| /** | ||
| * If cover is already a blob URL, it was already loaded | ||
| */ | ||
| if (item.cover.startsWith('blob:')) { | ||
| return; | ||
| } | ||
|
|
||
| return list; | ||
| const url = await noteListService.loadCover(item.id, item.cover); | ||
|
|
||
| if (url !== null) { | ||
| const currentItem = list.items[index]; | ||
|
|
||
| if (currentItem?.id !== item.id) { | ||
| return; | ||
| } | ||
| /** | ||
| * Update the specific note's cover reactively so the card renders the image | ||
| */ | ||
| list.items[index] = { | ||
| ...currentItem, | ||
| cover: url, | ||
| }; | ||
| } | ||
| })); | ||
| }; |
There was a problem hiding this comment.
loadCovers() re-scans the whole noteList.value.items array on every call (not just the page that was just loaded). If loadMoreNotes() fires again before an earlier page's covers finish downloading (easy to hit on the slow connections), the still-pending items from the previous page get requested a second time in parallel with the first request.
Might be worth tracking in-flight note ids or only passing the newly-appended items into loadCovers(), so fast second page load can't re-trigger downloads for covers that are already underway?
Refactor: optimize notes covers rendering
Problem
The homepage currently suffers from poor loading performance. The note list is not displayed until all cover images have finished downloading, causing significant delays — especially on slower connections.
Solution
Preview
Implementation Details
noteList.service.tsgetNoteList()now returns metadata only (cover downloads are excluded)loadCover()method for per-note asynchronous cover fetchinguseNoteList.tsloadCovers()which downloads all covers in parallel usingPromise.all()and updates each note reactively upon completionloadNextPage()triggersloadCovers()in the background after the list is rendered, ensuring a smooth user experiencePerformance Improvements
Tested on Linux in Firefox and Chrome
Home page loading (30 notes with covers):