Uh oh!
There was an error while loading. Please reload this page.
Fix/loading issue - #1007
Merged
Merged
Conversation
…nd changed the query on the home page to get the count only instead of all the images in the personal documents as not required.
…red. Request data that you need instead of getting all data and then throwing it away.
Contributor
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds IndexedDB photo caching, splits personal binding loaders (parameters, security question, photographs) with a skip-photo-blobs mode, narrows binding queries, and updates main/personal routes to hydrate progressively with skeleton UI and non-blocking parallel loads. ChangesProgressive Photo Loading and Skeleton UI for eID Wallet
Sequence Diagram(s)sequenceDiagram
participant User
participant MainPage
participant PersonalBinding
participant SocialBinding
participant PhotoCache
participant VaultAPI
User->>MainPage: Navigate to /main (login)
MainPage->>MainPage: Init bindingDocsLoading = true, show skeleton
par Parallel hydration
MainPage->>SocialBinding: fetchNameFromVault(nameOnly=true)
MainPage->>PersonalBinding: loadPersonalBindings(skipPhotoBlobs=true)
end
PersonalBinding->>PhotoCache: getCachedPhotosForEname() (immediate hydration)
PhotoCache-->>PersonalBinding: Cached photos
PersonalBinding->>MainPage: Update personal store with cached data
par Parallel network loads
SocialBinding->>VaultAPI: Fetch id_document, self binding docs
PersonalBinding->>VaultAPI: Fetch parameters, security_question, photograph ids
end
VaultAPI-->>SocialBinding: binding edges
VaultAPI-->>PersonalBinding: personal edges
PersonalBinding->>PhotoCache: Write server photos to cache
MainPage->>MainPage: bindingDocsLoading = false, render content
User->>MainPage: View personal/binding data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ly as they are small.
…click until data integrity is ensured.
Sahil2004
marked this pull request as ready for review
June 5, 2026 08:20
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
infrastructure/eid-wallet/src/lib/utils/photoCache.ts (2)
82-93: ⚡ Quick winPartial-write risk in atomic cache replacement.
replaceAllCachedPhotosclears the cache and then writes all photos viaPromise.all. If any singlesetItemcall fails mid-flight, the outercatchaborts the entire operation, leaving the cache cleared but only partially refilled.While the cache is non-fatal and the network remains the source of truth, a partial write means users see incomplete cached data (longer skeleton screens) on the next visit until the network fetch completes.
🛡️ Suggested improvement
Use
Promise.allSettledinstead ofPromise.allso individual write failures don't abort the batch:export async function replaceAllCachedPhotos( photos: CachedPhoto[], ): Promise<void> { try { await store.clear(); - await Promise.all(+ const results = await Promise.allSettled( photos.map((p) => store.setItem(p.metaEnvelopeId, p)), ); + const failures = results.filter((r) => r.status === "rejected");+ if (failures.length > 0) {+ console.warn(`[photoCache] ${failures.length} photo(s) failed to cache`);+ } } catch { // non-fatal } }This allows partial success—most photos are cached even if a few writes fail.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@infrastructure/eid-wallet/src/lib/utils/photoCache.ts` around lines 82 - 93, The current replaceAllCachedPhotos implementation clears the store then uses Promise.all for store.setItem calls which can leave the cache partially refilled if one write fails; change the writes to use Promise.allSettled so individual setItem failures don't reject the whole batch (call store.clear(), then await Promise.allSettled(photos.map(p => store.setItem(p.metaEnvelopeId, p))) and optionally inspect results to log or ignore rejected entries) while keeping the outer try/catch non-fatal.
43-48: ⚡ Quick winConsider optimizing photo retrieval by ename.
getCachedPhotosForEnameloads all cached photos viagetAllCachedPhotos(), then filters byenamein memory. If the cache accumulates many photos across multiple users, this becomes inefficient.Consider using localforage's
iteratedirectly with early-exit or adding an IndexedDB index onenamefor O(log n) lookup instead of O(n) scan.⚡ Suggested optimization
export async function getCachedPhotosForEname( ename: string, ): Promise<CachedPhoto[]> { - const all = await getAllCachedPhotos();- return all.filter((p) => p.ename === ename);+ try {+ const photos: CachedPhoto[] = [];+ await store.iterate<CachedPhoto, void>((value) => {+ if (value.ename === ename) {+ photos.push(value);+ }+ });+ return photos;+ } catch {+ return [];+ } }This avoids loading unrelated photos into memory. For a more robust solution, consider creating a localforage instance with an IndexedDB index on
ename.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@infrastructure/eid-wallet/src/lib/utils/photoCache.ts` around lines 43 - 48, The current getCachedPhotosForEname function loads everything via getAllCachedPhotos then filters in memory; change it to iterate the underlying store (e.g., localforage.iterate) and collect only entries whose p.ename === ename, returning early if you can satisfy any expected limit, so you avoid O(n) memory allocation; alternatively create a separate localforage instance or IndexedDB index keyed by ename and query that store instead of calling getAllCachedPhotos — update the implementation of getCachedPhotosForEname to use localforage.iterate (or the new indexed store) and remove the getAllCachedPhotos-dependent filtering.infrastructure/eid-wallet/src/routes/(app)/main/+page.svelte (1)
155-204: ⚡ Quick winConsider using the
vaultGqlRequesthelper for consistency.
loadBindingDocumentsconstructs inlinefetch()calls with manual header building and query strings (lines 155-176). The codebase already provides avaultGqlRequesthelper (imported from$lib/utilsat line 42) that handles headers, error parsing, and auth tokens.Using the helper would reduce duplication and keep the GraphQL request pattern consistent across the eID wallet module.
♻️ Suggested refactor
Replace the inline fetch with
vaultGqlRequest:+ import { vaultGqlRequest } from "$lib/utils/socialBinding";++ const BINDING_DOCS_BY_TYPE_QUERY = `+ query($type: BindingDocumentType!) {+ bindingDocuments(type: $type, first: 50) {+ edges { node { id parsed } }+ }+ }+ `;+ async function loadBindingDocuments(): Promise<void> { if (!globalState) return; const vault = await globalState.vaultController.vault; if (!vault?.uri || !vault?.ename) return; - const enameHeader = vault.ename.startsWith("@")- ? vault.ename- : `@${vault.ename}`; const gqlUrl = new URL("/graphql", vault.uri).toString(); -- const headers: Record<string, string> = {- "Content-Type": "application/json",- "X-ENAME": enameHeader,- ...(PUBLIC_EID_WALLET_TOKEN- ? { Authorization: `Bearer ${PUBLIC_EID_WALLET_TOKEN}` }- : {}),- };- const typedQuery = (type: string) =>- JSON.stringify({- query: `query($type: BindingDocumentType!) {- bindingDocuments(type: $type, first: 50) {- edges { node { id parsed } }- }- }`,- variables: { type },- });+ const ownerEname = vault.ename; try { const [idDocRes, selfRes] = await Promise.all([ - fetch(gqlUrl, { method: "POST", headers, body: typedQuery("id_document") }),- fetch(gqlUrl, { method: "POST", headers, body: typedQuery("self") }),+ vaultGqlRequest(gqlUrl, ownerEname, BINDING_DOCS_BY_TYPE_QUERY, { type: "id_document" }),+ vaultGqlRequest(gqlUrl, ownerEname, BINDING_DOCS_BY_TYPE_QUERY, { type: "self" }), ]); - const parseEdges = async (res: Response) => {- const json = await res.json();- return (json?.data?.bindingDocuments?.edges ?? []) as {- node: { id: string; parsed: ParsedBindingDoc | null };- }[];- };-- const [idDocEdges, selfEdges] = await Promise.all([- parseEdges(idDocRes),- parseEdges(selfRes),- ]);+ const idDocEdges = idDocRes.bindingDocuments?.edges ?? [];+ const selfEdges = selfRes.bindingDocuments?.edges ?? []; const idDocEntry = idDocEdges.find((e) => e.node.parsed?.type === "id_document"); legalId = idDocEntry?.node.parsed ? toLegalIdDoc(idDocEntry.node.parsed) : null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@infrastructure/eid-wallet/src/routes/`(app)/main/+page.svelte around lines 155 - 204, Replace the two manual fetch calls and header construction in loadBindingDocuments with the existing vaultGqlRequest helper: stop building headers and Authorization using PUBLIC_EID_WALLET_TOKEN and instead call vaultGqlRequest(gqlUrl, { query: ..., variables: { type } }) for the "id_document" and "self" queries (reuse the existing typedQuery string/variables shape), then feed the returned Response/JSON into the existing parseEdges logic (or adapt parseEdges to accept the vaultGqlRequest result). This keeps auth/error handling consistent and removes the inline fetch/header duplication (references: vaultGqlRequest, typedQuery, parseEdges, idDocEntry/selfEntry).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@infrastructure/eid-wallet/src/lib/utils/photoCache.ts`:
- Around line 82-93: The current replaceAllCachedPhotos implementation clears
the store then uses Promise.all for store.setItem calls which can leave the
cache partially refilled if one write fails; change the writes to use
Promise.allSettled so individual setItem failures don't reject the whole batch
(call store.clear(), then await Promise.allSettled(photos.map(p =>
store.setItem(p.metaEnvelopeId, p))) and optionally inspect results to log or
ignore rejected entries) while keeping the outer try/catch non-fatal.
- Around line 43-48: The current getCachedPhotosForEname function loads
everything via getAllCachedPhotos then filters in memory; change it to iterate
the underlying store (e.g., localforage.iterate) and collect only entries whose
p.ename === ename, returning early if you can satisfy any expected limit, so you
avoid O(n) memory allocation; alternatively create a separate localforage
instance or IndexedDB index keyed by ename and query that store instead of
calling getAllCachedPhotos — update the implementation of
getCachedPhotosForEname to use localforage.iterate (or the new indexed store)
and remove the getAllCachedPhotos-dependent filtering.
In `@infrastructure/eid-wallet/src/routes/`(app)/main/+page.svelte:
- Around line 155-204: Replace the two manual fetch calls and header
construction in loadBindingDocuments with the existing vaultGqlRequest helper:
stop building headers and Authorization using PUBLIC_EID_WALLET_TOKEN and
instead call vaultGqlRequest(gqlUrl, { query: ..., variables: { type } }) for
the "id_document" and "self" queries (reuse the existing typedQuery
string/variables shape), then feed the returned Response/JSON into the existing
parseEdges logic (or adapt parseEdges to accept the vaultGqlRequest result).
This keeps auth/error handling consistent and removes the inline fetch/header
duplication (references: vaultGqlRequest, typedQuery, parseEdges,
idDocEntry/selfEntry).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f547d467-77a0-42d7-9d93-b0263d25b9a9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
infrastructure/eid-wallet/package.jsoninfrastructure/eid-wallet/src/lib/utils/personalBinding.tsinfrastructure/eid-wallet/src/lib/utils/photoCache.tsinfrastructure/eid-wallet/src/lib/utils/socialBinding.tsinfrastructure/eid-wallet/src/routes/(app)/main/+page.svelteinfrastructure/eid-wallet/src/routes/(app)/personal/+page.svelteinfrastructure/eid-wallet/src/routes/(app)/settings/+page.svelte
coodos
approved these changes
Jun 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of change
Fixed the loading issue. Removed all the unecessary loading of huge data and then stripping it at frontend in favour of getting only that data which is required at frontend. Implemented skeletons for places where data is loading asynchronously. Implemented caching using LocalForge. Fixed the ordering issue when creating, editing or deleting anything. Fixed inter-user cache data leak. Brought down the image loading from several minutes to around 30s.
Issue Number
Closes#1006
Type of change
How the change has been tested
Manually.
Change checklist
Summary by CodeRabbit
New Features
Improvements