feat: add pagination to blog page#5123
Conversation
jayvaliya
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg,
princerajpoot20 and
sambhavgupta0705
as code owners
❌ Deploy Preview for asyncapi-website failed.Built without sensitive environment variables
|
📝 WalkthroughWalkthroughAdds client-side pagination: new BlogPagination component and type, integrates pagination into the blog index (slicing posts per page, page validation, shallow routing), resets page on filter changes, updates Filter to omit Changes
Sequence DiagramsequenceDiagram
actor User
participant Router as Next.js Router
participant Blog as Blog Page
participant Filter as Filter Component
participant Pagination as BlogPagination
rect rgba(100,150,200,0.5)
Note over User,Pagination: Page navigation flow
User->>Pagination: Click page number / Next / Prev
Pagination->>Blog: onPageChange(page)
Blog->>Router: router.push(url with updated page) (shallow)
Router->>Blog: router.query updated
Blog->>Blog: Recompute displayedPosts (apply filters → slice)
Blog->>Pagination: Render updated pagination state
Pagination->>User: Updated posts + pagination UI
end
rect rgba(150,100,200,0.5)
Note over User,Filter: Filter apply flow
User->>Filter: Apply filters
Filter->>Blog: onFilterApply(query without `page`)
Blog->>Router: router.push(filtered query) (shallow)
Router->>Blog: router.query updated
Blog->>Blog: Detect filter change, reset page → page=1, recompute displayedPosts
Blog->>Pagination: Render page 1 with filtered posts
Pagination->>User: Filtered posts shown from page 1
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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 |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-5123--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@pages/blog/index.tsx`:
- Line 85: Currently showClearFilters uses Object.keys(router.query) so a bare
pagination key like page makes the button appear; change the logic to ignore the
pagination key by computing the query minus 'page' (e.g., destructure { page,
...rest } = router.query or filter out 'page') and set showClearFilters =
Object.keys(rest).length > 0 (or filter out keys === 'page' and empty values).
Update the expression that defines showClearFilters to exclude 'page' so the
Clear filters button only shows when real filters are present.
- Around line 47-49: The eslint-disable-next-line comment is suppressing the
wrong line; move the directive so it targets the unused parameter `_query` in
the onFilter callback: adjust the comment placement for the useCallback/onFilter
declaration so it sits immediately before the parameter or convert it to an
inline disable on the `_query` parameter (affecting the onFilter function using
useCallback and the `_query: FilterType` parameter) to only suppress the
unused-var rule for that parameter.
🧹 Nitpick comments (2)
cypress/blog.cy.js (1)
1-67: Good baseline E2E coverage for the happy path.A few gaps worth noting for future improvement:
- No test for the last page — verifying that the Next button is disabled on the final page.
- No test for invalid page values (e.g.,
?page=999,?page=-1,?page=abc) — the PR objectives mention invalid-page handling/redirect, but it's not covered here.- No assertion on the number of rendered posts — e.g., verifying that exactly 12
BlogPostItemelements appear on a full page.- Tests like line 53 (
/blog?page=3) assume ≥ 36 posts exist; these will break if the post count drops below that threshold.None of these are blockers, but they'd strengthen confidence in the pagination logic.
pages/blog/index.tsx (1)
48-67: Filter-change detection viaJSON.stringifyis order-sensitive.
JSON.stringify(currentQuery)comparison (line 55 vsprevFiltersRef.current) can produce false positives if query-parameter order differs between navigations (e.g., user editing the URL bar). The only consequence is an unnecessary page reset to 1, so it's not critical — but using a sorted-keys comparison or shallow object equality would be more robust.
| @@ -58,13 +84,40 @@ export default function BlogIndexPage() { | |||
| }; | |||
| const showClearFilters = Object.keys(router.query).length > 0; | |||
There was a problem hiding this comment.
"Clear filters" button appears when only ?page=N is in the URL.
showClearFilters is true whenever router.query has any keys, including just page. A user on /blog?page=2 with no active filters will see "Clear filters" — clicking it navigates to /blog which resets pagination but is misleading since no filters are active.
Consider excluding page from the check:
Proposed fix
- const showClearFilters = Object.keys(router.query).length > 0;
+ const showClearFilters = Object.keys(router.query).some((key) => key !== 'page');📝 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.
| const showClearFilters = Object.keys(router.query).length > 0; | |
| const showClearFilters = Object.keys(router.query).some((key) => key !== 'page'); |
🤖 Prompt for AI Agents
In `@pages/blog/index.tsx` at line 85, Currently showClearFilters uses
Object.keys(router.query) so a bare pagination key like page makes the button
appear; change the logic to ignore the pagination key by computing the query
minus 'page' (e.g., destructure { page, ...rest } = router.query or filter out
'page') and set showClearFilters = Object.keys(rest).length > 0 (or filter out
keys === 'page' and empty values). Update the expression that defines
showClearFilters to exclude 'page' so the Clear filters button only shows when
real filters are present.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@pages/blog/index.tsx`:
- Around line 186-190: Replace the unstable array index key on BlogPostItem with
a stable identifier: change the key used in the map over displayedPosts from the
numeric index to post.slug (i.e., in the JSX where displayedPosts.map((post,
index) => <BlogPostItem key=... post={post} />) use key={post.slug}); ensure
post.slug is unique for each post and fallback to a stable id only if necessary
to prevent duplicate keys.
- Around line 52-62: The query construction is incorrectly casting router.query
to Record<string,string> which corrupts array-valued params; add a helper
function buildQueryString(obj: Record<string, string | string[] | undefined>)
that creates a URLSearchParams by iterating Object.entries and for each key if
the value is an array append each element, if it's a string append it, then
return params.toString(); replace the four problematic spots (the onFilter block
using currentQuery, the handlePageChange usage, the pagination validation that
builds a URL, and the usage in components/navigation/Filter.tsx around
newQuery/currentQuery) to call buildQueryString(currentQuery) or
buildQueryString(newQuery) instead of new URLSearchParams(... as
Record<string,string>).toString().
🧹 Nitpick comments (3)
pages/blog/index.tsx (3)
110-119: Page-validation effect hasrouterin its dependency array — risk of redundantrouter.replacecalls.The
routerobject reference can change on every render in the Next.js Pages Router. Combined withrouter.replaceitself updating the router, this effect may fire more than necessary. While therawPage !== validCurrentPageguard prevents an infinite loop, the extra evaluations are wasteful.Consider using
router.pathnameor extracting a stable reference viauseRefinstead of depending on the entirerouterobject. Alternatively, the eslint exhaustive-deps rule may have forced this — if so, a suppression comment explaining why would be appropriate here.
97-101:scrollIntoViewfires before the shallow route transition commits.
router.pushreturns aPromisethat resolves after the transition. CallingscrollIntoViewsynchronously afterward means it executes before React re-renders with the new page's content (display of new posts, pagination update). This usually "works" because the target element already exists, but if the content height changes significantly between pages, the scroll position may be off.Proposed fix — scroll after transition
- router.push(`${router.pathname}?${queryParams}`, undefined, { + router.push(`${router.pathname}?${queryParams}`, undefined, { shallow: true - }); - document.getElementById('main-content')?.scrollIntoView({ behavior: 'smooth' }); + }).then(() => { + document.getElementById('main-content')?.scrollIntoView({ behavior: 'smooth' }); + });
47-49: ESLint suppression lacks an explanatory comment.The
eslint-disableis correct but doesn't explain why the parameter is unused. A brief note helps future readers understand this is intentional (the_queryparameter is required by theFiltercomponent's callback signature but not needed here).- // eslint-disable-next-line `@typescript-eslint/no-unused-vars` + // eslint-disable-next-line `@typescript-eslint/no-unused-vars` -- _query is required by Filter's onFilter signature but unused hereBased on learnings, in the asyncapi/website repository, when you intentionally omit dependencies in React hooks, add an eslint-disable comment with an explanatory note to justify the design choice.
| const currentQuery = { ...router.query }; | ||
|
|
||
| delete currentQuery.page; | ||
| const currentFilters = JSON.stringify(currentQuery); | ||
|
|
||
| if (router.query.page && prevFiltersRef.current && currentFilters !== prevFiltersRef.current) { | ||
| const queryParams = new URLSearchParams(currentQuery as Record<string, string>).toString(); | ||
| const url = queryParams ? `${router.pathname}?${queryParams}` : router.pathname; | ||
|
|
||
| router.push(url, undefined, { shallow: true }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the file and check its structure
find . -name "index.tsx" -path "*/pages/blog/*" 2>/dev/nullRepository: asyncapi/website
Length of output: 412
🏁 Script executed:
# Get the file content around the specified lines
cat -n pages/blog/index.tsx | sed -n '45,70p'Repository: asyncapi/website
Length of output: 382
🏁 Script executed:
# Check for handlePageChange function and line 95 context
cat -n pages/blog/index.tsx | sed -n '90,105p'Repository: asyncapi/website
Length of output: 412
🏁 Script executed:
# Search for all URLSearchParams usages in the file
rg -n 'URLSearchParams' pages/blog/index.tsx -C 3Repository: asyncapi/website
Length of output: 390
🏁 Script executed:
# Check the router.query usage pattern
rg -n 'router\.query' pages/blog/index.tsx -C 2Repository: asyncapi/website
Length of output: 1699
🏁 Script executed:
# Search for filter-related code and how filters are built
rg -n 'onFilter|handleFilter|filter' pages/blog/index.tsx -A 5 -B 2Repository: asyncapi/website
Length of output: 1325
🏁 Script executed:
# Check if there's any multi-value query parameter usage
rg -n '\?.*=.*&.*=' pages/blog/index.tsx
rg -n 'Array\.isArray.*query' pages/blog/index.tsxRepository: asyncapi/website
Length of output: 42
🏁 Script executed:
# Search for query builder patterns
rg -n 'type.*=' pages/blog/index.tsxRepository: asyncapi/website
Length of output: 369
🏁 Script executed:
# Check the broader context of how filters are created and passed
cat -n pages/blog/index.tsx | sed -n '65,92p'Repository: asyncapi/website
Length of output: 929
🏁 Script executed:
# Verify if Next.js router types indicate string | string[]
rg -n 'NextRouter|ParsedUrlQuery|router:' pages/blog/index.tsx -B 2 -A 2Repository: asyncapi/website
Length of output: 42
🏁 Script executed:
# Check imports and type definitions
cat -n pages/blog/index.tsx | head -30Repository: asyncapi/website
Length of output: 1584
🏁 Script executed:
# Find the Filter component to see how it builds query parameters
find . -name "Filter*" -type f | grep -E "\.(tsx?|jsx?)$"Repository: asyncapi/website
Length of output: 320
🏁 Script executed:
# Check the applyFilter helper to understand filter structure
find . -name "*applyFilter*" -o -name "*filter*" | grep -E "helpers|utils" | head -10Repository: asyncapi/website
Length of output: 95
🏁 Script executed:
# Search for how query parameters are constructed with filters
rg -n 'router\.push.*query\|router\.replace.*query' -A 3 -B 3Repository: asyncapi/website
Length of output: 42
🏁 Script executed:
# Look for multi-valued query parameters in tests or documentation
rg -n 'type=.*&type=' pages/blog/
rg -n 'tags=.*&tags=' pages/blog/Repository: asyncapi/website
Length of output: 42
🏁 Script executed:
# Check how Filter component might add array values to query
rg -n 'append|push.*query' pages/ --type=tsx -A 2 -B 2Repository: asyncapi/website
Length of output: 87
🏁 Script executed:
# Create a test to verify URLSearchParams behavior
python3 << 'PYTHON_EOF'
from urllib.parse import urlencode, parse_qs
# Test 1: Direct object with array
test_obj = {'type': 'a', 'tags': ['tag1', 'tag2']}
print("Test 1: Object with array value")
print(f" Object: {test_obj}")
# When URLSearchParams gets an array as string, it calls toString()
import json
print(f" Array toString(): {json.dumps(test_obj['tags']).replace('"', '')}")
# Test 2: How to properly handle arrays
params_list = []
for key, value in test_obj.items():
if isinstance(value, list):
for v in value:
params_list.append((key, v))
else:
params_list.append((key, value))
print(f" Proper approach: {urlencode(params_list)}")
# Test 3: What happens with cast
print("\nTest 2: Simulating cast behavior")
# When you cast array to string in URLSearchParams context
stringified = str(test_obj['tags']) # This becomes "['tag1', 'tag2']"
print(f" Array stringified: {stringified}")
print(f" URL encoded: {urlencode([('tags', stringified)])}")
PYTHON_EOFRepository: asyncapi/website
Length of output: 308
🏁 Script executed:
# Check Next.js documentation on router.query to confirm array behavior
rg -n 'string\[\]|Array.*query|ParsedUrlQuery' --type=tsRepository: asyncapi/website
Length of output: 3901
🏁 Script executed:
# Check the Filter component to see how it constructs query params
cat -n components/navigation/Filter.tsxRepository: asyncapi/website
Length of output: 3399
🏁 Script executed:
# Check applyFilter helper
cat -n components/helpers/applyFilter.tsRepository: asyncapi/website
Length of output: 5824
🏁 Script executed:
# Look for how query is updated from Filter component
rg -n 'router\.push.*filter\|router\.query.*filter' components/navigation/ -B 3 -A 3Repository: asyncapi/website
Length of output: 42
🏁 Script executed:
# Verify actual URLSearchParams behavior with Python
python3 << 'PYTHON_EOF'
from urllib.parse import urlencode
# Test how URLSearchParams in JavaScript would handle an object with array
# Simulating JavaScript URLSearchParams behavior
test_cases = [
("Case 1: string value", {"type": "a"}),
("Case 2: array as string", {"type": ["a", "b"]}),
]
print("=== URLSearchParams behavior ===\n")
for label, obj in test_cases:
print(f"{label}")
print(f" Input: {obj}")
# This is what happens when JS calls toString() on array
if isinstance(obj["type"], list):
array_str = str(obj["type"]) # JavaScript: ['a', 'b'].toString() = 'a,b'
print(f" Array.toString() result: {array_str}")
# Then URLSearchParams would encode this
encoded = urlencode({"type": array_str})
print(f" URLSearchParams result: {encoded}")
print(f" (Incorrect - should be: type=a&type=b)\n")
else:
encoded = urlencode({"type": obj["type"]})
print(f" URLSearchParams result: {encoded}\n")
print("=== How to fix it ===")
print("When array values exist, append them separately:")
obj_with_array = {"type": ["a", "b"], "page": "1"}
params = []
for key, val in obj_with_array.items():
if isinstance(val, list):
for v in val:
params.append((key, v))
else:
params.append((key, val))
print(f" Result: {urlencode(params)}")
PYTHON_EOFRepository: asyncapi/website
Length of output: 463
🏁 Script executed:
# Check if the Filter component actually creates multi-valued query params
cat -n components/navigation/Filter.tsx | head -100Repository: asyncapi/website
Length of output: 3399
Fix URLSearchParams handling of array-valued query parameters in three locations.
router.query values can be string | string[] (e.g. ?type=a&type=b → { type: ['a','b'] }). The casts to Record<string, string> cause URLSearchParams to call .toString() on array values, producing corrupted query strings like type=%5B%27a%27%2C+%27b%27%5D instead of type=a&type=b.
This affects:
pages/blog/index.tsxline 52 (onFiltercallback)pages/blog/index.tsxline 95 (handlePageChange)pages/blog/index.tsxline 115 (pagination validation)components/navigation/Filter.tsxline 81
Create a helper to properly handle array values:
Proposed fix — helper to safely build query strings
+// Place near the top of the file or in a utils module
+function buildQueryString(query: Record<string, string | string[] | undefined>): string {
+ const params = new URLSearchParams();
+ for (const [key, value] of Object.entries(query)) {
+ if (value === undefined) continue;
+ if (Array.isArray(value)) {
+ value.forEach((v) => params.append(key, v));
+ } else {
+ params.append(key, value);
+ }
+ }
+ return params.toString();
+}Then replace all four instances of the problematic pattern with buildQueryString(currentQuery) or buildQueryString(newQuery) accordingly.
🤖 Prompt for AI Agents
In `@pages/blog/index.tsx` around lines 52 - 62, The query construction is
incorrectly casting router.query to Record<string,string> which corrupts
array-valued params; add a helper function buildQueryString(obj: Record<string,
string | string[] | undefined>) that creates a URLSearchParams by iterating
Object.entries and for each key if the value is an array append each element, if
it's a string append it, then return params.toString(); replace the four
problematic spots (the onFilter block using currentQuery, the handlePageChange
usage, the pagination validation that builds a URL, and the usage in
components/navigation/Filter.tsx around newQuery/currentQuery) to call
buildQueryString(currentQuery) or buildQueryString(newQuery) instead of new
URLSearchParams(... as Record<string,string>).toString().
| <ul className='mx-auto mt-12 grid max-w-lg gap-5 lg:max-w-none lg:grid-cols-3'> | ||
| {displayedPosts.map((post, index) => ( | ||
| <BlogPostItem key={index} post={post} /> | ||
| ))} | ||
| </ul> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find IBlogPost interface definition
fd . -type f -name "*.ts" -o -name "*.tsx" | xargs rg -l "interface IBlogPost"Repository: asyncapi/website
Length of output: 232
🏁 Script executed:
# Search for BlogPostItem component
fd . -type f -name "*.tsx" | xargs rg -l "BlogPostItem" | head -5Repository: asyncapi/website
Length of output: 437
🏁 Script executed:
# Check post.slug usage to see if it's a stable identifier
rg "post\.slug" --type ts --type tsx -B2 -A2Repository: asyncapi/website
Length of output: 87
🏁 Script executed:
# Examine the BlogPostItem component for nested anchor tags
fd -name "BlogPostItem.tsx" -exec cat {} \;Repository: asyncapi/website
Length of output: 290
🏁 Script executed:
# Find IBlogPost interface
rg "interface IBlogPost" -A 20Repository: asyncapi/website
Length of output: 42
🏁 Script executed:
# Read BlogPostItem.tsx
cat components/navigation/BlogPostItem.tsxRepository: asyncapi/website
Length of output: 5868
🏁 Script executed:
# Check pages/blog/index.tsx for context around line 186-190
sed -n '180,195p' pages/blog/index.tsxRepository: asyncapi/website
Length of output: 773
🏁 Script executed:
# Search for where post.slug is used to verify it exists
rg "\.slug" | head -20Repository: asyncapi/website
Length of output: 2111
Replace array index with post.slug as the React key in pagination.
Using array indices as keys breaks reconciliation when navigating between pages. When the user moves from page 1 to page 2, displayedPosts contains entirely different posts, but React reuses keys 0–11, attempting to patch existing DOM nodes instead of remounting the components. This causes stale content and styling issues. Use the stable post.slug identifier instead.
Proposed fix
- {displayedPosts.map((post, index) => (
- <BlogPostItem key={index} post={post} />
+ {displayedPosts.map((post) => (
+ <BlogPostItem key={post.slug} post={post} />
))}📝 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.
| <ul className='mx-auto mt-12 grid max-w-lg gap-5 lg:max-w-none lg:grid-cols-3'> | |
| {displayedPosts.map((post, index) => ( | |
| <BlogPostItem key={index} post={post} /> | |
| ))} | |
| </ul> | |
| <ul className='mx-auto mt-12 grid max-w-lg gap-5 lg:max-w-none lg:grid-cols-3'> | |
| {displayedPosts.map((post) => ( | |
| <BlogPostItem key={post.slug} post={post} /> | |
| ))} | |
| </ul> |
🤖 Prompt for AI Agents
In `@pages/blog/index.tsx` around lines 186 - 190, Replace the unstable array
index key on BlogPostItem with a stable identifier: change the key used in the
map over displayedPosts from the numeric index to post.slug (i.e., in the JSX
where displayedPosts.map((post, index) => <BlogPostItem key=... post={post} />)
use key={post.slug}); ensure post.slug is unique for each post and fallback to a
stable id only if necessary to prevent duplicate keys.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5123 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 796 796
Branches 146 146
=========================================
Hits 796 796 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jayvaliya
force-pushed
the
feat/4938-blog-pagination
branch
from
b0b24ad to
5fa7356
Compare
- Add BlogPaginationProps type definitions - Create reusable pagination component with: - Previous/Next navigation buttons - Page number buttons with ellipsis for large counts - 'Showing X to Y of Z posts' text - Accessibility attributes (aria-label, aria-current) - Test IDs for E2E testing
- Add POSTS_PER_PAGE constant (12 posts) - Extract current page from URL query params - Calculate pagination values from filtered posts - Add handlePageChange for URL-based navigation - Redirect invalid page numbers to valid pages - Reset page to 1 when filters change - Scroll to top when changing pages
Prevents pagination query param from being treated as a filter,which would cause empty results when navigating directly to apaginated URL
- Test pagination controls visibility - Test page navigation via buttons and page numbers - Test Previous/Next button disabled states - Test scroll to top behavior - Test direct URL navigation - Test filter + pagination URL params - Test Clear filters functionality
jayvaliya
force-pushed
the
feat/4938-blog-pagination
branch
from
5fa7356 to
c96ddfc
Compare
|
2 participants
Description
This PR implements client-side pagination for the blog page, addressing issue #4938. The implementation displays 12 posts per page with URL-based state management for shareability and browser history support.
Features Added
/blog?page=2) for shareable linksRelated issue(s)
Resolves #4938
🚀 Performance Improvements
This PR delivers significant performance gains by reducing the number of DOM nodes rendered at once:
⚡ Load Time Performance
🎨 Rendering Performance
💻 JavaScript Efficiency
🌳 DOM Optimization
📈 Before/After Comparison
🎯 User Experience Impact
📸 Performance Screenshots
Before (without pagination)
After (with pagination)
✅ Testing
Summary by CodeRabbit
New Features
Tests