Skip to content

web : infinite scroll for project and dataset lists#41

Open
surajmn1 wants to merge 2 commits intomainfrom
feat/infinite-scroll
Open

web : infinite scroll for project and dataset lists#41
surajmn1 wants to merge 2 commits intomainfrom
feat/infinite-scroll

Conversation

@surajmn1
Copy link
Copy Markdown
Contributor

@surajmn1 surajmn1 commented Dec 18, 2025

implements infinite scrolling for projects and datasets in home and project detail pages

Summary by CodeRabbit

  • New Features
    • Added infinite scrolling to the Projects page—items load automatically as you scroll down.
    • Added infinite scrolling to the Datasets page—items load automatically as you scroll down.
    • Loading indicators display when fetching additional items.

✏️ Tip: You can customize this high-level summary in your review settings.

@claude
Copy link
Copy Markdown

claude Bot commented Dec 18, 2025

Claude finished @surajmn1's task —— View job


Reviewing Infinite Scroll Implementation

Todo List

  • Read repository guidelines (CLAUDE.md)
  • Analyze the new intersection observer hook
  • Review query modifications for pagination
  • Review home page implementation (page.tsx)
  • Review project detail page implementation (projects/[projectId]/page.tsx)
  • Check for potential bugs and edge cases
  • Evaluate performance considerations
  • Assess security implications
  • Review code quality and best practices
  • Check test coverage
  • Provide comprehensive feedback

Code Review Summary

This PR implements infinite scrolling functionality for projects and datasets using a clean, well-structured approach. The implementation follows React best practices and integrates well with the existing TanStack Query architecture.

Strengths

Architecture & Design

  • Clean separation of concerns: Custom useIntersectionObserver hook isolates intersection logic
  • Consistent API patterns: Both useProjectsInfinite and useDatasetsInfinite follow identical patterns
  • Proper integration: Seamlessly integrates with existing TanStack Query setup
  • Performance-first: Uses Intersection Observer API for efficient scroll detection

Implementation Quality

  • Robust error handling: Proper null checks for page.results in pagination logic
  • Loading states: Clear visual feedback with spinner during fetching
  • Proper cleanup: Intersection observer disconnected in useEffect cleanup
  • Type safety: Full TypeScript support with proper generics

User Experience

  • Smooth UX: 0.1 threshold provides natural trigger point
  • Visual feedback: Loading spinner clearly indicates when more content is loading
  • Performance: 12 items per page provides good balance

⚠️ Issues Found

Critical Issues

1. Memory Leak Risk in Intersection Observer Hook (use-intersection-observer.ts:40)

}, [threshold, root, rootMargin, enabled]);

Issue: Missing ref.current in dependency array could cause stale closures
Impact: Potential memory leaks and incorrect behavior when element changes
Solution: Add ref.current to dependencies or use callback ref pattern

2. Race Condition in fetchNextPage (both pages)

React.useEffect(() => {
  if (inView && hasNextPage) {
    fetchNextPage();
  }
}, [inView, hasNextPage, fetchNextPage]);

Issue: No protection against rapid successive calls if fetchNextPage changes
Impact: Potential duplicate API requests
Solution: Add isFetchingNextPage check or debounce the effect

Medium Priority Issues

3. Inconsistent Error Handling (list-projects.ts:30, list-datasets.ts:41)

throw new Error("Failed to fetch projects: " + error);

Issue: String concatenation loses original error context and stack trace
Impact: Harder debugging, poor error reporting
Solution:

throw error instanceof Error ? error : new Error(`Failed to fetch projects: ${String(error)}`);

4. Hardcoded Pagination Logic (both query files)

if (results.length < limit) return undefined;

Issue: Assumes API behavior - might break if API changes
Impact: Brittle pagination logic
Solution: Use API-provided pagination metadata when available

5. Cache Key Inconsistency

  • Projects: ["projects", "infinite"]
  • Datasets: ["datasets", projectId, "infinite"]

Issue: Different patterns could cause cache conflicts
Impact: Potential cache invalidation issues
Solution: Standardize to ["projects", "infinite"] and ["datasets", "infinite", projectId]

🔒 Security Assessment

✅ No security vulnerabilities found

  • No exposed sensitive data
  • Proper input sanitization in API calls
  • No XSS vulnerabilities in rendering
  • Secure API endpoint usage

Performance Considerations

Excellent Performance Patterns

  • ✅ Intersection Observer is performance-optimal
  • ✅ Proper React Query caching reduces API calls
  • ✅ Efficient list rendering with unique keys
  • ✅ Reasonable page size (12 items)

