Pagination for list repositories #2654
Conversation
Console (appwrite/console)Project ID: Sites (1)
Tip Silent mode disables those chatty PR comments if you prefer peace and quiet |
WalkthroughThis PR updates the Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/components/git/repositories.svelte (1)
196-198: Disabled state prevents searching when API returns zero results on initial load.The condition
disabled={!search && !$repositories?.repositories?.length}blocks the search input when users haven't typed anything and the repositories array is empty. In the edge case where an installation has no repositories (or the initial query returns zero results), users cannot type to search. Other similar components in the codebase usedisabled={totalResults === 0 && !search}, which is clearer and uses the explicit.totalproperty already available in$repositories. Consider usingdisabled={$repositories.total === 0 && !search}for consistency, or remove the disabled condition entirely since searching an empty result set is still useful for UX.
🧹 Nitpick comments (1)
src/lib/components/git/repositories.svelte (1)
70-84: Pagination reset logic looks correct.The
loadRepositoryPagefunction handles pagination clicks, while the$effecthandles search/installation changes with offset reset. One minor note: the comment on line 81 says "when search changes" but this also resets on installation changes—which is the correct behavior (new context = page 1). Consider updating the comment to clarify.$effect(() => { if (selectedInstallation && search !== undefined) { - offset = 0; // reset offset to 0 when search changes + offset = 0; // reset to first page when search or installation changes debouncedLoadRepositories(selectedInstallation, search); } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
package.json(1 hunks)src/lib/components/git/repositories.svelte(7 hunks)src/lib/components/git/skeletonRepoList.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/functions/function-[function]/store.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/store.tssrc/lib/components/git/skeletonRepoList.sveltesrc/lib/components/git/repositories.svelte
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Define types inline or in .d.ts files, avoid creating separate .types.ts files
Use TypeScript in non-strict mode; any type is tolerated in this project
Files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/store.ts
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/store.tssrc/lib/components/git/skeletonRepoList.sveltesrc/lib/components/git/repositories.sveltepackage.json
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/store.ts
src/lib/components/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for component file names and place them in src/lib/components/[feature]/ directory structure
Files:
src/lib/components/git/skeletonRepoList.sveltesrc/lib/components/git/repositories.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/lib/components/git/skeletonRepoList.sveltesrc/lib/components/git/repositories.svelte
🧠 Learnings (2)
📚 Learning: 2025-10-13T05:13:54.542Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/table.svelte:33-39
Timestamp: 2025-10-13T05:13:54.542Z
Learning: In Svelte 5, `import { page } from '$app/state'` provides a reactive state proxy that can be accessed directly (e.g., `page.params`), unlike the older `import { page } from '$app/stores'` which returns a readable store requiring the `$page` syntax for auto-subscription in components.
Applied to files:
src/lib/components/git/repositories.svelte
📚 Learning: 2025-09-26T06:48:57.938Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:629-631
Timestamp: 2025-09-26T06:48:57.938Z
Learning: In the Appwrite console codebase using appwrite.io/pink-svelte, the Icon component automatically handles CSS variable names passed to its color prop by internally wrapping them with var(). Therefore, passing '--some-css-variable' as a string to the Icon color prop works correctly without needing to manually wrap it with var().
Applied to files:
src/lib/components/git/repositories.sveltepackage.json
🔇 Additional comments (5)
package.json (1)
25-25: LGTM - dependency update enables pagination Query support.The commit-hash reference ensures reproducible builds. Verify the package provides the expected
Queryexport andProviderRepositoryRuntimeList/ProviderRepositoryFrameworkListtypes used in the other changed files.src/routes/(console)/project-[region]-[project]/functions/function-[function]/store.ts (1)
17-27: LGTM - Store type properly extended for pagination.The
totalfield and union type for repositories correctly support the pagination feature and multi-product usage (functions/sites).src/lib/components/git/skeletonRepoList.svelte (1)
4-8: LGTM - Dynamic skeleton count properly implemented.The Svelte 5
$props()syntax is correct. Minor note:Array(count)creates a sparse array which works here but{#each Array(count).fill(null) as _}or{#each { length: count } as _}would be slightly more explicit. Not blocking.src/lib/components/git/repositories.svelte (2)
117-143: Pagination queries and type casting implemented correctly.The
Query.limit()andQuery.offset()calls properly paginate results, and capturingtotalenables proper pagination UI. Theas unknown ascasts are acceptable given the TODO note about backend fixes pending.
301-318: Pagination footer implementation is solid.Showing the total results count and
PaginationInlinecomponent with proper bindings provides good UX. The conditional rendering (isLoadingRepositories || $repositories.total > 0) ensures the pagination area maintains layout during loading.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/lib/components/git/repositories.svelte (4)
70-77: Consider extracting shared loading logic.
loadRepositoryPageand the callback indebouncedLoadRepositories(lines 59-65) share identical loading-state management. You could extract this into a helper:+ const withLoading = async (fn: () => Promise<void>) => { + isLoadingRepositories = true; + try { + await fn(); + } finally { + isLoadingRepositories = false; + } + }; + const debouncedLoadRepositories = debounce( - async (installationId: string, searchTerm: string) => { - isLoadingRepositories = true; - try { - await loadRepositories(installationId, searchTerm); - } finally { - isLoadingRepositories = false; - } - }, + (installationId: string, searchTerm: string) => + withLoading(() => loadRepositories(installationId, searchTerm)), 300 ); - const loadRepositoryPage = async () => { - isLoadingRepositories = true; - try { - await loadRepositories(selectedInstallation, search); - } finally { - isLoadingRepositories = false; - } - }; + const loadRepositoryPage = () => + withLoading(() => loadRepositories(selectedInstallation, search));
128-134: Type assertions bypass type safety.The double cast
as unknown asindicates a type mismatch between the SDK response and expected types. While the TODO acknowledges this, consider adding runtime validation to catch unexpected shapes:$repositories.repositories = product === 'functions' - ? (result as unknown as Models.ProviderRepositoryRuntimeList) - .runtimeProviderRepositories - : (result as unknown as Models.ProviderRepositoryFrameworkList) - .frameworkProviderRepositories; //TODO: remove forced cast after backend fixes + ? (result as unknown as Models.ProviderRepositoryRuntimeList) + .runtimeProviderRepositories ?? [] + : (result as unknown as Models.ProviderRepositoryFrameworkList) + .frameworkProviderRepositories ?? []; //TODO: remove forced cast after backend fixesThe
?? []fallback guards against undefined if the backend response structure differs from expectations.
296-312: Stale total displayed during loading.When
isLoadingRepositoriesis true,$repositories.totalreflects the previous query's count. This could briefly show an incorrect total (e.g., "Total results: 50" while loading a new search that yields 3 results).Consider conditionally hiding the total during loading or showing a placeholder:
<Typography.Text variant="m-400" color="--fgcolor-neutral-secondary"> - Total results: {$repositories.total} + Total results: {isLoadingRepositories ? '...' : $repositories.total} </Typography.Text>
140-140: Unclear purpose of bare expression.The standalone
selectedRepository;statement appears to be dead code or a reactivity workaround. If it's intentional (e.g., to satisfy a linter or trigger reactive tracking), add a comment explaining why. Otherwise, remove it.- selectedRepository; + // Intentional: reference to maintain reactivity binding + selectedRepository;Or simply remove if not needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/components/git/repositories.svelte(7 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/lib/components/git/repositories.svelte
src/lib/components/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for component file names and place them in src/lib/components/[feature]/ directory structure
Files:
src/lib/components/git/repositories.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/lib/components/git/repositories.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/lib/components/git/repositories.svelte
🧠 Learnings (4)
📚 Learning: 2025-10-13T05:13:54.542Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/table.svelte:33-39
Timestamp: 2025-10-13T05:13:54.542Z
Learning: In Svelte 5, `import { page } from '$app/state'` provides a reactive state proxy that can be accessed directly (e.g., `page.params`), unlike the older `import { page } from '$app/stores'` which returns a readable store requiring the `$page` syntax for auto-subscription in components.
Applied to files:
src/lib/components/git/repositories.svelte
📚 Learning: 2025-10-05T09:41:40.439Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2398
File: src/routes/(console)/verify-email/+page.svelte:48-51
Timestamp: 2025-10-05T09:41:40.439Z
Learning: In SvelteKit 5, `page` imported from `$app/state` is a reactive state object (using runes), not a store. It should be accessed as `page.data` without the `$` prefix, unlike the store-based `$page` from `$app/stores` in earlier versions.
Applied to files:
src/lib/components/git/repositories.svelte
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to **/*.{ts,tsx,js,jsx,svelte} : Import reusable modules from the src/lib directory using the $lib alias
Applied to files:
src/lib/components/git/repositories.svelte
📚 Learning: 2025-09-26T06:48:57.938Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:629-631
Timestamp: 2025-09-26T06:48:57.938Z
Learning: In the Appwrite console codebase using appwrite.io/pink-svelte, the Icon component automatically handles CSS variable names passed to its color prop by internally wrapping them with var(). Therefore, passing '--some-css-variable' as a string to the Icon color prop works correctly without needing to manually wrap it with var().
Applied to files:
src/lib/components/git/repositories.svelte
🔇 Additional comments (4)
src/lib/components/git/repositories.svelte (4)
2-2: LGTM!Imports are properly organized and use the
$libalias as per coding guidelines.Also applies to: 20-20
51-52: LGTM!Correctly uses
$statefor the mutableoffsetandconstfor the immutablelimit.
79-84: LGTM!Correctly resets
offsetto 0 when the search term or installation changes, ensuring pagination starts from the first page.
198-277: LGTM!The table rendering correctly handles both product types with proper defensive checks (
'framework' in repo,'runtime' in repo) before accessing type-specific properties. The skeleton count matchinglimitprovides consistent loading UX.
Closes SER-504
Screenshot
Loading
Loaded
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.