feat!: mise à jour vers nextjs 16 #297
Conversation
chore!: mise à jour vers nextjs 16. Ne fonctionne pas encore totalement du à next-intl ne supportant pas entièrement les `cachedComponents` (amannn/next-intl#1493)
…onne pas avec next-intl pour l'instant. amannn/next-intl#1627 amannn/next-intl#1493 J'ai essayé de suivre ça (https://github.com/vercel/next.js/blob/7e27470461fe5bbe1098f213477b5d8305656fcd/docs/01-app/03-api-reference/04-functions/next-root-params.mdx) pour faire fonctionner les 'cachedComponents', mais sans succés.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCentralizes many locale-scoped imports to Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Browser as Browser
participant AuthClient as "authClient (browser)"
participant NextAPI as "/api/auth (Next.js)"
participant AuthServer as "better-auth (server)"
participant DB as Database
Browser->>AuthClient: user clicks provider sign-in
AuthClient->>NextAPI: POST /api/auth (toNextJsHandler)
NextAPI->>AuthServer: delegate request to better-auth
AuthServer->>DB: read/write users/accounts/sessions
DB-->>AuthServer: return records
AuthServer-->>NextAPI: return response/redirect
NextAPI-->>AuthClient: forward response
AuthClient-->>Browser: follow redirect / update UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/[locale]/ui/feeds/add-feed-form.tsx (1)
28-42: Potential hydration mismatch with useIsMobile.The
useIsMobilehook returnsundefinedduring the initial render (beforeuseEffectruns), which means!!isMobileevaluates tofalse, always rendering the desktop variant first. This can cause:
- Hydration mismatch warnings if SSR and client disagree
- Layout shift when the effect runs and switches to mobile variant
- Brief flash of desktop UI on mobile devices
Consider handling the
undefinedstate explicitly:export function AddFeedForm(): React.JSX.Element { const [isOpen, setIsOpen] = useState(false); const isMobile = useIsMobile(); + // Avoid rendering until mobile detection completes + if (isMobile === undefined) { + return null; // or a loading skeleton + } + return isMobile ? ( <AddFeedFormMobile isOpen={isOpen} onOpen={(status: boolean): void => setIsOpen(status)} /> ) : ( <AddFeedFormDesktop isOpen={isOpen} onOpen={(status: boolean): void => setIsOpen(status)} /> ); }Alternatively, use CSS media queries for the initial render to match the client-side detection.
🧹 Nitpick comments (1)
src/app/[locale]/ui/landing/scroll-indicator.tsx (1)
24-34: Double visibility control may be redundant.The component now has two layers of visibility control:
- The
Activitycomponent withmode={isMobile ? "hidden" : "visible"}(mobile vs desktop)- The opacity-based transitions with the
isVisiblestate (scroll-based)If the
Activitycomponent's "hidden" mode removes the element from the render tree entirely, the opacity transitions will have no effect on mobile. If it only hides visually, this creates redundant visibility logic.Please clarify the intended behavior and consider whether both mechanisms are necessary. If
Activity"hidden" mode prevents rendering, the opacity classes are unnecessary on mobile.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (52)
biome.json(3 hunks)next.config.ts(1 hunks)package.json(1 hunks)src/app/[locale]/(app)/d/(no-sidebar)/archive/page.tsx(1 hunks)src/app/[locale]/(app)/d/(no-sidebar)/links/page.tsx(1 hunks)src/app/[locale]/(app)/d/(no-sidebar)/newsletters/page.tsx(1 hunks)src/app/[locale]/(app)/d/(no-sidebar)/settings/page.tsx(1 hunks)src/app/[locale]/(app)/d/(sidebar)/feeds/page.tsx(1 hunks)src/app/[locale]/(app)/error.tsx(1 hunks)src/app/[locale]/(landing)/page.tsx(3 hunks)src/app/[locale]/(userfeeds)/userfeeds/[slug]/content/[feedContentEID]/route.tsx(1 hunks)src/app/[locale]/(userfeeds)/userfeeds/[slug]/route.ts(1 hunks)src/app/[locale]/layout.tsx(3 hunks)src/app/[locale]/legal/privacy/page.tsx(0 hunks)src/app/[locale]/legal/terms/page.tsx(0 hunks)src/app/[locale]/lib/hooks/use-media-query.tsx(0 hunks)src/app/[locale]/login/page.tsx(1 hunks)src/app/[locale]/ui/app-navigation-menu.tsx(1 hunks)src/app/[locale]/ui/auth/login-form.tsx(1 hunks)src/app/[locale]/ui/auth/logout-form.tsx(1 hunks)src/app/[locale]/ui/feeds/add-feed-form.tsx(1 hunks)src/app/[locale]/ui/feeds/feed-context-menu.tsx(1 hunks)src/app/[locale]/ui/feeds/feeds-header-context-menu.tsx(2 hunks)src/app/[locale]/ui/feeds/feeds-header.tsx(1 hunks)src/app/[locale]/ui/feeds/feeds-timeline.tsx(1 hunks)src/app/[locale]/ui/feeds/sidebar/feeds-sidebar-content.tsx(2 hunks)src/app/[locale]/ui/feeds/sidebar/feeds-sidebar-folder.tsx(1 hunks)src/app/[locale]/ui/feeds/sidebar/feeds-sidebar-footer.tsx(2 hunks)src/app/[locale]/ui/feeds/sidebar/feeds-sidebar-item-all.tsx(1 hunks)src/app/[locale]/ui/feeds/sidebar/feeds-sidebar-item.tsx(1 hunks)src/app/[locale]/ui/feeds/sidebar/feeds-sidebar.tsx(1 hunks)src/app/[locale]/ui/landing/scroll-indicator.tsx(2 hunks)src/app/[locale]/ui/links/add-link-form.tsx(2 hunks)src/app/[locale]/ui/links/links-context-menu.tsx(1 hunks)src/app/[locale]/ui/links/links.tsx(1 hunks)src/app/[locale]/ui/links/search-links.tsx(1 hunks)src/app/[locale]/ui/links/sort-links.tsx(1 hunks)src/app/[locale]/ui/newsletters/add-newsletter-form.tsx(1 hunks)src/app/[locale]/ui/newsletters/newsletter-item.tsx(2 hunks)src/app/[locale]/ui/settings/view/view-section.tsx(2 hunks)src/app/robots.tsx(1 hunks)src/app/sitemap.tsx(1 hunks)src/auth.ts(1 hunks)src/components/ui/navigation-menu.tsx(3 hunks)src/db/schema.ts(1 hunks)src/i18n/request.ts(1 hunks)src/lib/actions.ts(1 hunks)src/lib/dal.ts(1 hunks)src/lib/feed-service.ts(2 hunks)src/lib/stores/search-params-states.ts(1 hunks)src/proxy.ts(3 hunks)tsconfig.json(2 hunks)
💤 Files with no reviewable changes (3)
- src/app/[locale]/legal/privacy/page.tsx
- src/app/[locale]/lib/hooks/use-media-query.tsx
- src/app/[locale]/legal/terms/page.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-09T01:11:03.087Z
Learnt from: mtlaso
Repo: mtlaso/bocal PR: 253
File: src/app/[locale]/ui/feeds/sidebar/feeds-sidebar-content.tsx:31-39
Timestamp: 2025-08-09T01:11:03.087Z
Learning: In the bocal application, the `getUserFeedsGroupedByFolder()` function returns a `Map<string | null, FeedFolder>` where the key is either a string (representing the folder name) or null (representing uncategorized feeds). When rendering in `FeedsSidebarContent`, checking `if (key)` correctly distinguishes between named folders (truthy string) and uncategorized feeds (falsy null).
Applied to files:
src/app/[locale]/ui/feeds/sidebar/feeds-sidebar-content.tsxsrc/app/[locale]/ui/feeds/sidebar/feeds-sidebar.tsx
📚 Learning: 2025-04-08T06:37:10.493Z
Learnt from: mtlaso
Repo: mtlaso/bocal PR: 98
File: src/app/[locale]/(app)/feed/page.tsx:54-54
Timestamp: 2025-04-08T06:37:10.493Z
Learning: React's `cache` function imported from 'react' can be used in Next.js server components without requiring "use client" directive. It's specifically designed to deduplicate data requests in server components.
Applied to files:
src/lib/dal.tssrc/lib/feed-service.tsnext.config.ts
📚 Learning: 2025-04-08T06:37:10.493Z
Learnt from: mtlaso
Repo: mtlaso/bocal PR: 98
File: src/app/[locale]/(app)/feed/page.tsx:54-54
Timestamp: 2025-04-08T06:37:10.493Z
Learning: React's `cache()` function imported from 'react' is specifically designed for Server Components in Next.js and doesn't require the "use client" directive. It's used to deduplicate data fetching calls within a single request and can only be used in Server Components, not Client Components.
Applied to files:
src/lib/dal.tssrc/lib/feed-service.tsnext.config.ts
🧬 Code graph analysis (17)
src/app/[locale]/ui/auth/logout-form.tsx (1)
src/app/[locale]/lib/actions.ts (1)
logout(98-100)
src/app/[locale]/(app)/error.tsx (1)
src/app/[locale]/lib/logging.ts (2)
error(5-9)info(15-19)
src/app/[locale]/ui/feeds/feed-context-menu.tsx (1)
src/app/[locale]/lib/actions.ts (1)
archiveFeedContent(777-827)
src/lib/actions.ts (1)
src/app/[locale]/lib/actions.ts (1)
moveFeedIntoFolder(1055-1111)
src/app/[locale]/ui/landing/scroll-indicator.tsx (2)
src/lib/hooks/use-mobile.ts (1)
useIsMobile(5-21)src/lib/utils.ts (1)
cn(4-6)
src/app/[locale]/ui/feeds/sidebar/feeds-sidebar-content.tsx (1)
src/app/[locale]/lib/actions.ts (2)
moveFeedIntoFolder(1055-1111)addFeedFolder(974-1048)
src/app/[locale]/ui/settings/view/view-section.tsx (1)
src/app/[locale]/lib/actions.ts (1)
setFeedContentLimit(678-724)
src/app/[locale]/(userfeeds)/userfeeds/[slug]/content/[feedContentEID]/route.tsx (1)
src/app/[locale]/lib/feed-service.ts (1)
triggerBackgroundSync(257-349)
src/app/[locale]/ui/newsletters/newsletter-item.tsx (2)
src/app/[locale]/ui/newsletters/newsletters.tsx (1)
Newsletters(11-31)src/app/[locale]/lib/actions.ts (1)
deleteNewsletter(918-968)
src/app/[locale]/ui/feeds/sidebar/feeds-sidebar-footer.tsx (1)
src/app/[locale]/lib/actions.ts (1)
moveFeedIntoFolder(1055-1111)
src/app/[locale]/ui/feeds/add-feed-form.tsx (1)
src/lib/hooks/use-mobile.ts (2)
useIsMobile(5-21)setIsMobile(12-14)
src/app/[locale]/ui/newsletters/add-newsletter-form.tsx (1)
src/lib/hooks/use-mobile.ts (2)
useIsMobile(5-21)setIsMobile(12-14)
src/app/[locale]/layout.tsx (2)
src/i18n/routing.ts (1)
routing(6-11)src/components/ui/base-layout.tsx (1)
BaseLayout(13-36)
src/app/[locale]/(app)/d/(no-sidebar)/archive/page.tsx (2)
src/app/[locale]/lib/dal.ts (1)
Promise(182-288)src/app/[locale]/lib/actions.ts (1)
archiveLink(212-262)
src/app/[locale]/ui/feeds/feeds-timeline.tsx (1)
src/app/[locale]/lib/actions.ts (2)
moveFeedIntoFolder(1055-1111)archiveFeedContent(777-827)
src/proxy.ts (1)
src/lib/constants.ts (1)
APP_ROUTES(69-78)
src/app/[locale]/ui/links/add-link-form.tsx (1)
src/lib/hooks/use-mobile.ts (1)
useIsMobile(5-21)
🔇 Additional comments (63)
src/i18n/request.ts (1)
5-7: Clear documentation for request configuration.The added comments effectively explain the purpose and scope of
getRequestConfig, which improves code maintainability and helps developers understand the request-scoped configuration pattern for internationalization.src/app/[locale]/ui/landing/scroll-indicator.tsx (1)
3-3: No issues identified. Next.js 16 is available as of October 2025, and the App Router in Next.js 16 uses React 19.2, which includes the Activity component that hides UI with display: none while maintaining state and cleaning up Effects. TheActivitycomponent import from React and themodeprop usage in the code are correct.Likely an incorrect or invalid review comment.
src/app/[locale]/(landing)/page.tsx (1)
22-22: All APP_ROUTES imports have been consistently updated to the new centralized path.The refactoring to centralize constants in
@/lib/constantsis complete across the codebase with no remaining old import paths. This change follows Next.js best practices for project organization.src/components/ui/navigation-menu.tsx (3)
78-78: LGTM: Valid simplification to built-in utility.The change from
top-[1px]totop-pxuses Tailwind's built-in utility class instead of arbitrary value syntax. Both produce identical CSS output (top: 1px), but the built-in class is more concise.
115-115: No changes needed — CSS variable syntax is valid in Tailwind CSS v4.The parenthesized syntax for CSS variables like
h-(--property)is documented and supported in Tailwind CSS v4 as shorthand notation. The syntax change fromh-[var(--radix-navigation-menu-viewport-height)]toh-(--radix-navigation-menu-viewport-height)is valid and aligns with Tailwind CSS v4.0's CSS-first approach. The viewport will maintain its dynamic height and width correctly.
148-148: Thez-1utility class is valid in Tailwind CSS v4.The change from
z-[1]toz-1is correct for this project. In Tailwind v4, any z-index is allowed and generated despite not being in a predefined scale, unlike Tailwind v3 where only predefined values (z-0, z-10, z-20, z-30, z-40, z-50) were available. Since this project uses @tailwindcss/postcss version 4.1.18, thez-1class will generate a validz-index: 1CSS rule without issues.Likely an incorrect or invalid review comment.
tsconfig.json (1)
14-14: LGTM! Modern JSX transform and enhanced type coverage.The JSX runtime change to "react-jsx" uses the modern automatic JSX transform, which is appropriate for React 19. Adding
.next/dev/types/**/*.tsto the include array ensures TypeScript picks up Next.js development-mode generated types.Also applies to: 25-31
src/lib/stores/search-params-states.ts (1)
2-2: LGTM! Centralized import path.The import path for
SortOptionshas been correctly updated to reference the centralized constants module, consistent with the broader refactor in this PR.src/app/[locale]/ui/feeds/sidebar/feeds-sidebar-item-all.tsx (1)
11-11: LGTM! Centralized import path.The import path for
searchParamsStatehas been correctly updated to reference the centralized stores module.src/app/[locale]/ui/feeds/sidebar/feeds-sidebar-item.tsx (1)
13-15: LGTM! Centralized imports and mobile detection hook.The import paths have been correctly updated to reference centralized modules, and the
useIsMobilehook is appropriately used for mobile detection logic (Line 78).src/app/[locale]/ui/feeds/feeds-header.tsx (1)
8-8: LGTM! Centralized import path.The import path for
FeedStatusTypeandFeedWithContentsCounthas been correctly updated to reference the centralized constants module.src/app/[locale]/ui/feeds/feeds-header-context-menu.tsx (2)
14-17: LGTM! Centralized imports.All import paths have been correctly updated to reference centralized modules, consistent with the broader refactor in this PR.
76-76: Good practice: Explicit radix in parseInt.Adding the explicit radix
10toparseIntensures consistent decimal parsing and avoids potential issues with strings that might be interpreted as octal.src/app/[locale]/ui/feeds/add-feed-form.tsx (1)
24-25: LGTM! Centralized imports.Import paths have been correctly updated to reference centralized modules.
src/app/robots.tsx (1)
2-2: Import path verified successfully.The
getAppBaseURLfunction is correctly exported from the centralized location atsrc/lib/get-app-base-url.ts.biome.json (2)
2-2: LGTM: Schema version aligned with package update.The schema URL update to 2.3.9 correctly matches the Biome package version bump in package.json.
39-39: No action required. TheuseUniqueElementIdsrule is part of Biome's correctness group and remains active. It is configured via the correctness domain, not the nursery, so its absence from nursery rules in biome.json is expected behavior, not a removal requiring verification.src/app/[locale]/ui/settings/view/view-section.tsx (2)
15-16: LGTM: Import paths centralized.The import paths have been correctly updated to use centralized library locations, aligning with the project-wide refactoring effort.
58-60: LGTM: Explicit radix prevents potential parsing ambiguities.Adding the radix parameter (10) to
Number.parseIntis a best practice that ensures consistent decimal parsing and prevents potential issues with strings that could be misinterpreted as octal or hexadecimal.src/app/[locale]/ui/feeds/sidebar/feeds-sidebar-folder.tsx (1)
25-31: LGTM: Import paths centralized.The import paths have been correctly updated to reference centralized library locations, maintaining consistency with the project-wide refactoring effort.
package.json (2)
101-102: LGTM: Package manager version pinning improves reproducibility.Adding the
packageManagerfield with a specific pnpm version ensures consistent behavior across different development environments and CI/CD pipelines.
6-6: No action needed. Starting with Next.js 16, Turbopack is stable and used by default with next dev and next build, so the --turbopack flag is no longer necessary and can be removed from your package.json scripts. The dev script change correctly reflects this default behavior.src/db/schema.ts (1)
23-23: LGTM: Import path centralized.The import path has been correctly updated to reference the centralized constants location, consistent with the project-wide refactoring effort.
src/app/[locale]/ui/auth/logout-form.tsx (1)
6-6: LGTM: Import path centralized.The import path has been correctly updated to use the centralized library location with an absolute import alias, improving maintainability.
src/app/[locale]/(userfeeds)/userfeeds/[slug]/route.ts (1)
2-2: LGTM: Import path centralized.The import path has been correctly updated to reference the centralized feed service location, maintaining consistency with the project-wide refactoring effort.
next.config.ts (1)
16-28: All configuration options are valid for Next.js 16.
reactCompiler: trueis now stable (promoted from experimental).cacheComponents: trueenables Cache Components functionality.experimental.turbopackFileSystemCacheForDevis enabled by default starting with Next.js 16. The removal ofexperimental.pprreflects Next.js 16's deprecation of the PPR flag in favor of Cache Components. TheclientSegmentCacheanddevtoolSegmentExplorerwere experimental canary features in Next.js 15.x and are not part of the stable Next.js 16 configuration.src/app/[locale]/(userfeeds)/userfeeds/[slug]/content/[feedContentEID]/route.tsx (1)
2-3: LGTM! Import paths correctly centralized.The refactor from locale-scoped to centralized library paths improves code organization and maintainability.
src/app/[locale]/ui/app-navigation-menu.tsx (1)
25-25: LGTM! Consistent with centralization refactor.The APP_ROUTES import path update aligns with the broader refactoring effort to centralize shared constants.
src/app/[locale]/ui/links/sort-links.tsx (1)
25-26: LGTM! Import paths correctly updated.The centralized import paths for SortOptions and searchParamsState are consistent with the refactoring pattern.
src/app/[locale]/ui/feeds/feeds-timeline.tsx (1)
12-16: LGTM! Import consolidation complete.All action and utility imports have been correctly updated to centralized library paths. The optimistic UI logic remains intact.
src/app/[locale]/ui/feeds/feed-context-menu.tsx (1)
15-15: LGTM! Import path correctly updated.The archiveFeedContent import has been properly centralized, consistent with the refactoring pattern.
src/app/[locale]/ui/links/search-links.tsx (1)
10-10: LGTM! Import path correctly updated.The searchParamsState import has been properly centralized, maintaining consistency with the refactoring effort.
src/app/[locale]/(app)/error.tsx (1)
6-6: LGTM! Import path correctly updated.The logger import has been properly centralized. The error logging functionality remains unchanged.
src/lib/actions.ts (1)
30-39: LGTM! Comprehensive import consolidation for server actions.All utility imports have been correctly updated to use centralized library paths. Verification confirms no residual references to old locale-scoped paths remain in the codebase, and the new import pattern is consistently applied across the application.
src/app/[locale]/ui/links/links.tsx (1)
16-18: LGTM—Import centralization.The migration from locale-scoped paths to centralized library imports is clean and consistent with the broader refactoring across the PR.
src/app/[locale]/ui/feeds/sidebar/feeds-sidebar-footer.tsx (2)
36-37: LGTM—Import centralization.The migration to centralized library paths is consistent with the PR-wide refactoring pattern.
160-160: Theh-4.5class works correctly in Tailwind CSS v4 without requiring custom configuration. Tailwind v4 allows spacing utilities to accept any value out of the box, dynamically derived from a single spacing scale variable, soh-4.5resolves tocalc(0.25rem * 4.5) = 1.125rem(18px) by default. No extension to the@themeblock is needed.src/lib/dal.ts (1)
17-25: LGTM—Import centralization and auth import addition.The migration to centralized library paths and the addition of the
authimport are clean. Thecache()usage from React (line 16) is appropriate for Server Components, as confirmed by learnings.Based on learnings, React's
cache()is specifically designed for Server Components and doesn't require the "use client" directive.src/app/[locale]/login/page.tsx (2)
2-2: LGTM—Import path update.The migration to an absolute alias path is consistent with the broader refactoring.
1-12: This removal is correct for Next.js 16.In Next.js 16, the experimental
experimental.pprflag and configuration options have been removed in favor of the Cache Components configuration. The login page appropriately does not include theexperimental_pprexport since Partial Prerendering is the default behavior with Cache Components when enabled via thecacheComponentsoption innext.config.ts. This is not a regression but rather the evolution of PPR from experimental to stable in Next.js 16.src/app/[locale]/ui/links/links-context-menu.tsx (1)
16-17: LGTM—Import centralization.The migration to centralized library imports is consistent and straightforward.
src/app/[locale]/(app)/d/(sidebar)/feeds/page.tsx (1)
14-14: LGTM—Import centralization.The migration of the
dalimport to a centralized path is consistent with the broader refactoring.src/app/[locale]/ui/auth/login-form.tsx (1)
7-9: LGTM—Import path updates.The migration of
authenticateto the centralized library while keepingSPACINGin the locale-scoped UI path is appropriate, as UI utilities remain localized while cross-cutting actions are centralized.src/app/[locale]/(app)/d/(no-sidebar)/links/page.tsx (1)
15-15: LGTM—Import centralization.The migration of the
dalimport to a centralized path is consistent with the refactoring pattern throughout the PR.src/auth.ts (1)
6-7: LGTM! Import path centralization.The import paths for constants and logging have been updated to use centralized root paths. This aligns with the broader refactor pattern across the codebase.
src/app/[locale]/(app)/d/(no-sidebar)/newsletters/page.tsx (1)
10-10: LGTM! Consistent import path centralization.The
dalimport follows the same centralization pattern applied across the codebase.src/app/[locale]/ui/feeds/sidebar/feeds-sidebar.tsx (1)
7-7: LGTM! Import centralization.The
dalimport path has been updated to the centralized location.src/lib/feed-service.ts (2)
9-11: LGTM! Import path centralization.The imports for constants, logging, and parsing utilities have been moved to centralized root paths, consistent with the broader refactor.
174-182: Good refactor:map→forEachfor side effects.The change from
maptoforEachis more semantically correct since the operation mutates theatomobject via side effects rather than building a new array. The originalmapcreated an unused array, makingforEachthe more appropriate choice here.src/app/[locale]/(app)/d/(no-sidebar)/settings/page.tsx (1)
9-10: LGTM! Import centralization.The imports for
APP_ROUTESanddalhave been updated to use centralized root paths.src/app/[locale]/ui/newsletters/add-newsletter-form.tsx (2)
24-26: LGTM! Centralized imports and new mobile detection hook.The imports have been updated to use centralized paths, and the new
useIsMobilehook provides a cleaner abstraction for mobile detection.
31-43: Good improvement: More intuitive conditional logic.The conditional
isMobile ? Mobile : Desktopis more intuitive than previous patterns. TheuseIsMobilehook uses window.matchMedia for efficiency and automatically updates when the screen size changes, providing cleaner mobile detection across the application.src/app/[locale]/ui/newsletters/newsletter-item.tsx (2)
30-31: LGTM! Import centralization.The imports for
deleteNewsletteranduserfeedsfuncshave been updated to centralized paths.
48-48: LGTM! Minor formatting cleanup.The className has been collapsed to a single line without any semantic changes.
src/app/[locale]/(app)/d/(no-sidebar)/archive/page.tsx (2)
14-14: Centralized import path verified and working correctly.The
dalimport from@/lib/dalresolves properly to the module atsrc/lib/dal.ts. The import is actively used in the file and no old locale-scoped imports remain in the codebase.
14-66: PPR has been intentionally disabled project-wide.The removal of
experimental_pprfrom this page is part of a comprehensive decision to not use Partial Prerendering. Since noppr: 'incremental'configuration exists innext.config.tsand no pages exportexperimental_ppr, the feature is completely disabled. The Suspense boundaries in this route will continue to work for streaming, just without PPR prerendering.src/app/[locale]/ui/feeds/sidebar/feeds-sidebar-content.tsx (2)
18-23: LGTM! Import path centralization.The import paths have been successfully updated to use centralized lib locations. This aligns with the PR's objective to consolidate module structure.
77-77: Type error with RestrictToVerticalAxis modifier is legitimate. The change from@ts-ignoreto@ts-expect-erroris the appropriate best practice approach. This TypeScript type incompatibility with @dnd-kit/abstract 0.1.21 and @dnd-kit/react 0.1.21 is a known issue where RestrictToVerticalAxis in DragDropProvider's modifiers prop results in a type mismatch error. Using@ts-expect-errorcorrectly signals that this is an expected type error from the library that may be resolved in future versions.src/app/[locale]/layout.tsx (1)
42-44: LGTM! Correct implementation of generateStaticParams.The
generateStaticParamsfunction correctly generates static params for all locales defined in the routing configuration.src/app/sitemap.tsx (1)
3-4: LGTM! Import path centralization.The import paths have been correctly updated to use centralized lib locations, maintaining the same functionality.
src/app/[locale]/ui/links/add-link-form.tsx (2)
6-6: LGTM! Import path centralization and hook replacement.The import paths have been correctly updated to centralized lib locations, and the replacement of
useMediaQuerywith the more semanticuseIsMobilehook improves code readability.Also applies to: 26-27
30-45: LGTM! Mobile detection logic correctly updated.The conditional rendering logic has been correctly updated to use the new
useIsMobilehook. The logic now renders the mobile form whenisMobileis true, which maintains the intended behavior.src/proxy.ts (1)
41-59: File naming and function naming follow the current Next.js convention.Starting with Next.js 16, Middleware is now called Proxy to better reflect its purpose, with the functionality remaining the same. The renaming from
middleware.tstoproxy.tsand the function frommiddlewaretoproxy(line 41) aligns with the current Next.js file convention. The proxy.ts file should be located at the same level as pages or app. No special middleware configuration in Next.js config files is required—the proxy function is automatically recognized when properly placed and exported.
…kageManager' est définie dans 'package.json' .
|
Vercel Preview URL 🚀 : https://bocal-2hbrsrvgo-euuuuhs-projects.vercel.app |
|
Vercel Preview URL 🚀 : https://bocal-qqmg9lq8s-euuuuhs-projects.vercel.app |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
package.json(1 hunks)src/app/[locale]/layout.tsx(3 hunks)src/lib/actions.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/actions.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/app/[locale]/layout.tsx (2)
src/i18n/routing.ts (1)
routing(6-11)src/components/ui/base-layout.tsx (1)
BaseLayout(13-36)
⏰ 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). (1)
- GitHub Check: Create Neon Branch
🔇 Additional comments (7)
src/app/[locale]/layout.tsx (2)
6-9: LGTM!Import changes align with the PR objectives:
Suspenseadded for the new rendering pattern, andgetAppBaseURLmoved to the centralized@/libpath consistent with other files in this refactor.
42-44: LGTM!Standard Next.js pattern for static parameter generation. This correctly derives locale params from
routing.localesto enable static generation for both"en"and"fr"locales.package.json (5)
99-99: Good addition: Enforces consistent package manager version.The
packageManagerfield ensures all contributors use the same pnpm version (10.25.0) via Corepack. This prevents "works on my machine" issues caused by package manager version differences and is a best practice for monorepos and collaborative projects.
51-59: All Next.js 16 and React 19.2 migrations have been successfully completed. Verification confirms:
- No synchronous dynamic API calls found (cookies, headers, draftMode properly use await)
- Middleware properly renamed to proxy.ts
- No deprecated React forwardRef usage found
No further action needed.
14-67: Test authentication flows thoroughly with next-auth beta update.next-auth v5 is a major rewrite, though with as few breaking changes as possible. The dependency update to
5.0.0-beta.30involves significant API changes from v4, including import restructuring and OAuth 1.0 support deprecation and a minimum Next.js version requirement of 14.0. While most other dependency updates (Radix UI patches, Drizzle ORM, next-intl) are conservative and compatible, verify the next-auth migration guide and test all authentication flows after the upgrade, particularly session management and OAuth provider integrations.
72-73: The codebase is already using Tailwind CSS v4 syntax. The globals.css file demonstrates proper v4 CSS-first configuration with@import 'tailwindcss',@plugindirectives, and@themedefinitions. Code inspection found no v3 breaking patterns (old important modifiers, removed opacity utilities, or old CSS variable syntax), and CSS variables are correctly using the v4 parentheses syntax throughout. No migration action is needed.Likely an incorrect or invalid review comment.
6-6: Turbopack is now the default bundler in Next.js 16.The removal of the
--turboflag is expected and correct. Turbopack is now the default bundler in Next.js and no configuration is needed to use it. Development will automatically use Turbopack. The optional filesystem cache configuration in yournext.config.tswill continue to work as expected.
|
Vercel Preview URL 🚀 : https://bocal-5m6tba8fl-euuuuhs-projects.vercel.app |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/actions.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: mtlaso
Repo: mtlaso/bocal PR: 297
File: src/app/[locale]/layout.tsx:61-65
Timestamp: 2025-12-17T07:26:49.098Z
Learning: In Next.js 16 with cacheComponents: true enabled, next-intl requires wrapping components that access request-scoped data (like locale from params or call setRequestLocale) in Suspense boundaries. This includes wrapping BaseLayout in Suspense in the root layout at src/app/[locale]/layout.tsx when using next-intl with cacheComponents, even though it renders the root <html> and <body> structure.
🔇 Additional comments (1)
src/lib/actions.ts (1)
29-38: LGTM! Import path centralization.The refactor from locale-scoped paths (
@/app/[locale]/lib/*) to centralized top-level paths (@/lib/*) improves code organization and aligns with the PR's objective to consolidate module locations.
|
Vercel Preview URL 🚀 : https://bocal-4urh9tsjq-euuuuhs-projects.vercel.app |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/app/[locale]/(landing)/page.tsx (1)
163-163: Replacewrap-break-wordwithbreak-words.As noted in the previous review,
wrap-break-wordis not a standard Tailwind utility. Usebreak-wordsinstead:- "hover:shadow-md transition-all duration-200 wrap-break-word", + "hover:shadow-md transition-all duration-200 break-words",
🧹 Nitpick comments (1)
src/app/[locale]/(landing)/page.tsx (1)
121-145: Consider refactoring async sections to proper React components.The current pattern works but is unconventional: async functions return Promises that are rendered directly in JSX. A more idiomatic Next.js 15+/React 19 pattern would be to define these as async component functions and render them as JSX elements:
🔎 Proposed refactor to async components
-const HeroSection = async (locale: Locale): Promise<React.JSX.Element> => { +async function HeroSection({ locale }: { locale: Locale }): Promise<React.JSX.Element> { const t = await getTranslations({ locale, namespace: "metadata" }); return ( // ... existing JSX ); -}; +}Then update the elements array:
const elements: { - el: () => Promise<React.JSX.Element> | React.JSX.Element; + el: React.JSX.Element | Promise<React.JSX.Element>; index: number; }[] = [ { - el: () => HeroSection(locale as Locale), + el: <HeroSection locale={locale as Locale} />, index: 0, }, // ... similar for other sections ];And rendering:
{elements.map((el) => ( <BlurFade key={el.index} delay={0.25 + el.index * 0.05}> - {el.el()} + {el.el} </BlurFade> ))}Also applies to: 147-199, 201-222
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
messages/en.json(1 hunks)package.json(1 hunks)src/app/[locale]/(app)/d/(no-sidebar)/archive/page.tsx(3 hunks)src/app/[locale]/(app)/d/(no-sidebar)/links/page.tsx(3 hunks)src/app/[locale]/(app)/d/(no-sidebar)/newsletters/page.tsx(2 hunks)src/app/[locale]/(app)/d/(no-sidebar)/settings/page.tsx(3 hunks)src/app/[locale]/(app)/d/(sidebar)/feeds/page.tsx(3 hunks)src/app/[locale]/(landing)/page.tsx(6 hunks)src/app/[locale]/layout.tsx(2 hunks)src/app/[locale]/legal/privacy/page.tsx(2 hunks)src/app/[locale]/legal/terms/page.tsx(2 hunks)src/app/[locale]/login/page.tsx(1 hunks)src/app/[locale]/ui/settings/settings.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- messages/en.json
🚧 Files skipped from review as they are similar to previous changes (4)
- src/app/[locale]/(app)/d/(sidebar)/feeds/page.tsx
- src/app/[locale]/(app)/d/(no-sidebar)/newsletters/page.tsx
- src/app/[locale]/(app)/d/(no-sidebar)/archive/page.tsx
- src/app/[locale]/legal/privacy/page.tsx
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: mtlaso
Repo: mtlaso/bocal PR: 297
File: src/app/[locale]/layout.tsx:61-65
Timestamp: 2025-12-17T07:26:49.098Z
Learning: In Next.js 16 with cacheComponents: true enabled, next-intl requires wrapping components that access request-scoped data (like locale from params or call setRequestLocale) in Suspense boundaries. This includes wrapping BaseLayout in Suspense in the root layout at src/app/[locale]/layout.tsx when using next-intl with cacheComponents, even though it renders the root <html> and <body> structure.
📚 Learning: 2025-12-17T07:26:49.098Z
Learnt from: mtlaso
Repo: mtlaso/bocal PR: 297
File: src/app/[locale]/layout.tsx:61-65
Timestamp: 2025-12-17T07:26:49.098Z
Learning: In Next.js 16 with cacheComponents: true enabled, next-intl requires wrapping components that access request-scoped data (like locale from params or call setRequestLocale) in Suspense boundaries. This includes wrapping BaseLayout in Suspense in the root layout at src/app/[locale]/layout.tsx when using next-intl with cacheComponents, even though it renders the root <html> and <body> structure.
Applied to files:
src/app/[locale]/ui/settings/settings.tsxsrc/app/[locale]/(app)/d/(no-sidebar)/settings/page.tsxsrc/app/[locale]/login/page.tsxsrc/app/[locale]/legal/terms/page.tsxsrc/app/[locale]/(landing)/page.tsxsrc/app/[locale]/(app)/d/(no-sidebar)/links/page.tsxpackage.jsonsrc/app/[locale]/layout.tsx
📚 Learning: 2025-04-08T06:37:10.493Z
Learnt from: mtlaso
Repo: mtlaso/bocal PR: 98
File: src/app/[locale]/(app)/feed/page.tsx:54-54
Timestamp: 2025-04-08T06:37:10.493Z
Learning: React's `cache` function imported from 'react' can be used in Next.js server components without requiring "use client" directive. It's specifically designed to deduplicate data requests in server components.
Applied to files:
src/app/[locale]/(app)/d/(no-sidebar)/links/page.tsxsrc/app/[locale]/layout.tsx
📚 Learning: 2025-04-08T06:37:10.493Z
Learnt from: mtlaso
Repo: mtlaso/bocal PR: 98
File: src/app/[locale]/(app)/feed/page.tsx:54-54
Timestamp: 2025-04-08T06:37:10.493Z
Learning: React's `cache()` function imported from 'react' is specifically designed for Server Components in Next.js and doesn't require the "use client" directive. It's used to deduplicate data fetching calls within a single request and can only be used in Server Components, not Client Components.
Applied to files:
src/app/[locale]/layout.tsx
🧬 Code graph analysis (7)
src/app/[locale]/ui/settings/settings.tsx (3)
src/auth.ts (2)
session(45-60)Session(25-30)src/lib/constants.ts (1)
APP_ROUTES(69-78)src/app/[locale]/ui/settings/view/view-section.tsx (1)
ViewSection(23-40)
src/app/[locale]/(app)/d/(no-sidebar)/settings/page.tsx (4)
src/app/[locale]/login/page.tsx (1)
Page(6-18)src/i18n/routing.ts (1)
Locale(4-4)src/lib/dal.ts (1)
dal(330-337)src/app/[locale]/ui/settings/settings.tsx (1)
Settings(18-34)
src/app/[locale]/login/page.tsx (6)
src/app/[locale]/(app)/d/(no-sidebar)/archive/page.tsx (1)
Page(33-64)src/app/[locale]/(app)/d/(no-sidebar)/links/page.tsx (1)
Page(34-66)src/app/[locale]/(app)/d/(no-sidebar)/newsletters/page.tsx (1)
Page(28-54)src/app/[locale]/(app)/d/(no-sidebar)/settings/page.tsx (1)
Page(27-48)src/app/[locale]/(app)/d/(sidebar)/feeds/page.tsx (1)
Page(33-62)src/i18n/routing.ts (1)
Locale(4-4)
src/app/[locale]/legal/terms/page.tsx (1)
src/i18n/routing.ts (1)
Locale(4-4)
src/app/[locale]/(landing)/page.tsx (2)
src/i18n/routing.ts (1)
Locale(4-4)src/components/ui/navigation-menu.tsx (1)
NavigationMenu(159-159)
src/app/[locale]/(app)/d/(no-sidebar)/links/page.tsx (4)
src/app/[locale]/(app)/d/(no-sidebar)/archive/page.tsx (1)
Page(33-64)src/app/[locale]/(app)/d/(no-sidebar)/settings/page.tsx (1)
Page(27-48)src/app/[locale]/login/page.tsx (1)
Page(6-18)src/i18n/routing.ts (1)
Locale(4-4)
src/app/[locale]/layout.tsx (2)
src/i18n/routing.ts (1)
routing(6-11)src/app/layout.tsx (1)
RootLayout(3-9)
🔇 Additional comments (14)
package.json (2)
6-6: Verify removal of--turbopackflag is intentional.The
--turbopackflag was removed from the dev script. Next.js 15+ supports stable Turbopack development mode, which typically offers faster refresh times. Was this removal intentional, or should Turbopack remain enabled for development?
99-99: LGTM! Package manager version pinning added.The
packageManagerfield correctly pins pnpm to version 10.25.0 with integrity hash. This ensures consistent package manager behavior across all environments and contributors.src/app/[locale]/login/page.tsx (1)
1-10: LGTM! Locale handling follows the established Next.js 16 pattern.The implementation correctly:
- Extracts locale from async params using React's
usehook- Calls
setRequestLocaleto configure server-side translations- Uses the consistent
PagePropstype pattern seen across other pages in this PRsrc/app/[locale]/ui/settings/settings.tsx (2)
14-22: Clean Promise-based session handling with React 19'susehook.The refactor correctly:
- Accepts a
Promise<Session | null>prop for streamed session data- Resolves it with
use(sess)which works in Server Components- Redirects to login when session is missing
This pattern aligns well with the parent component passing
dal.verifySession()(as seen insrc/app/[locale]/(app)/d/(no-sidebar)/settings/page.tsx).
25-29: Safe null-coalescing for user properties.The fallback to empty strings (
?? "") handles cases wherenameorsrc/app/[locale]/(app)/d/(no-sidebar)/links/page.tsx (2)
34-38: Locale handling follows the established Next.js 16 pattern.Consistent with other pages: extracts locale via
use(params)and configures server-side translations withsetRequestLocale.
15-15: Good refactor: Centralized dal import path.Moving from
@/app/[locale]/lib/dalto@/lib/dalaligns with the PR's goal of centralizing imports.src/app/[locale]/legal/terms/page.tsx (1)
23-28: Correct async pattern withawait params.This async Server Component correctly uses
await params(vs.use(params)in sync components) and properly initializes the locale withsetRequestLocalebefore the dynamic MDX import.src/app/[locale]/layout.tsx (3)
41-43: Essential addition:generateStaticParamsfor static locale routes.This function enables Next.js to statically generate pages for each locale (
["en", "fr"]fromrouting.locales), which is important for Next.js 16's static generation behavior.
45-55: Layout correctly implements Next.js 16 locale handling.The implementation:
- Uses
LayoutPropstype consistent with other files- Validates locale against allowed values before proceeding
- Calls
setRequestLocalefor server-side i18n setupBased on learnings, the Suspense boundary requirement for
cacheComponents: trueis handled appropriately in the component hierarchy.
8-8: Consistent import centralization.Moving
getAppBaseURLto@/lib/get-app-base-urlaligns with the broader PR effort to centralize utility imports.src/app/[locale]/(app)/d/(no-sidebar)/settings/page.tsx (1)
27-44: LGTM! Correct Next.js 16 + next-intl integration pattern.The implementation correctly follows Next.js 16 patterns:
- Locale extracted via
use(params)and initialized withsetRequestLocaledal.verifySession()returns a Promise passed toSettingsSettingsis wrapped inSuspense, where it usesuse(sess)to unwrap the Promise- Import path centralized to
@/lib/dalBased on learnings, this pattern aligns with Next.js 16 requirements for request-scoped data access.
src/app/[locale]/(landing)/page.tsx (2)
27-50: LGTM! Correct Next.js 16 locale integration.The implementation correctly:
- Extracts locale via
use(params)and initializes withsetRequestLocale- Passes locale to async section functions
- Handles both sync and async element rendering
Based on learnings, this pattern aligns with Next.js 16 requirements.
55-55: Good catch on the typo fix!Correcting
justif-aroundtojustify-aroundfixes the flexbox layout.
|
Vercel Preview URL 🚀 : https://bocal-bqvb5ikfw-euuuuhs-projects.vercel.app |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
package.json (3)
52-52: Verify next-auth@5.0.0-beta.29 compatibility with Next.js 16.1.0.A previous review flagged next-auth@5.0.0-beta.30 as having peer dependency issues with Next.js 16. The current version (beta.29) may be pinned to avoid those issues, but verify that:
- beta.29 has no peer dependency conflicts with Next.js 16.1.0
- Authentication flows (login/logout/session) work correctly with React 19.2.0
- Consider upgrading to a stable next-auth v5 release once available
#!/bin/bash # Check for newer next-auth versions and peer dependency requirements echo "=== Checking next-auth peer dependencies ===" npm info next-auth@5.0.0-beta.29 peerDependencies 2>/dev/null || echo "Failed to fetch peer dependencies" echo "" echo "=== Checking for latest next-auth v5 versions ===" npm view next-auth versions --json 2>/dev/null | jq '.[] | select(startswith("5."))' | tail -5 || echo "Failed to fetch versions"
51-51: Verify Next.js 16.1.0 breaking changes have been addressed.The upgrade to Next.js 16.1.0 includes several breaking changes. While the AI summary indicates some migrations were completed (proxy middleware rename), verify that all async API changes have been applied throughout the codebase:
params,searchParamsnow requireawaitcookies(),headers(),draftMode()now return Promises- Route handler caching defaults changed
- Variant order now reads left → right
#!/bin/bash # Search for potentially un-migrated synchronous usage of async APIs echo "=== Checking for synchronous usage of async Next.js 16 APIs ===" # Check for direct property access on params/searchParams without await rg -n "params\.[a-zA-Z]|searchParams\.[a-zA-Z]" --type ts --type tsx -g '!*.test.*' -g '!*.spec.*' 2>/dev/null || true echo "" echo "=== Checking for cookies/headers/draftMode without await ===" rg -n "cookies\(\)|headers\(\)|draftMode\(\)" --type ts --type tsx -A2 -g '!*.test.*' 2>/dev/null | rg -v "await|useActionState" || true
58-59: Critical: Upgrade React to 19.2.1 for security patches.React 19.2.0 has known critical security vulnerabilities (CVSS 10.0) in the React Server Components protocol. React 19.2.1 addresses these issues and was released in December 2025.
🔎 Apply security update
- "react": "19.2.0", - "react-dom": "19.2.0", + "react": "19.2.1", + "react-dom": "19.2.1",
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mtlaso
Repo: mtlaso/bocal PR: 297
File: src/app/[locale]/layout.tsx:61-65
Timestamp: 2025-12-17T07:26:49.098Z
Learning: In Next.js 16 with cacheComponents: true enabled, next-intl requires wrapping components that access request-scoped data (like locale from params or call setRequestLocale) in Suspense boundaries. This includes wrapping BaseLayout in Suspense in the root layout at src/app/[locale]/layout.tsx when using next-intl with cacheComponents, even though it renders the root <html> and <body> structure.
📚 Learning: 2025-12-17T07:26:49.098Z
Learnt from: mtlaso
Repo: mtlaso/bocal PR: 297
File: src/app/[locale]/layout.tsx:61-65
Timestamp: 2025-12-17T07:26:49.098Z
Learning: In Next.js 16 with cacheComponents: true enabled, next-intl requires wrapping components that access request-scoped data (like locale from params or call setRequestLocale) in Suspense boundaries. This includes wrapping BaseLayout in Suspense in the root layout at src/app/[locale]/layout.tsx when using next-intl with cacheComponents, even though it renders the root <html> and <body> structure.
Applied to files:
package.json
⏰ 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). (1)
- GitHub Check: Create Neon Branch
🔇 Additional comments (2)
package.json (2)
84-99: Good: Package manager and overrides configured correctly.The explicit
packageManagerfield and pnpm overrides are well-configured for project consistency. Note that@types/reactand@types/react-domoverrides (19.2.2) are slightly ahead of the runtime versions (19.2.0). Once React is upgraded to 19.2.1 per the security fix, verify type compatibility remains sound.
6-6: No concerns with the dev script change. Turbopack is now the default bundler in Next.js, so removing the flag in Next.js 16.1.0 is the correct approach.
|
Vercel Preview URL 🚀 : https://bocal-epba895c0-euuuuhs-projects.vercel.app |
 |
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Fix all issues with AI agents
In `@auth-schema.ts`:
- Around line 61-63: The accounts table column definition for updatedAt (the
timestamp builder named updatedAt in auth-schema.ts) is marked .notNull() but
lacks a default, causing INSERTs to fail; fix by adding .defaultNow() to the
updatedAt column definition (the same pattern used for createdAt/defaultNow) so
that updatedAt has a default value on INSERT and still uses .$onUpdate(() => new
Date()) for updates.
- Around line 32-34: The sessions.updatedAt timestamp is marked .notNull() but
lacks a default causing INSERT failures; update the timestamp("updated_at")
definition for updatedAt (the same pattern used by createdAt) to include
.defaultNow() (i.e.,
timestamp("updated_at").defaultNow().$onUpdate(...).notNull()) so new session
rows can be inserted without explicitly providing updatedAt.
- Around line 11-23: The users table definition in auth-schema.ts must match
src/db/schema.ts: remove .notNull() from the email column (make email nullable),
add .unique() to the name column, and remove the inline preferences
jsonb("preferences") column so preferences are modeled via the separate
users_preferences table (which uses a prefs column); update the users export
accordingly (refer to the users constant and its id, name, email, preferences
symbols) to match the DB schema.
In `@drizzle/0020_daffy_nekra.sql`:
- Line 5: The migration is renaming accounts.token_type to accounts.idToken
which mixes semantics; instead decide whether to preserve token_type values or
create a separate idToken column: if the old token_type must be preserved, add a
new column idToken (ALTER TABLE "accounts" ADD COLUMN "idToken" TEXT),
migrate/copy relevant values from "token_type" into "idToken" with an UPDATE,
and only then DROP COLUMN "token_type" if appropriate; if token_type should be
discarded, simply DROP COLUMN "token_type" and ADD "idToken" separately; update
the migration SQL to use ADD COLUMN, UPDATE (data copy) and DROP COLUMN
statements rather than a direct RENAME so token semantics remain correct.
- Line 15: The migration adds accounts.accountId as nullable but your schema
requires notNull: true; update the migration to (1) keep the ADD COLUMN
"accountId" text as a nullable step, (2) add a backfill step that populates
"accountId" for all existing rows (e.g., copy from an existing identifier or
generate UUIDs) and commit, then (3) add an ALTER TABLE "accounts" ALTER COLUMN
"accountId" SET NOT NULL step to enforce the constraint; reference the
"accounts" table and "accountId" column in the migration so the sequence is add
-> backfill -> set NOT NULL.
- Line 27: The migration adds a new NOT NULL primary key column "id" to table
"verification_tokens" which will fail on tables with existing rows; change the
migration to a safe multi-step approach: first add "id" as nullable (ALTER TABLE
"verification_tokens" ADD COLUMN "id" text), populate it for existing rows
(e.g., update rows with gen_random_uuid() or another unique generator), then
alter the column to be NOT NULL and add the PRIMARY KEY constraint (or create
the PK separately), or alternatively add the column with a DEFAULT
gen_random_uuid() so existing rows are assigned values, or guard the statement
to run only when the table is empty—target the "verification_tokens" table and
the "id" column in your fixes.
In `@drizzle/0024_famous_nico_minoru.sql`:
- Around line 1-2: The migration currently adds "id" as NOT NULL PRIMARY KEY
immediately which will fail on non-empty "sessions"; instead: drop
"sessions_pkey" as you already do, ADD COLUMN "id" text NULL (no NOT NULL/PK),
run an UPDATE to backfill "id" for every row (preferably copying the existing
primary key column if one exists, otherwise generate stable unique values e.g.
uuid functions), then ALTER TABLE "sessions" ALTER COLUMN "id" SET NOT NULL and
finally ADD CONSTRAINT "sessions_pkey" PRIMARY KEY ("id"); ensure the backfill
uses a deterministic function available in your DB (uuid_generate_v4() or
gen_random_uuid()) or copies the existing key to preserve identity.
In `@package.json`:
- Around line 52-53: Remove the stale next-auth dependency from package.json and
update any remaining type/import references: delete the "next-auth" entry and
replace the type import in src/app/[locale]/ui/settings/settings.tsx with the
equivalent type from better-auth (or use a local session type), and
remove/ignore src/old-auth.ts if it's truly legacy; verify usage across
src/auth.ts, src/auth-client.ts, and src/proxy.ts to ensure nothing still
imports next-auth before committing.
In `@src/app/`[locale]/ui/auth/logout-form.tsx:
- Around line 14-23: Wrap the authClient.signOut call in error handling: perform
the existing toast.info(t("loggingOut")) then await authClient.signOut(...)
inside a try block (keeping the fetchOptions.onSuccess that calls
router.push("/")), and add a catch block that calls toast.error(...) with a
localized failure message (e.g. t("logoutFailed")), logs the error
(console.error or a logger), and prevents navigation so the user gets feedback;
ensure you reference authClient.signOut, toast, router.push and the existing
fetchOptions.onSuccess when making this change.
In `@src/auth-client.ts`:
- Around line 1-3: The client is initialized with an empty config; update the
createAuthClient call used to export authClient so it sets baseURL to the public
frontend env var (e.g., use process.env.NEXT_PUBLIC_BETTER_AUTH_URL) to match
server-side baseURL and production domain routing; ensure the
NEXT_PUBLIC_BETTER_AUTH_URL environment variable is exposed to the frontend so
the authClient uses the correct endpoint.
In `@src/auth.ts`:
- Around line 101-102: The code is using unsafe type assertions
(clientId/clientSecret) for environment variables (AUTH_GITHUB_ID,
AUTH_GITHUB_SECRET and the other pair at lines 108-109); replace these with a
small runtime validation helper (e.g., getRequiredEnv or assertEnv) and call it
when constructing the auth provider config so the app fails fast with a clear
error if a variable is missing, then use the validated value in the provider
configuration (replace direct uses of clientId/clientSecret with the returned
non-null string).
- Around line 104-107: The inline comment in the google config object is
malformed—split the concatenated URL/comment and the config option into two
parts: add a proper comment line with the URL
("https://www.better-auth.com/docs/authentication/google#always-get-refresh-token")
above or beside the google config, and restore the accessType option inside the
google object as accessType: "offline" (ensuring it's a separate property
alongside prompt). Update the google config (the google object and its
properties prompt and accessType) to remove the broken comment and include
accessType correctly.
- Around line 22-32: The preferences additional field is required but has no
defaultValue; import the DEFAULT_USERS_PREFERENCES constant and set defaultValue
for user.additionalFields.preferences to that constant (uncomment and assign
DEFAULT_USERS_PREFERENCES) so new users get the expected defaults; ensure the
import is added near other imports and that the symbol name
(DEFAULT_USERS_PREFERENCES) matches the exported identifier used elsewhere.
In `@src/db/schema.ts`:
- Around line 253-265: The sessions pgTable definition is missing an index on
the userId column; update the sessions table (pgTable named "sessions") to
create an index on the userId field so lookups by userId are efficient (mirror
the index that exists in auth-schema.ts). Add the index declaration alongside
the sessions definition (using your project's index helper/DSL so it becomes
part of the table schema) and ensure it references the sessions.userId column
and uses a sensible name (e.g., sessions_user_id_idx) and non-unique
configuration.
In `@src/old-auth.ts`:
- Around line 1-80: Delete this legacy NextAuth configuration file (old-auth.ts)
since it is dead code; remove the file entirely and update any imports that
might reference its exported symbols (handlers, signIn, signOut, auth) to use
the new auth implementation (e.g., auth.ts). Ensure there are no remaining
usages of config, createUser event handler, or the DrizzleAdapter setup in the
repo before committing the deletion.
🧹 Nitpick comments (12)
src/app/[locale]/ui/auth/logout-form.tsx (1)
24-24: Remove unnecessarytype="submit"attribute.This
Buttonis not inside a<form>element and uses anonClickhandler, sotype="submit"has no effect and could be misleading.🧹 Proposed fix
- type="submit" + type="button"src/lib/dal.ts (3)
17-18: Remove duplicate commented import.Line 18 contains a commented-out duplicate of the import on line 17. This appears to be leftover from the migration and should be removed.
🧹 Proposed fix
import { auth } from "@/auth"; -// import { auth } from "@/auth";
39-44: Remove commented-out legacy code.This commented block is the old NextAuth-based implementation. If the migration to better-auth is complete, this dead code should be removed to avoid confusion.
🧹 Proposed fix
-// /** -// * verifySession returns the current session (logged in user). -// */ -// const verifySession = cache(async (): Promise<Session | null> => { -// return await auth(); -// }); -
33-37: Add explicit return type for better type clarity and IDE support.The function lacks type annotation, which reduces clarity and IDE support despite strict TypeScript configuration. Use the proper better-auth type inference:
Promise<typeof auth.$Infer.Session | null>or create a named type alias for cleaner code.src/lib/actions.ts (1)
8-8: Remove commented-out import.The
signInandsignOutimports are commented out and no longer used in this file since the authentication functions were removed during the migration to better-auth.🧹 Proposed fix
-// import { signIn, signOut } from "@/auth";src/old-auth.ts (1)
45-60: Consider caching user preferences lookup in session callback.The session callback queries the database for user preferences on every session access. This could impact performance for authenticated requests.
Consider using React's
cache()or a similar memoization strategy to deduplicate preference lookups within a single request, or explore better-auth's built-in session data mechanisms.src/db/schema.ts (1)
224-251: Clean up commented-out code in accounts table.The accounts table has extensive commented-out code (lines 233-234, 238, 241, 248-250) that appears to be remnants of the old next-auth schema. Consider removing these comments to improve readability, or add a brief explanatory comment if they're intentionally kept for reference.
package.json (1)
6-6: Turbopack flag removed from dev script.The
--turbopackflag was removed from the dev script. In Next.js 15, Turbopack Dev was stable and production-ready. If this was intentional (e.g., due to compatibility issues with Next.js 16), this is fine. Otherwise, consider re-enabling it for faster development builds:"dev": "next dev --turbopack"src/app/api/auth/[...nextauth]/route.ts (1)
1-6: Remove commented-out code and consider renaming the route path.The commented-out next-auth code (lines 1-2) should be removed since the migration to better-auth is complete.
Additionally, the route path
[...nextauth]is misleading since better-auth is now being used. Better-auth typically uses[...all]as the catch-all route name. Consider renaming the directory to avoid confusion, though this would require updatingBETTER_AUTH_URLaccordingly.♻️ Proposed cleanup
-// import { handlers } from "@/auth"; -// export const { GET, POST } = handlers; - import { toNextJsHandler } from "better-auth/next-js"; import { auth } from "@/auth"; + export const { POST, GET } = toNextJsHandler(auth);src/auth.ts (1)
34-94: Remove or document the extensive commented-out code.Lines 34-94 contain a large block of commented-out hooks and database hooks. If this code is for future reference or planned implementation, consider:
- Moving it to a separate documentation file or README
- Creating a GitHub issue to track the implementation
- Removing it entirely if it's no longer needed
Leaving large blocks of commented code makes the file harder to maintain.
drizzle/0022_worried_mac_gargan.sql (1)
6-16: Ensureaccounts.idhas a generation strategy for future inserts.After Line 9-14,
idis NOT NULL/PK but no DEFAULT is set; inserts will fail unless the app always suppliesid. If the DB should generate it, add a default.✅ Optional default for `id`
ALTER TABLE "accounts" ADD COLUMN "id" text; +ALTER TABLE "accounts" ALTER COLUMN "id" SET DEFAULT gen_random_uuid()::text; UPDATE "accounts" SET "id" = gen_random_uuid()::text WHERE "id" IS NULL;drizzle/meta/0021_snapshot.json (1)
559-637: Consider adding a primary key to thesessionstable.The
sessionstable relies solely on a unique constraint ontokenwithout an explicit primary key. While PostgreSQL permits this, having a primary key is generally recommended for:
- Better ORM compatibility
- Clearer intent for the table's identity
- Potential performance benefits with clustered indexes
If
tokenis intended as the identifier, consider making it the primary key instead of just a unique constraint.
| export const users = pgTable("users", { | ||
| id: text("id").primaryKey(), | ||
| name: text("name").notNull(), | ||
| email: text("email").notNull().unique(), | ||
| emailVerified: boolean("email_verified").default(false).notNull(), | ||
| image: text("image"), | ||
| createdAt: timestamp("created_at").defaultNow().notNull(), | ||
| updatedAt: timestamp("updated_at") | ||
| .defaultNow() | ||
| .$onUpdate(() => /* @__PURE__ */ new Date()) | ||
| .notNull(), | ||
| preferences: jsonb("preferences").notNull(), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find SQL migrations touching the users table
fd -e sql . drizzle/ | xargs rg -l "users" | head -5
# Inspect users schema in src/db/schema.ts
rg -nA20 "export const users = pgTable" src/db/schema.tsRepository: mtlaso/bocal
Length of output: 769
🏁 Script executed:
#!/bin/bash
# Check for DEFAULT_USERS_PREFERENCES usage
rg -n "DEFAULT_USERS_PREFERENCES" .
# Inspect migrations for preferences column
rg -n "preferences" drizzle/Repository: mtlaso/bocal
Length of output: 6553
Align auth-schema.ts with src/db/schema.ts
- email: auth-schema sets
.notNull(), but DB definesemailas nullable. - name: auth-schema omits
.unique(), while DB enforces unique names. - preferences: auth-schema introduces an inline
preferencescolumn, but the DB uses a separateusers_preferencestable with aprefscolumn.
🤖 Prompt for AI Agents
In `@auth-schema.ts` around lines 11 - 23, The users table definition in
auth-schema.ts must match src/db/schema.ts: remove .notNull() from the email
column (make email nullable), add .unique() to the name column, and remove the
inline preferences jsonb("preferences") column so preferences are modeled via
the separate users_preferences table (which uses a prefs column); update the
users export accordingly (refer to the users constant and its id, name, email,
preferences symbols) to match the DB schema.
| updatedAt: timestamp("updated_at") | ||
| .$onUpdate(() => /* @__PURE__ */ new Date()) | ||
| .notNull(), |
There was a problem hiding this comment.
Missing defaultNow() on updatedAt will cause INSERT failures.
The sessions.updatedAt column is marked as .notNull() but lacks .defaultNow(), unlike createdAt. This will cause database errors when inserting new sessions without explicitly providing updatedAt.
🐛 Proposed fix
updatedAt: timestamp("updated_at")
+ .defaultNow()
.$onUpdate(() => /* `@__PURE__` */ new Date())
.notNull(),📝 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.
| updatedAt: timestamp("updated_at") | |
| .$onUpdate(() => /* @__PURE__ */ new Date()) | |
| .notNull(), | |
| updatedAt: timestamp("updated_at") | |
| .defaultNow() | |
| .$onUpdate(() => /* `@__PURE__` */ new Date()) | |
| .notNull(), |
🤖 Prompt for AI Agents
In `@auth-schema.ts` around lines 32 - 34, The sessions.updatedAt timestamp is
marked .notNull() but lacks a default causing INSERT failures; update the
timestamp("updated_at") definition for updatedAt (the same pattern used by
createdAt) to include .defaultNow() (i.e.,
timestamp("updated_at").defaultNow().$onUpdate(...).notNull()) so new session
rows can be inserted without explicitly providing updatedAt.
| updatedAt: timestamp("updated_at") | ||
| .$onUpdate(() => /* @__PURE__ */ new Date()) | ||
| .notNull(), |
There was a problem hiding this comment.
Missing defaultNow() on updatedAt will cause INSERT failures.
Same issue as with sessions table - the accounts.updatedAt column is .notNull() without a default value.
🐛 Proposed fix
updatedAt: timestamp("updated_at")
+ .defaultNow()
.$onUpdate(() => /* `@__PURE__` */ new Date())
.notNull(),📝 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.
| updatedAt: timestamp("updated_at") | |
| .$onUpdate(() => /* @__PURE__ */ new Date()) | |
| .notNull(), | |
| updatedAt: timestamp("updated_at") | |
| .defaultNow() | |
| .$onUpdate(() => /* `@__PURE__` */ new Date()) | |
| .notNull(), |
🤖 Prompt for AI Agents
In `@auth-schema.ts` around lines 61 - 63, The accounts table column definition
for updatedAt (the timestamp builder named updatedAt in auth-schema.ts) is
marked .notNull() but lacks a default, causing INSERTs to fail; fix by adding
.defaultNow() to the updatedAt column definition (the same pattern used for
createdAt/defaultNow) so that updatedAt has a default value on INSERT and still
uses .$onUpdate(() => new Date()) for updates.
| ALTER TABLE "accounts" RENAME COLUMN "refresh_token" TO "refreshToken";--> statement-breakpoint | ||
| ALTER TABLE "accounts" RENAME COLUMN "access_token" TO "accessToken";--> statement-breakpoint | ||
| ALTER TABLE "accounts" RENAME COLUMN "expires_at" TO "expiresAt";--> statement-breakpoint | ||
| ALTER TABLE "accounts" RENAME COLUMN "token_type" TO "idToken";--> statement-breakpoint |
There was a problem hiding this comment.
Semantic mismatch: token_type renamed to idToken.
The column token_type typically stores the token type (e.g., "Bearer"), while idToken represents the actual ID token value from OIDC. This rename appears to repurpose the column with different semantics.
If migrating from NextAuth to better-auth, verify:
- Whether existing
token_typedata should be preserved elsewhere - If this column should be dropped instead of renamed
- If a new
idTokencolumn should be added separately
🤖 Prompt for AI Agents
In `@drizzle/0020_daffy_nekra.sql` at line 5, The migration is renaming
accounts.token_type to accounts.idToken which mixes semantics; instead decide
whether to preserve token_type values or create a separate idToken column: if
the old token_type must be preserved, add a new column idToken (ALTER TABLE
"accounts" ADD COLUMN "idToken" TEXT), migrate/copy relevant values from
"token_type" into "idToken" with an UPDATE, and only then DROP COLUMN
"token_type" if appropriate; if token_type should be discarded, simply DROP
COLUMN "token_type" and ADD "idToken" separately; update the migration SQL to
use ADD COLUMN, UPDATE (data copy) and DROP COLUMN statements rather than a
direct RENAME so token semantics remain correct.
| ALTER TABLE "users" ALTER COLUMN "emailVerified" SET DATA TYPE boolean USING ("emailVerified" IS NOT NULL);--> statement-breakpoint | ||
| ALTER TABLE "users" ALTER COLUMN "emailVerified" SET NOT NULL;--> statement-breakpoint | ||
| ALTER TABLE "accounts" ADD CONSTRAINT "accounts_providerId_providerAccountId_pk" PRIMARY KEY("providerId","providerAccountId");--> statement-breakpoint | ||
| ALTER TABLE "accounts" ADD COLUMN "accountId" text;--> statement-breakpoint |
There was a problem hiding this comment.
accountId column is added as nullable but schema expects NOT NULL.
The column is added without NOT NULL, but the schema snapshot shows notNull: true. A subsequent migration or manual step is needed to:
- Populate existing rows with values
- Add the NOT NULL constraint
Verify the migration sequence handles this correctly, or add the constraint after data population.
🤖 Prompt for AI Agents
In `@drizzle/0020_daffy_nekra.sql` at line 15, The migration adds
accounts.accountId as nullable but your schema requires notNull: true; update
the migration to (1) keep the ADD COLUMN "accountId" text as a nullable step,
(2) add a backfill step that populates "accountId" for all existing rows (e.g.,
copy from an existing identifier or generate UUIDs) and commit, then (3) add an
ALTER TABLE "accounts" ALTER COLUMN "accountId" SET NOT NULL step to enforce the
constraint; reference the "accounts" table and "accountId" column in the
migration so the sequence is add -> backfill -> set NOT NULL.
| clientId: process.env.AUTH_GITHUB_ID as string, | ||
| clientSecret: process.env.AUTH_GITHUB_SECRET as string, |
There was a problem hiding this comment.
Environment variable assertions may fail silently at runtime.
The as string type assertions on environment variables (lines 101-102, 108-109) will mask undefined values and could cause cryptic runtime errors. Consider validating these at startup or using a more defensive pattern.
🛡️ Proposed fix with runtime validation
+const getRequiredEnv = (name: string): string => {
+ const value = process.env[name];
+ if (!value) {
+ throw new Error(`Missing required environment variable: ${name}`);
+ }
+ return value;
+};
+
export const auth = betterAuth({
// ...
socialProviders: {
github: {
scopes: ["read:user", "user:email"],
- clientId: process.env.AUTH_GITHUB_ID as string,
- clientSecret: process.env.AUTH_GITHUB_SECRET as string,
+ clientId: getRequiredEnv("AUTH_GITHUB_ID"),
+ clientSecret: getRequiredEnv("AUTH_GITHUB_SECRET"),
},
google: {
prompt: "select_account consent",
- clientId: process.env.AUTH_GOOGLE_ID as string,
- clientSecret: process.env.AUTH_GOOGLE_SECRET as string,
+ clientId: getRequiredEnv("AUTH_GOOGLE_ID"),
+ clientSecret: getRequiredEnv("AUTH_GOOGLE_SECRET"),
},
},Also applies to: 108-109
🤖 Prompt for AI Agents
In `@src/auth.ts` around lines 101 - 102, The code is using unsafe type assertions
(clientId/clientSecret) for environment variables (AUTH_GITHUB_ID,
AUTH_GITHUB_SECRET and the other pair at lines 108-109); replace these with a
small runtime validation helper (e.g., getRequiredEnv or assertEnv) and call it
when constructing the auth provider config so the app fails fast with a clear
error if a variable is missing, then use the validated value in the provider
configuration (replace direct uses of clientId/clientSecret with the returned
non-null string).
| google: { | ||
| // https://www.better-auth.com/docs/authentication/google#always-get-refresh-tokenaccessType: "offline", | ||
| // // Always get refresh token | ||
| prompt: "select_account consent", |
There was a problem hiding this comment.
Fix malformed comment on line 105.
The comment on line 105 has a URL concatenated with a config option, making it unreadable:
// https://www.better-auth.com/docs/authentication/google#always-get-refresh-tokenaccessType: "offline",
This should be split into a proper comment and potentially the accessType option:
🔧 Proposed fix
google: {
- // https://www.better-auth.com/docs/authentication/google#always-get-refresh-tokenaccessType: "offline",
- // // Always get refresh token
+ // https://www.better-auth.com/docs/authentication/google#always-get-refresh-token
+ // accessType: "offline", // Always get refresh token
prompt: "select_account consent",📝 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.
| google: { | |
| // https://www.better-auth.com/docs/authentication/google#always-get-refresh-tokenaccessType: "offline", | |
| // // Always get refresh token | |
| prompt: "select_account consent", | |
| google: { | |
| // https://www.better-auth.com/docs/authentication/google#always-get-refresh-token | |
| // accessType: "offline", // Always get refresh token | |
| prompt: "select_account consent", |
🤖 Prompt for AI Agents
In `@src/auth.ts` around lines 104 - 107, The inline comment in the google config
object is malformed—split the concatenated URL/comment and the config option
into two parts: add a proper comment line with the URL
("https://www.better-auth.com/docs/authentication/google#always-get-refresh-token")
above or beside the google config, and restore the accessType option inside the
google object as accessType: "offline" (ensuring it's a separate property
alongside prompt). Update the google config (the google object and its
properties prompt and accessType) to remove the broken comment and include
accessType correctly.
| export const sessions = pgTable("sessions", { | ||
| sessionToken: text().primaryKey(), | ||
| // sessionToken: text().primaryKey(), | ||
| id: text("id").primaryKey(), | ||
| token: text("token").notNull().unique(), | ||
| userId: text() | ||
| .notNull() | ||
| .references(() => users.id, { onDelete: "cascade" }), | ||
| expires: timestamp({ mode: "date" }).notNull(), | ||
| // expires: timestamp({ mode: "date" }).notNull(), | ||
| expiresAt: timestamp("expires_at").notNull(), | ||
| ipAddress: text("ip_address"), | ||
| userAgent: text("user_agent"), | ||
| ...timestampFields, | ||
| }); |
There was a problem hiding this comment.
Add missing index on userId for sessions table.
The sessions table is missing an index on userId that exists in the reference auth-schema.ts (lines 24-41). This index is important for efficient session lookups by user.
🔧 Proposed fix to add the missing index
-export const sessions = pgTable("sessions", {
+export const sessions = pgTable(
+ "sessions",
+ {
// sessionToken: text().primaryKey(),
id: text("id").primaryKey(),
token: text("token").notNull().unique(),
userId: text()
.notNull()
.references(() => users.id, { onDelete: "cascade" }),
// expires: timestamp({ mode: "date" }).notNull(),
expiresAt: timestamp("expires_at").notNull(),
ipAddress: text("ip_address"),
userAgent: text("user_agent"),
...timestampFields,
-});
+ },
+ (table) => [index("sessions_userId_idx").on(table.userId)],
+);📝 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.
| export const sessions = pgTable("sessions", { | |
| sessionToken: text().primaryKey(), | |
| // sessionToken: text().primaryKey(), | |
| id: text("id").primaryKey(), | |
| token: text("token").notNull().unique(), | |
| userId: text() | |
| .notNull() | |
| .references(() => users.id, { onDelete: "cascade" }), | |
| expires: timestamp({ mode: "date" }).notNull(), | |
| // expires: timestamp({ mode: "date" }).notNull(), | |
| expiresAt: timestamp("expires_at").notNull(), | |
| ipAddress: text("ip_address"), | |
| userAgent: text("user_agent"), | |
| ...timestampFields, | |
| }); | |
| export const sessions = pgTable( | |
| "sessions", | |
| { | |
| // sessionToken: text().primaryKey(), | |
| id: text("id").primaryKey(), | |
| token: text("token").notNull().unique(), | |
| userId: text() | |
| .notNull() | |
| .references(() => users.id, { onDelete: "cascade" }), | |
| // expires: timestamp({ mode: "date" }).notNull(), | |
| expiresAt: timestamp("expires_at").notNull(), | |
| ipAddress: text("ip_address"), | |
| userAgent: text("user_agent"), | |
| ...timestampFields, | |
| }, | |
| (table) => [index("sessions_userId_idx").on(table.userId)], | |
| ); |
🤖 Prompt for AI Agents
In `@src/db/schema.ts` around lines 253 - 265, The sessions pgTable definition is
missing an index on the userId column; update the sessions table (pgTable named
"sessions") to create an index on the userId field so lookups by userId are
efficient (mirror the index that exists in auth-schema.ts). Add the index
declaration alongside the sessions definition (using your project's index
helper/DSL so it becomes part of the table schema) and ensure it references the
sessions.userId column and uses a sensible name (e.g., sessions_user_id_idx) and
non-unique configuration.
|
Vercel Preview URL 🚀 : https://bocal-m8j96r951-euuuuhs-projects.vercel.app |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/app/`[locale]/(app)/d/(sidebar)/feeds/page.tsx:
- Around line 77-79: The fallback "Not logged in" string is hardcoded when sess
is falsy; replace it with a translated string by using the project's i18n
utilities: either call useTranslations (or getTranslations in this async
component) and return t('yourNamespace.notLoggedIn') instead of the literal, or
pass a translation function/prop into FeedsWrapper so it renders the translated
text; update the locale files with the notLoggedIn key accordingly. Reference:
sess check, FeedsWrapper, useTranslations, getTranslations.
In `@src/app/`[locale]/ui/settings/settings.tsx:
- Around line 14-23: The Settings component currently expects a sess Promise
(Props with sess and calling use(sess)) but the parent passes user={sess.user};
fix by changing Settings to accept the resolved user prop (rename Props from
sess to user), update the function signature to Settings({ user }: Props),
remove the use(sess) call, and replace checks that reference session.user with
the passed user (or redirect to APP_ROUTES.login when user is falsy); reference
the Props type, Settings function, sess/use(sess) usage, and the parent usage
user={sess.user} when making these edits.
🧹 Nitpick comments (2)
src/app/[locale]/(app)/d/(sidebar)/feeds/page.tsx (1)
86-88: Track the@ts-expect-errortechnical debt.The suppressed type error with TODO ("corriger quand Preferences fonctionnera dans auth.ts") indicates a known issue with the auth migration affecting
sess.user.preferencestyping. Consider creating an issue to track this fix once thePreferencestype is properly integrated inauth.ts.Would you like me to open a GitHub issue to track resolving this type suppression after the auth/preferences work is complete?
src/app/[locale]/ui/settings/settings.tsx (1)
30-32: Avoid@ts-ignore; address the type mismatch properly.The
Sessioninterface inauth.tsalready typespreferencesastypeof DEFAULT_USERS_PREFERENCES. If there's a mismatch withViewSection's expected type, consider:
- Exporting and reusing
DEFAULT_USERS_PREFERENCEStype inViewSection.- Using
@ts-expect-errorwith a specific explanation if this is a temporary workaround (it will error when the underlying issue is fixed, unlike@ts-ignore).Leaving
@ts-ignorerisks silently masking future type regressions.
| if (!sess) { | ||
| return <div>Not logged in</div>; | ||
| } |
There was a problem hiding this comment.
Hardcoded string is not internationalized.
The fallback "Not logged in" text is hardcoded in English while the rest of the page uses the useTranslations hook for i18n. This creates an inconsistent user experience for non-English locales.
🌐 Proposed fix using translations
Pass the translations to FeedsWrapper or use getTranslations within the async component:
async function FeedsWrapper(): Promise<React.JSX.Element> {
+ const t = await getTranslations("rssFeed");
const [timeline, sess] = await Promise.all([
dal.getUserFeedsTimeline(),
dal.verifySession(),
]);
if (!sess) {
- return <div>Not logged in</div>;
+ return <div>{t("notLoggedIn")}</div>;
}Then add the corresponding translation key to your locale files.
🤖 Prompt for AI Agents
In `@src/app/`[locale]/(app)/d/(sidebar)/feeds/page.tsx around lines 77 - 79, The
fallback "Not logged in" string is hardcoded when sess is falsy; replace it with
a translated string by using the project's i18n utilities: either call
useTranslations (or getTranslations in this async component) and return
t('yourNamespace.notLoggedIn') instead of the literal, or pass a translation
function/prop into FeedsWrapper so it renders the translated text; update the
locale files with the notLoggedIn key accordingly. Reference: sess check,
FeedsWrapper, useTranslations, getTranslations.
|
Vercel Preview URL 🚀 : https://bocal-5t8fddyq9-euuuuhs-projects.vercel.app |
|
Vercel Preview URL 🚀 : https://bocal-9w570bn6e-euuuuhs-projects.vercel.app |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/auth.ts`:
- Around line 14-17: The trustedOrigins array in src/auth.ts contains a
duplicate reference to process.env.BETTER_AUTH_URL; update the second entry to
the intended preview/alternate origin (e.g., process.env.VERCEL_URL or a preview
pattern based on VERCEL_URL) so trustedOrigins contains the primary
BETTER_AUTH_URL and the preview/alternate URL; edit the trustedOrigins array
definition to replace the redundant BETTER_AUTH_URL reference with the correct
environment variable or pattern (referencing the trustedOrigins symbol in the
file).
|
Vercel Preview URL 🚀 : https://bocal-flxacfsr4-euuuuhs-projects.vercel.app |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/auth.ts`:
- Around line 14-17: The trustedOrigins array is using process.env.VERCEL_URL
which is just a hostname; update the trustedOrigins construction (the
trustedOrigins variable/array in src auth module) to prepend "https://" to
process.env.VERCEL_URL when present (e.g., const vercelOrigin =
process.env.VERCEL_URL ? `https://${process.env.VERCEL_URL}` : undefined),
include only defined values (filter out undefined), and keep
process.env.BETTER_AUTH_URL as-is so origin validation receives a full URL for
VERCEL.
|
Vercel Preview URL 🚀 : https://bocal-6a6fz4afr-euuuuhs-projects.vercel.app |
|
Vercel Preview URL 🚀 : https://bocal-lzipoz53s-euuuuhs-projects.vercel.app |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/app/`[locale]/(landing)/page.tsx:
- Around line 121-124: The HeroSection server component is currently dumping the
entire environment via console.log(JSON.stringify(process.env)), which risks
leaking secrets and noisy per-request logs; remove that console.log call from
the HeroSection function (the async HeroSection(locale: Locale) surrounding the
getTranslations(...) call) and, if you need to inspect a value during
development, replace it with a targeted, non-sensitive log (e.g., log a single
safe variable or a debug flag) or use secure server-side debugging tools—ensure
no other occurrences of JSON.stringify(process.env) remain in the module.
- Around line 27-33: The Home component calls request-scoped APIs (use(params)
and setRequestLocale) while cacheComponents: true is enabled; convert export
default function Home to an async component or wrap the parts that access
request-scoped data in a React.Suspense boundary (matching other pages) so
request state doesn’t leak across cached instances — ensure any async child that
reads use(params) is suspended; also remove the console.log(process.env) call
(or replace it with logging of explicit, non-sensitive vars) to avoid exposing
secrets.
In `@src/auth.ts`:
- Around line 46-60: The current after hooks (after: async (user, _ctx) in the
user create handler and after: async (account) in the account.create handler)
log full user/account objects via logger.info which may contain PII or tokens;
change those logger calls to only log non-sensitive identifiers (e.g., user.id
and account.id) or explicitly redact sensitive fields before logging (e.g.,
construct a minimal object { id: user.id, email: user.email? } or a redacted
copy) and replace the two logger.info("User created", { user }) and
logger.info("Account created", { account }) calls accordingly.
…auth.com/docs/plugins/oauth-proxy#add-redirect-url-to-your-oauth-provider) pour faire fonctionner l'authentification des les previews env (vercel).
|
Vercel Preview URL 🚀 : https://bocal-lrh4joeja-euuuuhs-projects.vercel.app |
|
Vercel Preview URL 🚀 : https://bocal-qs5npmalr-euuuuhs-projects.vercel.app |
Merge pull request #297 from mtlaso/next16-update
1 participant
Summary by CodeRabbit
New Features
UI
Bug Fixes
Chores