Development#53
Conversation
Bumps [axios](https://github.com/axios/axios) from 1.11.0 to 1.12.0. - [Release notes](https://github.com/axios/axios/releases) - [Changelog](https://github.com/axios/axios/blob/v1.x/CHANGELOG.md) - [Commits](axios/axios@v1.11.0...v1.12.0) --- updated-dependencies: - dependency-name: axios dependency-version: 1.12.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
Introduce Comment table with content, timestamps, visibility flags, and foreign keys to Novel and User. Add indexes for efficient querying by novel, user, and creation date. Update relations to link comments to novels and users. Minor cleanup in existing migrations and novel page import.
…s-1.12.0 chore(deps): bump axios from 1.11.0 to 1.12.0
Bumps [next](https://github.com/vercel/next.js) from 15.4.3 to 15.5.3. - [Release notes](https://github.com/vercel/next.js/releases) - [Changelog](https://github.com/vercel/next.js/blob/canary/release.js) - [Commits](vercel/next.js@v15.4.3...v15.5.3) --- updated-dependencies: - dependency-name: next dependency-version: 15.5.3 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
…-15.5.3 chore(deps): bump next from 15.4.3 to 15.5.3
- Add revalidate export to static MDX pages for consistent caching - Implement server-side and client-side post list components - Refactor home page to use new server-side post list component - Add memoized chapter and volume list components for improved performance - Introduce analytics and Google Adsense wrapper components - Update middleware and layout configurations - Add role-based caching and database migration scripts - Improve page load times and reduce unnecessary re-renders
WalkthroughAdded a caching framework and cached DB query wrappers, per-page revalidate exports, server/client post-list components, a KV-backed role cache, Comment table and DB indexes/snapshots, middleware role checks and legacy redirects, analytics/ads wrappers with an error boundary, several UI/component refactors, and dependency bumps. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant S as NextServer
participant Page as PostListServer
participant Cache as createFrequentCachedQuery
participant DB as Database
participant C as PostListClient
U->>S: GET /home
S->>Page: render PostListServer(premium?)
Page->>Cache: getCachedReleases({skip:0,premium})
alt cache hit
Cache-->>Page: cached chapters
else cache miss
Cache->>DB: _getReleases(...)
DB-->>Cache: chapters
Cache-->>Page: chapters
end
Page->>C: render with initialChapters
C-->>U: interactive UI (load more, toasts)
sequenceDiagram
autonumber
participant Req as Request
participant M as middleware
participant Map as chapterRedirectMap
participant Auth as AuthService
participant Role as getCachedUserRole
participant KV as KVStore
participant DB as Database
participant Resp as NextResponse
Req->>M: incoming request
M->>Map: check legacy chapter id
alt redirect found
M-->>Resp: redirect to /novels/[slug]/[chapter]
else not found
M->>M: is route protected?
alt public
M-->>Resp: Next()
else protected
M->>Auth: get userId/email
alt no user
M-->>Resp: redirect to sign-in
else user present
M->>Role: getCachedUserRole(userId)
Role->>KV: kv.get(key)
alt cache miss
Role->>DB: getUserRoleFromDb(userId)
DB-->>Role: role
Role->>KV: kv.set(key, role)
end
alt role == ADMIN
M-->>Resp: Next()
else
M-->>Resp: redirect to /
end
end
end
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/lib/db/0000_yielding_maelstrom.sql (1)
3-7: Critical: Modifying an already-applied migration creates schema drift.According to
src/lib/db/meta/_journal.json, this migration (idx 0, tag "0000_yielding_maelstrom") was already applied at timestamp 1725722358811. Uncommenting the Role enum creation block now will not retroactively apply it to databases where this migration already ran, causing schema inconsistency between environments.If the Role enum needs to be created (or was missing from the original migration), create a new migration file instead:
-- src/lib/db/0006_new_migration_name.sql DO $$ BEGIN CREATE TYPE "public"."Role" AS ENUM('ADMIN', 'SUBSCRIBER', 'MEMBER'); EXCEPTION WHEN duplicate_object THEN null; END $$;This ensures the enum creation is properly tracked and applied consistently across all environments.
src/app/(main)/novels/[slug]/page.tsx (1)
151-155: Broken "Read Now" link (relative path)href is relative and repeats the slug. Use an absolute path or only the chapter segment.
- <Link href={`${paramsResolved.slug}/${chapters[0].slug}`}> + <Link href={`/novels/${paramsResolved.slug}/${chapters[0].slug}`}>src/lib/db/query.ts (1)
316-334: Query builder not reassigned; filters/orderBy are ignoredDrizzle builders are immutable;
where/orderByreturn new builders. The current code never uses them.Fix:
- const data = db.select({ + let query = db.select({ id: chapter.id, serial: chapter.serial, slug: chapter.slug, number: chapter.number, title: chapter.title, publishedAt: chapter.publishedAt, premium: chapter.premium, novel: { title: novel.title, id: novel.id, slug: novel.slug, } - }).from(chapter).innerJoin(novel, eq(chapter.novelId, novel.id)) - if (novelId) data.where(eq(chapter.novelId, novelId)).orderBy(desc(chapter.serial)) - else data.orderBy(desc(chapter.createdAt)) - - return await data.limit(limit).offset(skip); + }).from(chapter).innerJoin(novel, eq(chapter.novelId, novel.id)); + if (novelId) { + query = query.where(eq(chapter.novelId, novelId)).orderBy(desc(chapter.serial)); + } else { + query = query.orderBy(desc(chapter.createdAt)); + } + return await query.limit(limit).offset(skip);Also applies to: 330-332
🧹 Nitpick comments (9)
src/app/admin/chapters/chapters-table.tsx (1)
24-58: Remove redundant key prop inside ChapterRow.The
keyprop on line 25 is redundant because the key is already provided in themapcall on line 77. React uses the outer key, so the inner one has no effect.Apply this diff to remove the redundant key:
- <TableRow key={chap.id}> + <TableRow>src/app/_components/actions.ts (1)
7-11: Clarify intent behind nesting createFrequentCachedQuery around an already frequent-cached getReleases.
WrappinggetReleases(which already usescreateCachedQuerywithCACHE_PRESETS.frequentand key['releases']) again withcreateFrequentCachedQuery(same preset, outer key['releases-pagination']) introduces two cache lookups. If the goal is a separate pagination cache, either:
- Wrap the underlying
_getReleasesdirectly, or- Use a single cached query with both key segments (e.g.
['releases', 'pagination'])src/app/(main)/novels/page.tsx (1)
19-19: Consider extracting the revalidation interval to a constant.The page-level revalidation interval (43200 seconds) is duplicated at line 53 as
12*3600, which evaluates to the same value. While both settings serve different purposes (ISR vs. cache), using different expressions reduces maintainability.Consider defining a constant:
+const NOVELS_PAGE_REVALIDATE = 12 * 3600; // 12 hours + export const metadata: Metadata = { title: "Novels", description: "A list of asian web novels fan translated by the translators of Quaslation." } -export const revalidate = 43200; // 12 hours +export const revalidate = NOVELS_PAGE_REVALIDATE;Then reference it in the cache configuration:
const getCacheData = unstable_cache(getNovelList, ["novels"], { tags: ["chapter:update:free", "novel:update", "novel:create"], - revalidate: 12*3600 + revalidate: NOVELS_PAGE_REVALIDATE });src/lib/db/schema.ts (2)
90-91: Revisit Chapter indexes: possible redundancy and sort direction
- serialIdx may be redundant given the unique (novelId, serial) index; most queries filter by novelId and order by serial, which the composite index already serves. Consider removing serialIdx to reduce write overhead.
- If you typically sort chapters by recency, prefer a DESC index for publishedAt with NULLS LAST.
Example:
- serialIdx: index("Chapter_serial_idx").using("btree", table.serial.asc().nullsLast()), - publishedAtIdx: index("Chapter_publishedAt_idx").using("btree", table.publishedAt.asc().nullsLast()), + // Consider dropping serialIdx if (novelId, serial) covers your queries + // serialIdx: index("Chapter_serial_idx").using("btree", table.serial.asc().nullsLast()), + publishedAtIdx: index("Chapter_publishedAt_idx").using("btree", table.publishedAt.desc().nullsLast()),
113-139: Comment table: add composite/partial indexes for common readsIf you list comments per novel or per user by recency, composite indexes can help and be more selective than single-column ones:
- (novelId, createdAt DESC)
- (userId, createdAt DESC)
- Optionally, partial indexes for visible comments, e.g., WHERE isHidden = false.
Example:
novelIdCreatedAtIdx: index("Comment_novelId_createdAt_idx") .using("btree", table.novelId.asc().nullsLast(), table.createdAt.desc().nullsLast()), userIdCreatedAtIdx: index("Comment_userId_createdAt_idx") .using("btree", table.userId.asc().nullsLast(), table.createdAt.desc().nullsLast()), // Partial index example (Postgres): // sql`CREATE INDEX "Comment_visible_novelId_createdAt_idx" // ON "Comment" ("novelId" ASC, "createdAt" DESC) WHERE "isHidden" = false;`src/app/(main)/novels/[slug]/page.tsx (1)
27-27: Use standard Next params typing (not Promise) and drop awaitCleaner, aligns with Next types, and avoids needless await.
-export async function generateMetadata({ params }: { params: Promise<{ slug: string }> }): Promise<Metadata> { - const paramsResolved = await params; +export async function generateMetadata({ params }: { params: { slug: string } }): Promise<Metadata> { + const paramsResolved = params;-export default async function NovelPage({ params }: { params: Promise<{ slug: string }> }) { - const paramsResolved = await params; +export default async function NovelPage({ params }: { params: { slug: string } }) { + const paramsResolved = params;Also applies to: 98-98
src/lib/auth/role-cache.ts (1)
1-126: Tighten types to Role enum for safetyUse the DB Role enum as a union type to prevent invalid values and improve DX.
+import { role as roleEnum } from "../db/schema"; +type Role = (typeof roleEnum.enumValues)[number]; export interface RoleCache { - get(userId: string): Promise<string | null>; + get(userId: string): Promise<Role | null>; set(userId: string, role: string, ttl?: number): Promise<void>; invalidate(userId: string): Promise<void>; } @@ export const roleCache: RoleCache = { async get(userId: string): Promise<string | null> { @@ - const cachedRole = await kv.get<string>(cacheKey); + const cachedRole = await kv.get<Role>(cacheKey); return cachedRole; @@ - async set(userId: string, role: string, ttl: number = DEFAULT_ROLE_CACHE_TTL): Promise<void> { + async set(userId: string, role: Role, ttl: number = DEFAULT_ROLE_CACHE_TTL): Promise<void> { @@ export async function getCachedUserRole(userId: string): Promise<string> { + // Narrow return type to Role + // ... - const cachedRole = await roleCache.get(userId); + const cachedRole = await roleCache.get(userId); @@ - const role = await getUserRoleFromDb(userId); + const role: Role = await getUserRoleFromDb(userId) as Role; @@ - return "MEMBER"; + return "MEMBER" as Role;src/app/_components/post-list-server.tsx (1)
55-60: Avoid double-caching and ensure key separation by argsgetReleases is already cached. Re-wrapping with createFrequentCachedQuery adds unnecessary layers and risks key/tag mismatches. Call getReleases directly so keys/tags include args (skip, premium) as intended.
Verify createCachedQuery includes function arguments in the cache key; if not, incorporate skip/premium into keyParts/tags in the source query.
-import { createFrequentCachedQuery, CACHE_TAGS } from "@/lib/cache" +// import { createFrequentCachedQuery, CACHE_TAGS } from "@/lib/cache" @@ -// Create cached query for releases with 1 hour revalidation -const getCachedReleases = createFrequentCachedQuery( - getReleases, - [CACHE_TAGS.releases.all, CACHE_TAGS.chapter.all], - ['releases'] -) +// Use getReleases directly (already cached) @@ - const initialChapters = await getCachedReleases({ skip: 0, premium }) + const initialChapters = await getReleases({ skip: 0, premium })Also applies to: 68-69
src/app/_components/post-list-client.tsx (1)
93-101: Pagination should consider deduped count, not raw response size
moreis driven bydata.lengthbefore deduplication; duplicates can make “Load More” persist when there’s nothing new.Suggested change inside the
skip > 0branch:- if(data.length < 10) { - setMore(false) - } + // Defer `more` decision until after dedupeand after computing
updatedChapters:- if (updatedChapters.length !== chaps.length + data.length) { + const uniqueNewCount = updatedChapters.length - chaps.length + if (uniqueNewCount < 10) { + setMore(false) + } + if (uniqueNewCount !== data.length) { toast({ title: "New Content available", description: `Wow! it seems like new webnovel chapter is available for ${premium ? "premium" : "free"}. Refresh the page to see new ${premium ? "premium" : "free"} chapter at the top.`, action: <ToastAction altText="Refresh" onClick={() => window.location.reload()}>Reload</ToastAction> }) }Also applies to: 96-107
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (32)
package.json(1 hunks)src/app/(main)/(mdx)/about/page.mdx(1 hunks)src/app/(main)/(mdx)/comment-policy/page.mdx(1 hunks)src/app/(main)/(mdx)/privacy/page.mdx(1 hunks)src/app/(main)/(mdx)/terms-of-service/page.mdx(1 hunks)src/app/(main)/home/page.tsx(2 hunks)src/app/(main)/layout.tsx(1 hunks)src/app/(main)/novels/[slug]/[chapter]/page.tsx(1 hunks)src/app/(main)/novels/[slug]/_components/chapter-list.tsx(2 hunks)src/app/(main)/novels/[slug]/page.tsx(3 hunks)src/app/(main)/novels/page.tsx(1 hunks)src/app/_components/actions.ts(1 hunks)src/app/_components/post-list-client.tsx(1 hunks)src/app/_components/post-list-server.tsx(1 hunks)src/app/admin/chapters/chapters-table.tsx(2 hunks)src/app/layout.tsx(2 hunks)src/components/system/analytics-error-boundary.tsx(1 hunks)src/components/system/google-adsense-wrapper.tsx(1 hunks)src/components/system/google-adsense.tsx(1 hunks)src/components/system/google-analytics-wrapper.tsx(1 hunks)src/lib/auth/role-cache.ts(1 hunks)src/lib/cache.ts(1 hunks)src/lib/db/0000_yielding_maelstrom.sql(1 hunks)src/lib/db/0004_happy_colonel_america.sql(1 hunks)src/lib/db/0005_marvelous_black_cat.sql(1 hunks)src/lib/db/meta/0004_snapshot.json(1 hunks)src/lib/db/meta/0005_snapshot.json(1 hunks)src/lib/db/meta/_journal.json(1 hunks)src/lib/db/query.ts(7 hunks)src/lib/db/relations.ts(3 hunks)src/lib/db/schema.ts(3 hunks)src/middleware.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (14)
src/components/system/analytics-error-boundary.tsx (1)
src/app/error.tsx (1)
Component(8-54)
src/lib/db/relations.ts (1)
src/lib/db/schema.ts (3)
comment(113-139)user(37-40)novel(49-70)
src/app/(main)/novels/[slug]/[chapter]/page.tsx (7)
src/app/(main)/home/page.tsx (1)
revalidate(5-5)src/app/(main)/novels/[slug]/page.tsx (1)
revalidate(15-15)src/app/(main)/novels/page.tsx (1)
revalidate(19-19)src/app/(main)/chapters/sitemap.ts (1)
revalidate(33-33)src/app/(main)/novels/[slug]/rss.xml/route.ts (1)
revalidate(7-7)src/app/(main)/novels/sitemap.ts (1)
revalidate(22-22)src/app/admin/chapters/actions.ts (1)
revalidate(38-40)
src/app/(main)/novels/page.tsx (7)
src/app/(main)/home/page.tsx (1)
revalidate(5-5)src/app/(main)/novels/[slug]/[chapter]/page.tsx (1)
revalidate(8-8)src/app/(main)/novels/[slug]/page.tsx (1)
revalidate(15-15)src/app/(main)/chapters/sitemap.ts (1)
revalidate(33-33)src/app/(main)/novels/[slug]/rss.xml/route.ts (1)
revalidate(7-7)src/app/(main)/novels/sitemap.ts (1)
revalidate(22-22)src/app/admin/chapters/actions.ts (1)
revalidate(38-40)
src/app/_components/post-list-server.tsx (6)
src/lib/db/query.ts (1)
getReleases(39-46)src/components/typography/h3.tsx (1)
H3(4-8)src/lib/utils.ts (2)
shortifyString(126-136)timeAgo(138-159)src/components/typography/muted.tsx (1)
Muted(4-8)src/lib/cache.ts (2)
createFrequentCachedQuery(149-155)CACHE_TAGS(37-75)src/app/_components/post-list-client.tsx (1)
PostListClient(67-166)
src/app/_components/post-list-client.tsx (4)
src/components/typography/h3.tsx (1)
H3(4-8)src/lib/utils.ts (2)
shortifyString(126-136)timeAgo(138-159)src/components/typography/muted.tsx (1)
Muted(4-8)src/lib/config.ts (1)
DISCORD_INVITE_URL(1-1)
src/app/(main)/novels/[slug]/page.tsx (10)
src/app/(main)/home/page.tsx (1)
revalidate(5-5)src/app/(main)/novels/[slug]/[chapter]/page.tsx (2)
revalidate(8-8)generateMetadata(15-26)src/app/(main)/novels/page.tsx (1)
revalidate(19-19)src/app/(main)/chapters/sitemap.ts (1)
revalidate(33-33)src/app/(main)/novels/[slug]/rss.xml/route.ts (1)
revalidate(7-7)src/app/(main)/novels/sitemap.ts (1)
revalidate(22-22)src/app/admin/chapters/actions.ts (1)
revalidate(38-40)src/lib/db/index.ts (1)
db(6-6)src/lib/db/schema.ts (1)
novel(49-70)src/lib/db/query.ts (1)
getNovel(77-84)
src/app/(main)/novels/[slug]/_components/chapter-list.tsx (2)
src/components/ui/badge.tsx (1)
Badge(36-36)src/components/ui/card.tsx (4)
Card(79-79)CardHeader(79-79)CardTitle(79-79)CardContent(79-79)
src/app/(main)/home/page.tsx (4)
src/app/(main)/novels/[slug]/[chapter]/page.tsx (1)
revalidate(8-8)src/app/(main)/novels/[slug]/page.tsx (1)
revalidate(15-15)src/app/(main)/novels/page.tsx (1)
revalidate(19-19)src/app/_components/post-list-server.tsx (1)
PostListServer(66-76)
src/lib/auth/role-cache.ts (1)
src/lib/db/schema.ts (1)
role(4-4)
src/middleware.ts (3)
src/lib/config.ts (1)
chapterIdRedirect(21-146)src/lib/db/schema.ts (2)
user(37-40)role(4-4)src/lib/auth/role-cache.ts (1)
getCachedUserRole(98-126)
src/app/admin/chapters/chapters-table.tsx (3)
src/lib/db/query.ts (1)
getChapters(337-344)src/lib/utils.ts (1)
shortifyString(126-136)src/app/admin/chapters/actions.ts (3)
freeChapter(7-18)publish(20-36)revalidate(38-40)
src/app/_components/actions.ts (2)
src/lib/cache.ts (2)
createFrequentCachedQuery(149-155)CACHE_TAGS(37-75)src/lib/db/query.ts (1)
getReleases(39-46)
src/lib/db/query.ts (2)
src/lib/db/schema.ts (4)
chapter(72-111)novel(49-70)richText(42-47)volume(17-35)src/lib/cache.ts (3)
createCachedQuery(97-122)CACHE_PRESETS(15-32)CACHE_TAGS(37-75)
🔇 Additional comments (28)
src/app/(main)/(mdx)/terms-of-service/page.mdx (1)
5-6: LGTM! Appropriate ISR configuration for static content.The 7-day revalidation interval is well-suited for Terms of Service content, which changes infrequently. This matches the revalidation strategy applied consistently across other policy pages.
src/app/(main)/(mdx)/comment-policy/page.mdx (1)
5-6: LGTM! Consistent ISR configuration.The 7-day revalidation aligns with the caching strategy for static policy content.
src/app/(main)/(mdx)/privacy/page.mdx (1)
5-6: LGTM! Consistent ISR configuration.src/app/(main)/(mdx)/about/page.mdx (1)
7-8: LGTM! Consistent ISR configuration.src/app/layout.tsx (1)
60-61: Verify the impact of moving GA from head to body.Moving Google Analytics initialization from
<head>to<body>may delay the start of tracking, potentially missing early page events or navigation. Standard practice places GA in the head for earliest initialization.Was this placement change intentional? If so, consider documenting the rationale (e.g., performance, user experience, or reducing head bloat). If early tracking is important, verify that the body placement doesn't introduce a significant gap in analytics data.
src/app/admin/chapters/chapters-table.tsx (3)
12-20: LGTM! Well-structured memoization.Good use of
React.memoanddisplayNamefor performance optimization and debugging clarity.
60-60: LGTM! Proper displayName for debugging.
77-77: LGTM! Clean component extraction.The refactoring successfully extracts row rendering logic into a memoized component while preserving all functionality.
src/lib/db/relations.ts (3)
2-2: LGTM! Import additions support Comment feature.
19-19: LGTM! Proper bidirectional relation for novel comments.
41-54: LGTM! Well-structured relations for Comment table.The
userRelationsandcommentRelationsproperly establish bidirectional associations matching the foreign key constraints defined in the schema. The relations correctly reference:
comment.novelId→novel.idcomment.userId→user.clerkIdsrc/lib/db/0004_happy_colonel_america.sql (1)
1-16: LGTM!The Comment table migration is well-structured with appropriate foreign keys, indexes, and CASCADE behavior for referential integrity. The indexes on
novelId,userId, andcreatedAtsupport common query patterns efficiently.src/components/system/google-adsense.tsx (1)
16-19: LGTM!The change to
lazyOnloadstrategy appropriately defers non-critical AdSense script loading until after the page is interactive and idle. The error handler provides useful debugging information.src/lib/db/0005_marvelous_black_cat.sql (1)
1-2: LGTM!The indexes on
serialandpublishedAtwill improve query performance for chapter ordering and filtering operations.src/lib/db/meta/_journal.json (1)
32-46: LGTM!The journal entries correctly track the new migrations (0004 and 0005) with appropriate version, timestamp, and breakpoint settings.
src/app/(main)/novels/[slug]/[chapter]/page.tsx (1)
8-8: LGTM!The page-level revalidation of 24 hours is appropriately set for ISR. The different cache revalidation intervals (7 days for metadata, 24 hours for content) reflect the different update frequencies of the data, which is a sound caching strategy.
package.json (2)
43-43: Approve axios version bump:^1.12.0is published in the npm registry and npm audit reports no known vulnerabilities.
51-51: Confirm Next.js v15.5.3 upgrade: This patch release exists and contains only bug fixes; no new breaking changes beyond those detailed in the 15.5 series release notes.src/app/(main)/layout.tsx (1)
3-3: LGTM!The refactor to use
GoogleAdsenseWrapperis correct and aligns with the PR's pattern of wrapping third-party analytics components with error boundaries and production checks.Also applies to: 9-9
src/app/(main)/home/page.tsx (1)
1-1: LGTM!The changes correctly implement server-side rendering with caching:
PostListServerenables cached data fetching on the server- The 1-hour revalidate time is appropriate for a home page displaying latest releases
Also applies to: 5-5, 26-26, 29-29
src/components/system/google-adsense-wrapper.tsx (1)
1-22: LGTM!The wrapper correctly implements production-only rendering with error boundary protection. The
process.env.NODE_ENVcheck is appropriate for Next.js client components.src/app/(main)/novels/[slug]/_components/chapter-list.tsx (1)
37-90: LGTM!The refactor to extract memoized
ChapterLinkandVolumeCardcomponents improves performance and maintainability. The memoization prevents unnecessary re-renders when parent state changes, which is beneficial for lists with many chapters.src/components/system/analytics-error-boundary.tsx (1)
1-39: LGTM!The error boundary is correctly implemented following React patterns. It properly catches errors, logs them, invokes the optional
onErrorcallback, and renders a fallback or null when errors occur.src/middleware.ts (1)
11-30: Good optimizations and error handling.The changes improve middleware performance and reliability:
- Map conversion (lines 11-12) provides O(1) chapter redirect lookups
- Early returns (lines 17-30) skip unnecessary processing for public routes and redirects
- Error handling (lines 53-57) prevents middleware crashes and provides safe fallback
Also applies to: 53-57
src/components/system/google-analytics-wrapper.tsx (1)
1-22: LGTM!The wrapper follows the same pattern as
GoogleAdsenseWrapper, correctly implementing production-only rendering with error boundary protection.src/lib/db/meta/0005_snapshot.json (1)
1-756: LGTM!This is an auto-generated database snapshot from Drizzle ORM that correctly reflects the schema changes, including the new
Commenttable with appropriate indexes and foreign keys.src/lib/db/meta/0004_snapshot.json (1)
1-726: Snapshot aligns with migration stateThe snapshot correctly captures the Comment table, FKs, and indexes expected for migration 0004. LGTM.
src/app/_components/post-list-client.tsx (1)
10-10: Import of getLatestPosts is client-safe The filesrc/app/_components/actions.tscontains no"use server"directive or server-only imports.
| const getNovelMetadata = unstable_cache(async (slug) => { | ||
| return await db.query.novel.findFirst({ | ||
| columns: { | ||
| title: true, | ||
| }, | ||
| where: (novel, { eq }) => eq(novel.slug, slug), | ||
| with: { | ||
| richText: { | ||
| columns: { | ||
| text: true, | ||
| } | ||
| return await db.query.novel.findFirst({ | ||
| columns: { | ||
| title: true, | ||
| }, | ||
| where: (novel, { eq }) => eq(novel.slug, slug), | ||
| with: { | ||
| richText: { | ||
| columns: { | ||
| text: true, | ||
| } | ||
| } | ||
| }) | ||
| }, | ||
| [], | ||
| } | ||
| }) | ||
| }, | ||
| [], | ||
| { | ||
| revalidate: 24*3600, | ||
| revalidate: 24 * 3600, | ||
| tags: [`novel:update:${paramsResolved.slug}`] | ||
| } | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
unstable_cache missing keyParts → cross-slug collisions
Using [] for keyParts can cause different slugs to share the same cached entry. Include the slug in keyParts.
- const getNovelMetadata = unstable_cache(async (slug) => {
+ const getNovelMetadata = unstable_cache(
+ async (slug: string) => {
return await db.query.novel.findFirst({
columns: { title: true },
where: (novel, { eq }) => eq(novel.slug, slug),
with: { richText: { columns: { text: true } } }
})
- },
- [],
- {
+ },
+ ['novel:metadata', paramsResolved.slug],
+ {
revalidate: 24 * 3600,
tags: [`novel:update:${paramsResolved.slug}`]
}
);🏁 Script executed:
#!/bin/bash
# Inspect createCachedQuery to confirm whether args are part of the cache key
rg -n -C3 "function createCachedQuery|export function createCachedQuery" --type ts
rg -n -C5 "unstable_cache\\(" src/lib/cache.tsLength of output: 658
🏁 Script executed:
#!/bin/bash
rg -n "unstable_cache" -C2 src/app/\(main\)/novels/\[slug\]/page.tsx
rg -n "<Link" -C2 src/app/\(main\)/novels/\[slug\]/page.tsx
rg -n "export default async" -C2 src/app/\(main\)/novels/\[slug\]/page.tsx
rg -n "Promise" -C2 src/app/\(main\)/novels/\[slug\]/page.tsxLength of output: 1978
Include unique keyParts in unstable_cache, fix Link href, and remove Promise wrapper on params
- getNovelMetadata: change
unstable_cache(async (slug) =>…, [], …)to include['novel:metadata', slug]as keyParts so each slug and function stay isolated. - getNovelCache: change
unstable_cache(getNovel, [], …)to include['novel:chapters', paramsResolved.slug]as keyParts. - In the “Read Now” Link, prepend
/novels/:
<Link href={/novels/${paramsResolved.slug}/${chapters[0].slug}}>. - Update both
generateMetadataandNovelPagesignatures from
{ params }: { params: Promise<{ slug: string }> }
to
{ params }: { params: { slug: string } }
and removeawait params.
| CREATE UNIQUE INDEX IF NOT EXISTS "Chapter_slug_key" ON "Chapter" USING btree ("slug");--> statement-breakpoint | ||
| CREATE UNIQUE INDEX IF NOT EXISTS "Chapter_volumeId_number_key" ON "Chapter" USING btree ("volumeId","number"); | ||
| */ No newline at end of file | ||
| CREATE UNIQUE INDEX IF NOT EXISTS "Chapter_volumeId_number_key" ON "Chapter" USING btree ("volumeId","number"); No newline at end of file |
There was a problem hiding this comment.
Critical: Moving index creation outside the block requires a new migration.
The Chapter_volumeId_number_key index was previously inside a commented block and is now moved outside. Since migration 0000 has already been applied (per _journal.json), this change will not be applied to existing databases. This creates schema drift where the index exists in new installations but not in upgraded ones.
Create a new migration to add this index:
-- src/lib/db/0006_new_migration_name.sql
CREATE UNIQUE INDEX IF NOT EXISTS "Chapter_volumeId_number_key" ON "Chapter" USING btree ("volumeId","number");🤖 Prompt for AI Agents
In src/lib/db/0000_yielding_maelstrom.sql around line 106 the CREATE UNIQUE
INDEX for "Chapter_volumeId_number_key" was moved out of a commented block after
migration 0000 was already applied, causing schema drift for existing databases;
create a new migration file (e.g.,
src/lib/db/0006_add_chapter_volume_number_index.sql) that contains a single
statement to create the index: CREATE UNIQUE INDEX IF NOT EXISTS
"Chapter_volumeId_number_key" ON "Chapter" USING btree ("volumeId","number");
and add it to your migration list so existing installations will apply the index
during upgrade.
| const userId = (await auth()).userId; | ||
| if(userId !== null) { | ||
|
|
||
| if (userId !== null) { | ||
| const user = await (await clerkClient()).users.getUser(userId); | ||
| const data = await getUserRole(user.emailAddresses[0] ? user.emailAddresses[0].emailAddress : ''); | ||
| if(data !== "ADMIN") return NextResponse.redirect(new URL("/", req.url)); | ||
| const userEmail = user.emailAddresses[0]?.emailAddress; | ||
|
|
||
| if (!userEmail) { | ||
| return NextResponse.redirect(new URL("/", req.url)); | ||
| } | ||
|
|
||
| // Use cached role lookup instead of direct database query | ||
| const role = await getCachedUserRole(userEmail); |
There was a problem hiding this comment.
Fix identifier mismatch in role lookup.
There's a critical bug: Line 45 passes userEmail to getCachedUserRole(), but the function expects a Clerk user ID, not an email address. The User table's primary key is clerkId (the Clerk user ID), and the role cache should look up by that identifier.
Apply this diff to fix the identifier mismatch:
const userId = (await auth()).userId;
if (userId !== null) {
const user = await (await clerkClient()).users.getUser(userId);
const userEmail = user.emailAddresses[0]?.emailAddress;
if (!userEmail) {
return NextResponse.redirect(new URL("/", req.url));
}
// Use cached role lookup instead of direct database query
- const role = await getCachedUserRole(userEmail);
+ const role = await getCachedUserRole(userId);
if (role !== "ADMIN") {
return NextResponse.redirect(new URL("/", req.url));
}📝 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 userId = (await auth()).userId; | |
| if(userId !== null) { | |
| if (userId !== null) { | |
| const user = await (await clerkClient()).users.getUser(userId); | |
| const data = await getUserRole(user.emailAddresses[0] ? user.emailAddresses[0].emailAddress : ''); | |
| if(data !== "ADMIN") return NextResponse.redirect(new URL("/", req.url)); | |
| const userEmail = user.emailAddresses[0]?.emailAddress; | |
| if (!userEmail) { | |
| return NextResponse.redirect(new URL("/", req.url)); | |
| } | |
| // Use cached role lookup instead of direct database query | |
| const role = await getCachedUserRole(userEmail); | |
| const userId = (await auth()).userId; | |
| if (userId !== null) { | |
| const user = await (await clerkClient()).users.getUser(userId); | |
| const userEmail = user.emailAddresses[0]?.emailAddress; | |
| if (!userEmail) { | |
| return NextResponse.redirect(new URL("/", req.url)); | |
| } | |
| // Use cached role lookup instead of direct database query | |
| const role = await getCachedUserRole(userId); | |
| if (role !== "ADMIN") { | |
| return NextResponse.redirect(new URL("/", req.url)); | |
| } |
🤖 Prompt for AI Agents
In src/middleware.ts around lines 34 to 45, the code calls
getCachedUserRole(userEmail) but the function expects a Clerk user ID (clerkId)
not an email; change the call to pass the Clerk user ID (use the existing userId
or user.id) and keep the email existence check for redirect logic — i.e.,
validate userEmail as before, but call getCachedUserRole(userId) (or user.id) to
perform the role lookup by the correct identifier.
- Add cache invalidation for chapter and release tags in `freeChapters` and `publishChapters` functions - Update `unstable_cache` calls in novel page to use dynamic cache keys with `slug` - Improve error handling in `PostListClient` with more descriptive error message - Add console error logging for better debugging - Ensure cache invalidation doesn't break mutation execution if it fails
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/app/(main)/novels/[slug]/page.tsx (1)
152-152: Critical: Incorrect Link href still not fixed from previous review.The "Read Now" button has an incorrect relative path that will cause 404 errors. The current href
${paramsResolved.slug}/${chapters[0].slug}creates a double-nested path like/novels/some-novel/some-novel/chapter-1instead of the intended/novels/some-novel/chapter-1.Apply this diff to fix:
- <Link href={`${paramsResolved.slug}/${chapters[0].slug}`}> + <Link href={`/novels/${paramsResolved.slug}/${chapters[0].slug}`}> Read Now </Link>src/lib/db/query.ts (1)
463-470: Critical: Placeholder tag prevents proper cache invalidation.The tag
CACHE_TAGS.role.byUser('{userId}')uses a literal string placeholder instead of the actualidparameter. This means:
- The cache tag will always be
'role:{userId}'regardless of which user- Cache invalidation for specific users won't work
- All users will share the same cache tag, defeating the purpose of per-user caching
Based on learnings about the caching framework from this PR,
createCachedQuerydoesn't currently support dynamic tag generation based on arguments. Until that feature is added, either:
- Use a single tag like
CACHE_TAGS.role.all(broader invalidation), or- Accept that per-user cache invalidation won't work (document this limitation)
Apply this diff to use a general tag until argument-derived tags are supported:
export const getUserRole = createCachedQuery( _getUserRole, { revalidate: CACHE_PRESETS.frequent.revalidate, - tags: [CACHE_TAGS.role.byUser('{userId}')], + tags: ['role:all'], // TODO: Use CACHE_TAGS.role.byUser(id) once createCachedQuery supports dynamic tags }, ['user-role'] );Alternatively, if per-user cache invalidation is critical, the caching framework in
src/lib/cache.tsneeds to be enhanced to accept a config factory function instead of a static config object, as suggested in the previous review.
🧹 Nitpick comments (3)
src/app/(main)/novels/[slug]/page.tsx (2)
29-49: Cache collision fixed, but consider adding namespace to keyParts.The slug is now included in keyParts, which resolves the critical cache collision issue flagged in the previous review. However, for better clarity and consistency with the tag naming convention, consider adding a semantic namespace.
Apply this diff to add namespace:
- const getNovelMetadata = unstable_cache(async (slug) => { + const getNovelMetadata = unstable_cache( + async (slug: string) => { return await db.query.novel.findFirst({ columns: { title: true, }, where: (novel, { eq }) => eq(novel.slug, slug), with: { richText: { columns: { text: true, } } } }) - }, - [paramsResolved.slug], + }, + ['novel:metadata', paramsResolved.slug], { revalidate: 24 * 3600, tags: [`novel:update:${paramsResolved.slug}`] } );
100-103: Cache collision fixed, but consider adding namespace to keyParts.Same as above - the slug is now included, preventing cache collisions. For consistency and clarity, add a semantic namespace prefix.
Apply this diff:
- const getNovelCache = unstable_cache(getNovel, [paramsResolved.slug], { + const getNovelCache = unstable_cache(getNovel, ['novel:chapters', paramsResolved.slug], { tags: [`novel:update:${paramsResolved.slug}`], revalidate: 12 * 3600 });src/app/_components/post-list-client.tsx (1)
135-136: Copy improvement suggestion.The current phrasing "report it, please" is slightly awkward. Consider rewording for better flow.
Apply this diff:
<p> - Something went wrong. Join our <a href={DISCORD_INVITE_URL} className="underline">Discord Server</a> and report it, please. + Something went wrong. Please join our <a href={DISCORD_INVITE_URL} className="underline">Discord Server</a> and report it. </p>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/(main)/novels/[slug]/page.tsx(3 hunks)src/app/_components/post-list-client.tsx(1 hunks)src/lib/db/query.ts(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/app/(main)/novels/[slug]/page.tsx (7)
src/app/(main)/home/page.tsx (1)
revalidate(5-5)src/app/(main)/novels/page.tsx (1)
revalidate(19-19)src/app/(main)/novels/[slug]/[chapter]/page.tsx (2)
revalidate(8-8)generateMetadata(15-26)src/app/(main)/chapters/sitemap.ts (1)
revalidate(33-33)src/app/(main)/novels/[slug]/rss.xml/route.ts (1)
revalidate(7-7)src/app/(main)/novels/sitemap.ts (1)
revalidate(22-22)src/app/admin/chapters/actions.ts (1)
revalidate(38-40)
src/lib/db/query.ts (1)
src/lib/cache.ts (3)
createCachedQuery(97-122)CACHE_PRESETS(15-32)CACHE_TAGS(37-75)
src/app/_components/post-list-client.tsx (4)
src/components/typography/h3.tsx (1)
H3(4-8)src/lib/utils.ts (2)
shortifyString(126-136)timeAgo(138-159)src/components/typography/muted.tsx (1)
Muted(4-8)src/lib/config.ts (1)
DISCORD_INVITE_URL(1-1)
🔇 Additional comments (6)
src/app/(main)/novels/[slug]/page.tsx (1)
15-16: LGTM!The ISR configuration is appropriate for a novel detail page. The 24-hour revalidation period balances freshness with server load.
src/app/_components/post-list-client.tsx (3)
75-86: LGTM! Deduplication logic is correct.The
removeDuplicateKeysfunction correctly deduplicates chapters by slug using a Set, ensuring no duplicate entries in the feed.
88-122: LGTM! Data fetching logic handles edge cases well.The
fetchLatestPostsfunction properly handles:
- Loading state management
- Error handling with user feedback
- Deduplication when appending data
- Detection of new content with toast notifications
- Scroll behavior for refresh scenarios
146-166: LGTM! Accessible and semantic markup.The feed implementation includes:
- Proper ARIA attributes (
role="feed",aria-busy,aria-live)- Screen reader-only heading for context
- Descriptive aria-labels on interactive elements
- Semantic HTML structure
These accessibility features ensure the component is usable by assistive technologies.
src/lib/db/query.ts (2)
363-372: LGTM! Cache invalidation properly implemented.The mutation functions now correctly invalidate related caches after updates:
- Invalidates multiple relevant tags (
chapter.all,releases.all,releases.free,releases.premium)- Uses try-catch to prevent cache invalidation failures from breaking the mutation
- Logs errors for debugging while continuing execution
This addresses the previous review concern about write operations not invalidating caches.
Also applies to: 392-401
5-10: LGTM! Caching framework integration is consistent.The imports and usage of
createCachedQuery,CACHE_PRESETS, andCACHE_TAGSfollow a consistent pattern across all wrapped queries. The wrapper functions properly delegate to internal_getXxxfunctions while adding caching semantics.Also applies to: 40-47
- Add support for dynamic cache tag generation in createCachedQuery - Implement config factory and tags function overloads for flexible caching - Update CacheConfig interface to support dynamic tag generation - Add comprehensive documentation explaining different caching strategies - Improve type safety for cache configuration and tag generation - Enable precise cache invalidation for resource-specific queries - Support static and dynamic cache tag configurations Enables more granular and flexible cache management with support for generating cache tags dynamically based on query arguments.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/lib/cache.ts (1)
258-263: Cache key generation might be unstable for complex arguments.Using
JSON.stringify(arg)for cache key generation can produce different results for semantically equivalent objects:// These would generate different cache keys despite being equivalent getNovel({ skip: 0, limit: 10 }) // Key includes '{"skip":0,"limit":10}' getNovel({ limit: 10, skip: 0 }) // Key includes '{"limit":10,"skip":0}'For the current usage (primitives and simple objects), this should work fine. However, if you later need to support complex objects or need guaranteed key stability, consider:
- Using a stable JSON stringifier that sorts keys (e.g.,
fast-json-stable-stringify)- Or requiring callers to pass a custom key serializer for complex arguments
- Or documenting that argument order matters for cache key generation
Example with stable stringifier:
+import stableStringify from 'fast-json-stable-stringify'; + // Create a unique cache key based on function name, key parts, and arguments const cacheKey = [ queryFn.name || "anonymous", ...keyParts, - ...args.map((arg) => JSON.stringify(arg)), + ...args.map((arg) => stableStringify(arg)), ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/cache.ts(1 hunks)src/lib/db/query.ts(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/db/query.ts (2)
src/lib/db/schema.ts (4)
chapter(72-111)novel(49-70)richText(42-47)volume(17-35)src/lib/db/index.ts (1)
db(6-6)
🔇 Additional comments (3)
src/lib/db/query.ts (2)
363-372: Cache invalidation errors are logged but execution continues.The try-catch blocks around
revalidateTagcalls allow mutations to succeed even if cache invalidation fails. While this prevents cache issues from breaking the mutation, it means stale data might be served until the next revalidation.Consider adding monitoring/alerting for cache invalidation failures to detect and respond to persistent issues:
} catch (error) { console.error('Cache invalidation failed:', error); // TODO: Add monitoring/alerting here (e.g., error tracking service) // Continue execution - cache invalidation failure shouldn't break the mutation }Also, verify that the revalidate intervals are short enough that stale data won't cause user-facing issues in practice. For example, with a 12-hour revalidate on chapters, a failed invalidation could serve stale data for up to 12 hours.
Also applies to: 392-401
463-470: LGTM! Dynamic tag implementation is correct.The
getUserRolewrapper now correctly uses a config factory to generate argument-derived tags. This enables precise cache invalidation withrevalidateTag(CACHE_TAGS.role.byUser(userId))rather than the ineffective placeholder pattern from before.Based on past review comments, this change addresses the previous issue where placeholder tags like
'{userId}'wouldn't match any invalidation calls.src/lib/cache.ts (1)
77-225: Excellent documentation!The comprehensive JSDoc with multiple examples, usage patterns, and guidance on when to use each approach is very helpful. This makes the caching API easy to understand and use correctly.
1 participant
Summary by CodeRabbit
New Features
Refactor
Database
Chores