Potential Optimizations

  1. Virtual scrolling: For very large datasets, consider react-window
  2. Prefetching: Could preload next page when approaching threshold
  3. Image loading: Add lazy loading for any images in cards

📋 Test Coverage Assessment

❌ Missing Test Coverage
The implementation lacks tests for critical functionality:

Recommended Tests:

// useIntersectionObserver hook tests
describe('useIntersectionObserver', () => {
  test('should trigger inView when element intersects')
  test('should cleanup observer on unmount')
  test('should handle disabled state')
})

// Infinite query tests  
describe('useProjectsInfinite', () => {
  test('should fetch next page correctly')
  test('should handle pagination end state')
  test('should handle API errors')
})

🔧 Recommendations

Immediate Fixes (Pre-merge)

  1. Fix intersection observer dependency array
  2. Add race condition protection in useEffect
  3. Improve error handling in query functions

Follow-up Improvements

  1. Add comprehensive test coverage
  2. Consider implementing loading skeletons during fetchNextPage
  3. Add retry logic for failed page loads
  4. Implement prefetching optimization

Code Quality Suggestions

// Better error handling pattern
const handleApiError = (error: unknown, context: string) => {
  console.error(`${context}:`, error);
  if (error instanceof Error) throw error;
  throw new Error(`${context}: ${String(error)}`);
};

// Debounced fetch effect
const debouncedFetchNextPage = useMemo(
  () => debounce(fetchNextPage, 100),
  [fetchNextPage]
);

📊 Overall Assessment

Score: 8/10

This is a solid implementation that follows modern React patterns and provides good user experience. The main concerns are around edge cases and missing tests rather than fundamental architectural issues.

Recommendation: ✅ Approve with minor fixes

The implementation is production-ready with the suggested fixes for the intersection observer and error handling.


@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 18, 2025

Walkthrough

This PR introduces infinite scrolling pagination across the projects and datasets pages. A new useIntersectionObserver hook detects when elements enter the viewport, triggering useInfiniteQuery-based hooks to fetch subsequent pages. Results are flattened and rendered incrementally with loading indicators.

Changes

Cohort / File(s) Summary
Infinite Query Hooks
web/src/lib/queries/project/list-projects.ts, web/src/lib/queries/dataset/list-datasets.ts
Added useProjectsInfinite and useDatasetsInfinite hooks using useInfiniteQuery to fetch paginated results with 12-item page size; getNextPageParam logic determines whether additional pages exist.
Intersection Observer Hook
web/src/hooks/use-intersection-observer.ts
New hook that wraps IntersectionObserver API, exposing ref and inView boolean to detect when elements enter viewport; supports threshold, root, rootMargin, and enabled options.
Projects Page
web/src/app/page.tsx
Migrated from useProjects to useProjectsInfinite(12); added infinite scroll triggering via useIntersectionObserver; flattened paginated results into single allProjects array; displays Loader2 spinner while fetching next page.
Datasets Page
web/src/app/projects/[projectId]/page.tsx
Migrated from useDatasets to useDatasetsInfinite(projectId, 12); integrated intersection observer for load-more sentinel; flattened datasetsData.pages into allDatasets; invalidates infinite query after dataset deletion; shows totalDatasets count.

Sequence Diagram

sequenceDiagram
    participant User
    participant Component as Page Component
    participant Observer as useIntersectionObserver
    participant RQ as React Query<br/>(useInfiniteQuery)
    participant API as Server API

    User->>Component: Scroll near load-more sentinel
    Component->>Observer: Observing loadMoreRef element
    Observer->>Observer: Check visibility threshold
    alt Sentinel in view
        Observer->>Component: inView = true
        Component->>Component: Effect triggered (inView && hasNextPage)
        Component->>RQ: fetchNextPage()
        RQ->>API: GET /datasets?page=2&limit=12
        API->>RQ: { results: [...], total: 45 }
        RQ->>Component: Update datasetsData.pages
        Component->>Component: Flatten pages → allDatasets
        Component->>Component: Render new DatasetCards
    else Sentinel not in view
        Observer->>Component: inView = false
        Component->>Component: Effect skipped
    end
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20–25 minutes

  • useIntersectionObserver hook: Verify observer cleanup on unmount and dependency array correctness (threshold, root, rootMargin, enabled)
  • getNextPageParam logic: Ensure pagination math is correct in both useDatasetsInfinite and useProjectsInfinite (checking lastPage.total and result length)
  • Data flattening: Confirm allProjects and allDatasets derivation from pages array handles edge cases (empty pages, single page)
  • Effect dependencies: Verify auto-fetch effects in both page components don't cause infinite loops or premature fetches
  • Query invalidation: Ensure dataset deletion properly invalidates the infinite query with correct key structure

Poem

🐰 Hop, scroll, and fetch with glee,
Pages load as far as we can see,
Observer watches, sentinel spins,
Infinite joy where the loading begins!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: implementing infinite scroll functionality for project and dataset lists, which aligns perfectly with the changeset's primary objective.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/infinite-scroll

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (4)
web/src/lib/queries/project/list-projects.ts (1)

39-57: Include limit in the query key to prevent stale cache issues.

If limit changes between calls (e.g., different components using different limits), React Query will return cached data for the wrong page size. Adding limit to the query key ensures proper cache separation.

Suggested fix
 export const useProjectsInfinite = (limit = 12) => {
   return useInfiniteQuery<PaginatedResponse<Project>, Error>({
-    queryKey: ["projects", "infinite"],
+    queryKey: ["projects", "infinite", limit],
     initialPageParam: 1,
     queryFn: ({ pageParam }) => 
       fetchProjects({ 
         limit, 
         page: pageParam as number 
       }),
web/src/lib/queries/dataset/list-datasets.ts (1)

50-71: Include limit in the query key for cache correctness.

Same issue as useProjectsInfinite — if limit varies, the cache could return data for a different page size.

Suggested fix
 export const useDatasetsInfinite = (projectId: string, limit = 12) => {
   return useInfiniteQuery<PaginatedResponse<Dataset>, Error>({
-    queryKey: ["datasets", projectId, "infinite"],
+    queryKey: ["datasets", projectId, "infinite", limit],
     initialPageParam: 1,
     enabled: !!projectId,
web/src/app/page.tsx (1)

367-380: Animation delay is constant; consider staggering for smoother UX.

The delay: 0.05 is applied uniformly to all items, so they animate simultaneously. For a staggered effect where items appear one after another, use idx * 0.05.

Suggested fix
                   {allProjects.map((project, idx) => (
                     <motion.div
                       key={project.id}
                       initial={{ opacity: 0, y: 20 }}
                       animate={{ opacity: 1, y: 0 }}
-                      transition={{ delay: 0.05 }}
+                      transition={{ delay: idx * 0.05 }}
                     >
web/src/app/projects/[projectId]/page.tsx (1)

176-189: Same animation delay issue — consider staggering.

Like the home page, the delay is constant. For consistency and improved UX, consider using idx * 0.05 for staggered animations.

Suggested fix
               {allDatasets.map((dataset, idx) => (
                 <motion.div
                   key={dataset.id}
                   initial={{ opacity: 0, y: 20 }}
                   animate={{ opacity: 1, y: 0 }}
-                  transition={{ delay: 0.05 }}
+                  transition={{ delay: idx * 0.05 }}
                 >
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7cce205 and 025bc50.

📒 Files selected for processing (5)
  • web/src/app/page.tsx (6 hunks)
  • web/src/app/projects/[projectId]/page.tsx (7 hunks)
  • web/src/hooks/use-intersection-observer.ts (1 hunks)
  • web/src/lib/queries/dataset/list-datasets.ts (2 hunks)
  • web/src/lib/queries/project/list-projects.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
web/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (web/CLAUDE.md)

web/src/**/*.{ts,tsx}: Use Next.js 15 with App Router and React 19 for the web frontend framework
Use TypeScript as the language for the web frontend
Generate and use types from API responses for type safety
Use dynamic imports and code splitting for optimal performance
Load DuckDB WASM on-demand and use web workers for client-side SQL execution

Files:

  • web/src/lib/queries/dataset/list-datasets.ts
  • web/src/hooks/use-intersection-observer.ts
  • web/src/lib/queries/project/list-projects.ts
  • web/src/app/page.tsx
  • web/src/app/projects/[projectId]/page.tsx
web/src/lib/{queries,mutations}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (web/CLAUDE.md)

Use React Query (TanStack Query) for server state management

Files:

  • web/src/lib/queries/dataset/list-datasets.ts
  • web/src/lib/queries/project/list-projects.ts
web/src/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (web/CLAUDE.md)

Protect routes using the ProtectedRoute wrapper HOC for authentication-required pages

Files:

  • web/src/app/page.tsx
  • web/src/app/projects/[projectId]/page.tsx
🧠 Learnings (7)
📚 Learning: 2025-12-02T11:48:50.411Z
Learnt from: CR
Repo: factly/gopie PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2025-12-02T11:48:50.411Z
Learning: Applies to web/src/components/dataset/**upload**.{ts,tsx} : Use Uppy for dataset file uploads in upload components

Applied to files:

  • web/src/lib/queries/dataset/list-datasets.ts
  • web/src/app/projects/[projectId]/page.tsx
📚 Learning: 2025-12-02T11:48:50.411Z
Learnt from: CR
Repo: factly/gopie PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2025-12-02T11:48:50.411Z
Learning: Applies to web/src/lib/{queries,mutations}/**/*.{ts,tsx} : Use React Query (TanStack Query) for server state management

Applied to files:

  • web/src/lib/queries/dataset/list-datasets.ts
  • web/src/lib/queries/project/list-projects.ts
  • web/src/app/page.tsx
  • web/src/app/projects/[projectId]/page.tsx
📚 Learning: 2025-12-02T11:48:50.411Z
Learnt from: CR
Repo: factly/gopie PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2025-12-02T11:48:50.411Z
Learning: Applies to web/src/components/**/*.{ts,tsx} : Use pre-configured queries and mutations from lib/queries and lib/mutations for API calls

Applied to files:

  • web/src/lib/queries/dataset/list-datasets.ts
  • web/src/lib/queries/project/list-projects.ts
  • web/src/app/page.tsx
  • web/src/app/projects/[projectId]/page.tsx
📚 Learning: 2025-12-02T11:48:50.411Z
Learnt from: CR
Repo: factly/gopie PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2025-12-02T11:48:50.411Z
Learning: Applies to web/src/**/*.{test,spec}.{ts,tsx} : Use React Testing Library for component testing

Applied to files:

  • web/src/hooks/use-intersection-observer.ts
📚 Learning: 2025-12-02T11:48:50.411Z
Learnt from: CR
Repo: factly/gopie PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2025-12-02T11:48:50.411Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use Next.js 15 with App Router and React 19 for the web frontend framework

Applied to files:

  • web/src/app/page.tsx
📚 Learning: 2025-07-29T09:36:39.908Z
Learnt from: CR
Repo: factly/gopie PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-29T09:36:39.908Z
Learning: Applies to web/**/*.{js,jsx,ts,tsx} : Use Zustand for global state management and React Query for server state in the Next.js frontend

Applied to files:

  • web/src/app/page.tsx
  • web/src/app/projects/[projectId]/page.tsx
📚 Learning: 2025-12-02T11:48:50.411Z
Learnt from: CR
Repo: factly/gopie PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2025-12-02T11:48:50.411Z
Learning: Applies to web/src/components/**/*.{ts,tsx} : Always handle loading and error states in UI components

Applied to files:

  • web/src/app/page.tsx
🧬 Code graph analysis (4)
web/src/lib/queries/dataset/list-datasets.ts (1)
web/src/lib/api-client.ts (1)
  • PaginatedResponse (144-149)
web/src/lib/queries/project/list-projects.ts (1)
web/src/lib/api-client.ts (1)
  • PaginatedResponse (144-149)
web/src/app/page.tsx (2)
web/src/lib/queries/project/list-projects.ts (1)
  • useProjectsInfinite (39-58)
web/src/hooks/use-intersection-observer.ts (1)
  • useIntersectionObserver (10-43)
web/src/app/projects/[projectId]/page.tsx (2)
web/src/lib/queries/dataset/list-datasets.ts (1)
  • useDatasetsInfinite (50-72)
web/src/hooks/use-intersection-observer.ts (1)
  • useIntersectionObserver (10-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: claude-review
  • GitHub Check: Analyze (go)
🔇 Additional comments (4)
web/src/hooks/use-intersection-observer.ts (1)

10-42: LGTM!

Clean implementation with proper cleanup. The observer is correctly disconnected and inView is reset on unmount/disable, preventing stale state.

web/src/app/page.tsx (1)

31-50: Infinite scroll integration looks good.

The combination of useProjectsInfinite, useIntersectionObserver, and the effect to trigger fetchNextPage is well-structured. Disabling the observer during fetches (enabled: hasNextPage && !isFetchingNextPage) prevents redundant API calls.

web/src/app/projects/[projectId]/page.tsx (2)

39-58: Infinite scroll integration is correct.

The datasets infinite query, intersection observer, and fetchNextPage effect follow the same solid pattern established in the home page. The enabled guard on the observer prevents unnecessary fetches.


67-70: Query invalidation key matches — good.

The invalidation uses ["datasets", projectId, "infinite"] which correctly matches the query key in useDatasetsInfinite. If you adopt the suggested fix to include limit in the query key, update this invalidation to match or use a partial match (predicate-based invalidation).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant