Migrate project to next 16 and update pkgs - #33
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @hyochan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request undertakes a significant technical debt reduction by upgrading the project's core framework to Next.js 16 and updating a wide array of dependencies. The changes span across client-side components, server-side API routes, and build configurations, ensuring compatibility with the latest ecosystem while improving code structure and maintainability. The refactoring of Supabase client interactions and the adoption of new animation libraries are key aspects of this modernization effort. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully migrates the project to a newer version of Next.js and updates numerous packages. The code has been adapted to the new APIs, especially for Supabase authentication and Next.js routing. However, this migration has introduced several issues that need attention. There's a widespread use of as any type assertions to bypass TypeScript errors, which significantly reduces type safety and should be resolved by fixing the underlying type mismatches. More critically, the middleware file appears to have been renamed, which would prevent it from executing, and the soft-delete functionality has been removed from the Prisma setup, which could lead to unintended data loss. I've also identified a UI regression and areas for code simplification.
e4cbc1a to
ee397f6
Compare
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis pull request modernizes the Next.js application with ESLint flat config migration, Node 20 compatibility updates, Next.js 15 dynamic params pattern adoption, Supabase client refactoring, type system enhancements with new database type aliases and Prisma soft-delete extension, import alias standardization, and major dependency version bumps including Tailwind CSS 3.4.18. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ 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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
styles/output.css (1)
1-2783: Addstyles/output.cssto.gitignoreand remove from version control.This file is auto-generated by the Tailwind CSS build process and should not be committed. Your
package.jsonbuild scripts already regenerate it:"dev": "concurrently \"bun dev:next\" \"tailwindcss -i ./styles/root.css -o ./styles/output.css -w\"", "build": "tailwindcss -i ./styles/root.css -o ./styles/output.css && bun generate:prisma && next build"The source of truth should be
styles/root.cssandtailwind.config.js. Addstyles/output.cssto.gitignoreto prevent merge conflicts and keep the repository focused on intentional changes.app/[lang]/(common)/Button.tsx (1)
64-69: Avoid usinganyin type assertions.The explicit cast to
ReactElement<any>bypasses TypeScript's type checking. SincestartElementis already typed asReactElement, consider using a more specific type or investigating why the cast is necessary with the React 19 upgrade.If the issue is with
cloneElement's type signature in React 19, consider:- ? cloneElement(startElement as ReactElement<any>, { + ? cloneElement(startElement, { style: { position: 'absolute', left: 0, }, })If that doesn't work, at least narrow the type to preserve some safety:
- ? cloneElement(startElement as ReactElement<any>, { + ? cloneElement(startElement as ReactElement<{style?: CSSProperties}>, { style: { position: 'absolute', left: 0, }, })pages/api/plugins.ts (1)
23-48: Wrap switch case declarations in blocks.Static analysis correctly identifies that declarations within switch cases can leak to other cases. Biome's suggestion to wrap declarations in blocks prevents potential bugs.
Apply this refactor to wrap the case in a block:
switch (method) { case 'POST': + { const supabase = getSupabaseClient(); const {data}: {data: PluginRow | null} = await supabase .from('plugins') .select('id, description, json') .eq('id', id) .maybeSingle(); if (!data) { res.status(404).json({message: 'No plugins found.'}); return; } const tiers = (data.json || []) as Tier[]; res.status(200).json({ id: data.id, description: data.description, tiers, }); break; + } default: res.status(404).end(); }Based on static analysis hints from Biome.
♻️ Duplicate comments (3)
package.json (1)
37-37: Verify Next.js 16.0.5 availability.A previous review flagged this version as potentially invalid. Next.js 15 was the latest stable release as of recent public information, and version 16.0.5 may be a pre-release, RC, or typo.
What is the latest stable version of Next.js and does version 16.0.5 exist?app/auth/callback/route.ts (1)
36-41: Consider extracting language from thenextparameter path.The current logic falls back to
langParamor default locale, but doesn't extract the language from thenextpath itself (e.g.,/ko/statscontainsko). The previous review suggested preserving the locale from the original path.You could enhance locale preservation:
+ const extractLangFromPath = (path: string): string | null => { + const match = path.match(/^\/([a-z]{2})(\/|$)/); + return match ? match[1] : null; + }; + const redirectPath = next && next.startsWith('/') ? next - : `/${langParam || i18n.defaultLocale}`; + : `/${langParam || extractLangFromPath(next || '') || i18n.defaultLocale}`;This extracts the language code from the
nextpath if available before falling back tolangParam.proxy.ts (1)
29-33: Middleware will not run unless wired throughmiddleware.ts/middlewareexportAs written, this file exports
proxyfromproxy.ts. Next.js only auto-runs middleware from a file namedmiddleware.(ts|js)that exports amiddlewarefunction. Unless you have a separatemiddleware.tsthat re-exports this (export {proxy as middleware} from './proxy'or similar), this logic will never execute in the request pipeline, which is a functional blocker.I recommend either:
- Renaming this file to
middleware.tsand the function tomiddleware, or- Adding a small
middleware.tsthat re-exportsproxyasmiddleware.Please double-check the current Next.js middleware docs for your exact Next version to confirm the required shape.
🧹 Nitpick comments (6)
app/[lang]/(home)/SectionFooter.tsx (1)
49-57: Accessibility and security improvements look good.The updated link correctly points to the github-stats repository and includes an appropriate
aria-label. Therel="noreferrer"attribute enhances privacy.Consider adding
noopenerexplicitly for maximum compatibility with older browsers:- rel="noreferrer" + rel="noopener noreferrer"app/auth/callback/route.ts (1)
26-26: Type the cookies options parameter.The
optionsparameter usesany, which reduces type safety. Consider using the proper CookieOptions type.+ import type {CookieOptions} from '@supabase/auth-helpers-nextjs'; + cookies: { get(name: string) { return cookieStore.get(name)?.value; }, - set(name: string, value: string, options: any) { + set(name: string, value: string, options: CookieOptions) { cookieStore.set(name, value, options); }, },src/prisma.ts (1)
40-49: Consider interceptingfindUniqueandfindUniqueOrThrowfor completeness.The extension intercepts
findFirst,findMany, andcount, but notfindUniqueorfindUniqueOrThrow. This could allow retrieval of soft-deleted records through those methods.$allModels: { findFirst({model, args, query}) { return query(appendNotDeletedFilter(model, args || {})); }, + findUnique({model, args, query}) { + return query(appendNotDeletedFilter(model, args || {})); + }, + findUniqueOrThrow({model, args, query}) { + return query(appendNotDeletedFilter(model, args || {})); + }, findMany({model, args, query}) { return query(appendNotDeletedFilter(model, args || {})); },app/[lang]/loading.tsx (1)
4-7: LGTM! Lottie library migration looks correct.The migration from
@crello/react-lottietolottie-reactis properly implemented with the correct API usage.Consider using the path alias
@/publicfor consistency with the project's alias configuration:-import animationData from '../../public/lotties/dooboo-loading.json'; +import animationData from '@/public/lotties/dooboo-loading.json';app/[lang]/(common)/Header/index.tsx (1)
47-47: Consider passing the memoized Supabase client to child components.
DesktopNavMenuscallsgetSupabaseBrowserClient()directly on each render, creating a new client instance. Since the parentHeadercomponent already memoizes the client, consider passing it as a prop for consistency.The same issue exists in
MobileNavMenusat line 129.function DesktopNavMenus( props: Props & { login: string; isDark: boolean; setIsDark: Dispatch<SetStateAction<boolean>>; navLinks: NavLink[]; + supabase: ReturnType<typeof getSupabaseBrowserClient>; }, ): ReactElement { - const {t, lang, login, isDark, setIsDark, navLinks} = props; + const {t, lang, login, isDark, setIsDark, navLinks, supabase} = props; const pathname = usePathname(); const router = useRouter(); - const supabase = getSupabaseBrowserClient();server/services/githubService.ts (1)
649-651: Re-throwing error loses original stack trace.Wrapping the caught error in a new
Errordiscards the original stack trace. Consider re-throwing the original error or usingcause:} catch (e: any) { - throw new Error(e); + throw e; }Or to preserve context with
cause(ES2022+):} catch (e: any) { - throw new Error(e); + throw new Error('Failed to get dooboo stats', { cause: e }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
bun.lockis excluded by!**/*.lockbun.lockbis excluded by!**/bun.lockbyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (55)
.eslintignore(0 hunks).eslintrc.js(0 hunks).github/workflows/ci.yml(2 hunks)app/[lang]/(common)/Button.tsx(1 hunks)app/[lang]/(common)/Dropdown.tsx(1 hunks)app/[lang]/(common)/GreatFrontEnd.tsx(2 hunks)app/[lang]/(common)/Header/index.tsx(3 hunks)app/[lang]/(home)/Hero/StatsSymbol.tsx(4 hunks)app/[lang]/(home)/Hero/StatsUrlCards.tsx(2 hunks)app/[lang]/(home)/SectionFooter.tsx(1 hunks)app/[lang]/layout.tsx(1 hunks)app/[lang]/loading.tsx(2 hunks)app/[lang]/page.tsx(1 hunks)app/[lang]/recent-list/page.tsx(1 hunks)app/[lang]/sign-in/SocialButtons.tsx(3 hunks)app/[lang]/sign-in/page.tsx(1 hunks)app/[lang]/stats/[login]/Scouter/StatsHeader.tsx(2 hunks)app/[lang]/stats/[login]/Scouter/index.tsx(1 hunks)app/[lang]/stats/[login]/page.tsx(1 hunks)app/[lang]/stats/page.tsx(1 hunks)app/auth/callback/route.ts(1 hunks)eslint.config.mjs(1 hunks)next.config.js(1 hunks)package.json(1 hunks)pages/api/github-stats-advanced.ts(1 hunks)pages/api/github-stats.ts(1 hunks)pages/api/github-trophies.ts(1 hunks)pages/api/news-letter.ts(2 hunks)pages/api/plugins.ts(2 hunks)pages/api/recent-users.ts(1 hunks)proxy.ts(2 hunks)server/context.ts(1 hunks)server/plugins/stats/earth.ts(0 hunks)server/plugins/svgs/assets.ts(0 hunks)server/plugins/svgs/githubTiers.ts(0 hunks)server/plugins/svgs/githubTrophies.ts(0 hunks)server/services/githubService.ts(8 hunks)server/supabaseClient.ts(1 hunks)server/supabaseServerClient.ts(1 hunks)src/components/ErrorBoundary.tsx(0 hunks)src/fetches/github.ts(0 hunks)src/prisma.ts(1 hunks)src/services/userService.ts(0 hunks)src/types/supabase.ts(8 hunks)src/types/types.ts(1 hunks)src/utils/common.ts(0 hunks)src/utils/functions.ts(2 hunks)src/utils/supabase.ts(1 hunks)src/utils/theme.ts(2 hunks)styles/output.css(23 hunks)test/(common)/Button.test.tsx(1 hunks)test/css-color-shim.js(1 hunks)test/lru-cache.mock.ts(1 hunks)tsconfig.json(2 hunks)vitest.config.ts(1 hunks)
💤 Files with no reviewable changes (10)
- src/services/userService.ts
- .eslintrc.js
- src/components/ErrorBoundary.tsx
- .eslintignore
- server/plugins/svgs/assets.ts
- server/plugins/stats/earth.ts
- server/plugins/svgs/githubTiers.ts
- server/plugins/svgs/githubTrophies.ts
- src/fetches/github.ts
- src/utils/common.ts
🧰 Additional context used
🧬 Code graph analysis (16)
server/supabaseClient.ts (1)
src/types/supabase.ts (1)
Database(9-318)
app/[lang]/sign-in/page.tsx (3)
app/[lang]/page.tsx (1)
Page(13-52)app/[lang]/stats/page.tsx (1)
Page(14-35)src/i18n.ts (1)
Locale(6-6)
app/[lang]/page.tsx (4)
app/[lang]/recent-list/page.tsx (1)
Page(22-66)app/[lang]/sign-in/page.tsx (1)
Page(19-83)app/[lang]/stats/page.tsx (1)
Page(14-35)src/i18n.ts (1)
Locale(6-6)
app/auth/callback/route.ts (2)
src/types/supabase.ts (1)
Database(9-318)src/i18n.ts (1)
i18n(1-4)
app/[lang]/stats/[login]/Scouter/StatsHeader.tsx (2)
app/[lang]/stats/[login]/Scouter/index.tsx (1)
StatName(29-29)src/fetches/github.ts (1)
StatsInfo(41-48)
app/[lang]/layout.tsx (1)
src/i18n.ts (1)
Locale(6-6)
src/utils/functions.ts (4)
src/types/types.ts (2)
PluginRow(8-8)UserPluginRow(10-10)server/supabaseClient.ts (1)
getSupabaseClient(9-17)src/types/supabase.ts (1)
Database(9-318)server/plugins/svgs/functions.ts (1)
getTierName(35-55)
src/utils/supabase.ts (2)
src/types/supabase.ts (1)
Database(9-318)src/utils/assert.ts (1)
assert(7-14)
app/[lang]/(home)/Hero/StatsSymbol.tsx (2)
src/fetches/github.ts (1)
StatsInfo(41-48)app/[lang]/stats/[login]/Scouter/index.tsx (1)
StatName(29-29)
app/[lang]/stats/page.tsx (5)
app/[lang]/page.tsx (1)
Page(13-52)app/[lang]/recent-list/page.tsx (1)
Page(22-66)app/[lang]/sign-in/page.tsx (1)
Page(19-83)app/[lang]/stats/[login]/page.tsx (1)
Page(18-52)src/i18n.ts (1)
Locale(6-6)
app/[lang]/recent-list/page.tsx (5)
app/[lang]/page.tsx (1)
Page(13-52)app/[lang]/sign-in/page.tsx (1)
Page(19-83)app/[lang]/stats/[login]/page.tsx (1)
Page(18-52)app/[lang]/stats/page.tsx (1)
Page(14-35)src/i18n.ts (1)
Locale(6-6)
server/context.ts (1)
src/types/supabase.ts (1)
Database(9-318)
proxy.ts (2)
src/utils/assert.ts (1)
assert(7-14)src/types/supabase.ts (1)
Database(9-318)
app/[lang]/(common)/Header/index.tsx (2)
src/utils/supabase.ts (1)
getSupabaseBrowserClient(7-18)src/utils/theme.ts (1)
isDarkMode(23-49)
pages/api/plugins.ts (2)
src/types/types.ts (1)
PluginRow(8-8)app/[lang]/recent-list/TierRowItem.tsx (1)
Tier(6-14)
pages/api/news-letter.ts (1)
src/types/types.ts (1)
NewsLetterInsert(16-16)
🪛 Biome (2.1.2)
pages/api/plugins.ts
[error] 26-30: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 38-38: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
pages/api/news-letter.ts
[error] 31-31: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (47)
pages/api/github-stats-advanced.ts (1)
6-10: LGTM! Path aliases improve import clarity.The migration from relative imports to path aliases (
@/server/...,~/localization,~/utils/assert) enhances maintainability and aligns with the broader refactoring in this PR.app/[lang]/(home)/Hero/StatsUrlCards.tsx (1)
3-3: LGTM! Explicit ReactElement typing enhances type safety.Adding explicit
ReactElementimport and return type annotation improves code clarity and aligns with React 19 best practices. This change is consistent with similar typing improvements across other components in this PR.Also applies to: 30-30
app/[lang]/(common)/Dropdown.tsx (1)
20-20: Type refinements address library compatibility.The non-null assertion (
null!) and explicit generic type parameter (useOnClickOutside<HTMLDivElement>) are necessary adjustments for theusehooks-tslibrary upgrade. While the non-null assertion is a workaround, it's appropriate here since the ref is assigned before use (Line 38), and it's safer than theas anypattern previously flagged in reviews.Also applies to: 27-27
app/[lang]/(common)/GreatFrontEnd.tsx (1)
3-3: LGTM! Consistent typing improvements.The explicit
ReactElementimport and return type annotation align with the typing improvements applied consistently across components in this PR, enhancing type safety and code documentation.Also applies to: 20-20
.github/workflows/ci.yml (1)
36-39: LGTM! Essential CI updates for Next.js 16 migration.The changes appropriately address the migration requirements:
- Node.js upgrade to v20 is required for Next.js 16 (minimum 20.9)
- GitHub Actions upgrade to v4 is standard maintenance
- Bun command adjustments (
bun run lint,--runflag in tests) align with Bun v1 compatibilityAlso applies to: 52-52, 55-55
test/lru-cache.mock.ts (1)
1-23: LGTM! Well-designed test mock.The minimal no-op
LRUCachemock effectively prevents heavy dependencies during test execution while providing a functional interface. The additional mocks for CSS-related modules (@asamuzakjp/css-color,cssstyle) further optimize test performance by preventing transitive dependency loading.src/utils/theme.ts (1)
4-6: LGTM! Critical SSR guards for Next.js 16.The
typeof window === 'undefined'checks are essential for preventing runtime errors in SSR/SSG contexts. These guards ensure the theme utilities gracefully handle server-side execution wherewindow,document, andlocalStorageare unavailable, which is critical for Next.js 16's App Router.Also applies to: 24-26
test/css-color-shim.js (1)
1-16: LGTM! Test shim implementation is appropriate.The no-op LRUCache implementation serves its purpose as a lightweight test substitute. The methods correctly return appropriate default values (undefined, this, false) and the dual export pattern ensures compatibility with different import styles.
eslint.config.mjs (1)
1-9: LGTM! Modern ESLint flat config is correctly structured.The migration to ESLint's flat config format is properly implemented with appropriate ignores for generated files and correct integration of Next.js core web vitals rules.
package.json (2)
19-19: LGTM! Improved Supabase configuration management.Replacing the hard-coded
$SUPABASE_PROJECT_DOOBOOIO_STAGINGwith the generic$SUPABASE_PROJECT_IDenvironment variable improves flexibility and security.
24-86: No known security vulnerabilities detected for updated dependencies.Verification confirms that all major packages—next@16.0.5, react@19.2.0, react-dom@19.2.0, @supabase/supabase-js@2.86.0, @prisma/client@6, and prisma@6—are current and not deprecated. The project's yarn audit reports no security issues across the dependency tree.
app/[lang]/stats/[login]/Scouter/index.tsx (1)
41-46: LGTM! Prop rename is correctly applied.The prop rename from
onChangeStattoonChangeStatActionis properly implemented with no functional changes to the handler logic.pages/api/recent-users.ts (1)
4-6: LGTM! Import path aliases improve maintainability.The migration from relative imports to path aliases (
@/for server,~/for src) enhances code readability and simplifies future refactoring.pages/api/github-stats.ts (1)
8-12: LGTM! Consistent path alias usage.The import path changes align with the project-wide migration to path aliases, maintaining consistency across API routes.
pages/api/news-letter.ts (1)
2-3: LGTM! Consistent path alias adoption.Import paths correctly migrated to project-wide aliases.
app/[lang]/sign-in/SocialButtons.tsx (1)
26-35: LGTM: OAuth redirect flow correctly integrates with callback route.The dynamic redirect URL construction properly encodes the current pathname and aligns with the new OAuth callback handler at
app/auth/callback/route.ts. The memoization dependency onpathnameis appropriate.Also applies to: 41-48
server/context.ts (1)
14-14: LGTM: Supabase client type simplification.The updated generic signature
SupabaseClient<Database>is the correct pattern for Supabase clients and removes redundant schema type parameters. This aligns with the library's type definitions and the broader refactor across the codebase.app/[lang]/recent-list/page.tsx (1)
22-24: LGTM: Correct Next.js 15 async params pattern.The migration to Promise-based params with
await props.paramscorrectly implements the Next.js 15 requirement for async route parameters. The type assertion toLocaleis appropriate given the runtime constraints.pages/api/github-trophies.ts (1)
6-12: LGTM: Import path migration to aliases.The migration from relative imports to path aliases (
@/serverand~/localization,~/utils) improves maintainability and aligns with the project-wide refactor.app/[lang]/layout.tsx (1)
21-24: LGTM: Layout correctly adopts async params pattern.The layout properly awaits
props.paramsto extract the language parameter before rendering, consistent with Next.js 15's async route parameter handling. This ensures translations are loaded with the correct locale.app/[lang]/stats/[login]/page.tsx (1)
18-21: LGTM: Multi-parameter async route handling.The page correctly awaits and destructures both
langandloginparameters from the Promise, following Next.js 15 conventions for dynamic routes with multiple segments.pages/api/plugins.ts (1)
26-38: Excellent type safety improvements.The changes significantly enhance type safety:
- Explicit
PluginRow | nulltyping removes reliance on implicit inferencemaybeSingle()instead ofsingle()prevents throwing on missing data(data.json || []) as Tier[]is safer than direct type assertion and provides a fallbackThis directly addresses the previous review concern about defeating TypeScript's purpose with
as anycasts.app/auth/callback/route.ts (1)
10-42: LGTM: OAuth callback correctly implements Next.js 15 server-side auth flow.The route handler properly:
- Uses
await cookies()(Next.js 15 async cookies API)- Creates a server-side Supabase client with the correct adapter pattern
- Performs code-to-session exchange
- Implements language-aware redirects
This integrates well with the OAuth flow initiated in
SocialButtons.tsx.server/supabaseClient.ts (1)
9-9: LGTM!The simplified return type
SupabaseClient<Database>is correct and aligns with the updated type surface across the codebase. The'public'schema is the default, so the explicit generic arguments are unnecessary.src/utils/functions.ts (1)
81-86: LGTM!The extraction of
pluginTierswith explicit typing and the updatedgetTierNamecall improve type safety. The inline type assertion is appropriate for the JSON field.app/[lang]/stats/[login]/Scouter/StatsHeader.tsx (2)
18-25: LGTM!Good improvements:
- The
svg: ReactElement<{className?: string}>typing addresses the previous concern aboutas anycasts.- The prop rename to
onChangeStatActionfollows Next.js conventions for action callbacks.
128-132: LGTM!The nullish coalescing (
??) forstat.svg.props.classNamecorrectly handles undefined className props when merging classes.app/[lang]/(home)/Hero/StatsSymbol.tsx (3)
55-55: LGTM!The simplified destructuring
const {name, description} = statsInfo[selectedStatName]directly addresses the previous review concern about nested ternary operators and is much more readable.
98-106: Consider if 50ms transition duration achieves the desired effect.The
isTransitioningstate is set totrueand then immediately reset tofalseafter 50ms. Combined with the 300ms/500ms CSS transitions on lines 66 and 74, this very short window may cause the fade-out to barely start before reversing. If you want a noticeable fade-out-then-in effect, consider increasing the timeout to match or exceed the CSS transition duration.If the 50ms delay is intentional to trigger a quick transition reset, no changes needed.
108-114: LGTM!Proper cleanup of the timeout ref on unmount prevents memory leaks and stale state updates.
tsconfig.json (2)
20-20: LGTM!The
jsx: "react-jsx"setting is correct for the React 19 upgrade, enabling the automatic JSX runtime introduced in React 17+.
43-53: LGTM!Good adjustments:
- Including
.next/dev/types/**/*.tsensures Next.js generated types are available during development.- Excluding
vitest.config.tsis appropriate since it uses different module resolution.app/[lang]/loading.tsx (1)
17-22: LGTM! Props correctly adapted to lottie-react API.The component props (
animationData,loop,autoplay,style) are correctly set for thelottie-reactlibrary.vitest.config.ts (2)
11-16: LGTM! Path alias simplification.The alias configuration has been cleaned up by removing glob patterns in favor of direct directory paths, which is appropriate for the project structure.
20-20: I'll help you verify the review comment. Let me check if the mock file exists at the expected location.
<function_calls>
#!/bin/bashVerify the lru-cache mock file exists at the specified path
fd -t f 'lru-cache.mock.ts' test/ 2>/dev/null || echo "File not found in test/"
</function_calls>Please wait for the script results.
app/[lang]/(common)/Header/index.tsx (2)
236-238: Good use of memoization for Supabase client.Memoizing the Supabase client prevents unnecessary recreation on each render. The lazy initialization of
isDarkstate is also appropriate sinceisDarkMode()has proper SSR guards.
263-301: Improved session synchronization logic.The refactored auth state handling properly:
- Fetches initial session on mount
- Handles
SIGNED_OUT,INITIAL_SESSION,SIGNED_IN, andTOKEN_REFRESHEDevents- Cleans up subscription on unmount
One minor consideration:
applySessionaccessessession?.user.user_metadata?.user_name- ifsessionis non-null butuseris somehow undefined, this could throw. The current optional chaining handles this safely.app/[lang]/sign-in/page.tsx (1)
15-22: LGTM! Correct Next.js 15+ async params pattern.The migration to async route parameters is correctly implemented:
paramsis now typed asPromise<{lang: string}>- The promise is awaited before accessing properties
- The
langis properly cast toLocaleThis pattern is consistent with other pages in the codebase (
app/[lang]/page.tsx,app/[lang]/stats/page.tsx, etc.).app/[lang]/stats/page.tsx (1)
10-17: LGTM! Consistent async params pattern.The async route parameter handling follows the same correct pattern used throughout the codebase. The implementation properly awaits
props.paramsbefore accessinglangand casts it toLocale.app/[lang]/page.tsx (1)
10-15: LGTM! Correct Next.js 15 async params migration.The Promise-based params pattern correctly aligns with Next.js 15's async request APIs. The implementation is consistent with other page components in this PR.
src/types/supabase.ts (1)
43-43: LGTM! Standard Supabase type generation update.The
Relationships: []additions across all tables are consistent with the updated Supabase CLI type generation format. This is typically auto-generated and aligns with newer Supabase tooling.server/supabaseServerClient.ts (1)
19-25: Cookie handler may be incomplete for auth flows.The cookie configuration only implements
get. If this client is used in contexts that require modifying cookies (e.g., route handlers, server actions for auth), the missingsetandremovemethods could cause silent failures.If this client is strictly for read-only server components, this is acceptable. Otherwise, consider:
{ cookies: { get(name: string) { return cookieStore.get(name)?.value; }, + set(name: string, value: string, options: CookieOptions) { + cookieStore.set({name, value, ...options}); + }, + remove(name: string, options: CookieOptions) { + cookieStore.delete({name, ...options}); + }, }, }server/services/githubService.ts (1)
5-15: Good type alias adoption.The migration to explicit type aliases (
PluginRow,UserPluginRow,StatsInsert, etc.) improves code clarity and type safety compared to the previousModel['table']['Row']pattern.next.config.js (2)
1-7: Good conditional PWA setup.The environment-based PWA toggle improves development experience by avoiding service worker interference during local development while maintaining production capability.
12-27: LGTM! Correct SVG handling for both bundlers.Having separate SVG configurations for Turbopack (lines 12-19) and Webpack (lines 20-27) is correct since they are independent build systems. This ensures SVG imports work regardless of which bundler is used (
next devvsnext dev --turbo).src/types/types.ts (1)
7-16: Typed table aliases are clear and consistentThe table-specific
Row/Insertaliases overModelnicely centralize Supabase types and improve readability across the codebase; no issues from my side here.proxy.ts (1)
2-3: Supabase server client & cookie adapter look good; small env message nitThe
createServerClient<Database>setup with explicit env asserts and acookiesadapter that mirrorsreq.cookiestores.cookiesis a solid pattern and should keep auth state in sync across middleware responses. One minor polish point: the second assert message says"SUPABASE_API_KEY is not defined"while the actual variable isNEXT_PUBLIC_SUPABASE_ANON_KEY; consider aligning the message to reduce confusion when diagnosing config issues.Given this is tightly coupled to
@supabase/auth-helpers-nextjs, please ensure thecookiesobject shape (getAll/setAll) matches what your installed version ofcreateServerClientexpects and that TypeScript isn’t flagging any type mismatches.Also applies to: 7-9, 12-13, 39-55
| const oauthRedirectUrl = useMemo(() => { | ||
| if (!process.env.NEXT_PUBLIC_ROOT_URL) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const normalizedRoot = process.env.NEXT_PUBLIC_ROOT_URL.replace(/\/$/, ''); | ||
|
|
||
| return `${normalizedRoot}/auth/callback?next=${encodeURIComponent(pathname || '/')}`; | ||
| }, [pathname]); |
There was a problem hiding this comment.
Handle missing NEXT_PUBLIC_ROOT_URL more gracefully.
When NEXT_PUBLIC_ROOT_URL is not set, oauthRedirectUrl becomes undefined, which is passed to Supabase's signInWithOAuth. While the OAuth flow may still work with default behavior, this could lead to users being redirected to an unexpected location after authentication.
Consider adding a fallback or logging a warning:
const oauthRedirectUrl = useMemo(() => {
if (!process.env.NEXT_PUBLIC_ROOT_URL) {
+ console.warn('NEXT_PUBLIC_ROOT_URL is not set; OAuth redirect may not work as expected');
return undefined;
}
const normalizedRoot = process.env.NEXT_PUBLIC_ROOT_URL.replace(/\/$/, '');
return `${normalizedRoot}/auth/callback?next=${encodeURIComponent(pathname || '/')}`;
}, [pathname]);📝 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 oauthRedirectUrl = useMemo(() => { | |
| if (!process.env.NEXT_PUBLIC_ROOT_URL) { | |
| return undefined; | |
| } | |
| const normalizedRoot = process.env.NEXT_PUBLIC_ROOT_URL.replace(/\/$/, ''); | |
| return `${normalizedRoot}/auth/callback?next=${encodeURIComponent(pathname || '/')}`; | |
| }, [pathname]); | |
| const oauthRedirectUrl = useMemo(() => { | |
| if (!process.env.NEXT_PUBLIC_ROOT_URL) { | |
| console.warn('NEXT_PUBLIC_ROOT_URL is not set; OAuth redirect may not work as expected'); | |
| return undefined; | |
| } | |
| const normalizedRoot = process.env.NEXT_PUBLIC_ROOT_URL.replace(/\/$/, ''); | |
| return `${normalizedRoot}/auth/callback?next=${encodeURIComponent(pathname || '/')}`; | |
| }, [pathname]); |
🤖 Prompt for AI Agents
In app/[lang]/sign-in/SocialButtons.tsx around lines 27 to 35, oauthRedirectUrl
is set to undefined when NEXT_PUBLIC_ROOT_URL is missing which is then passed to
signInWithOAuth; change the logic to never return undefined by providing a
fallback root (e.g. use window.location.origin when available, or default to an
empty string or '/'), construct the redirect URL from that fallback plus
pathname, and add a console.warn (or use your app logger) when
NEXT_PUBLIC_ROOT_URL is absent so the missing config is visible in logs.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
proxy.ts (1)
60-62: AwaitupsertUserto avoid unhandled async work in middleware.
upsertUseris very likely async (it probably does a Supabase write), but it’s called withoutawait. That means:
- The mutation can still be running after the middleware finishes, and
- Any rejection becomes an unhandled promise, which may be logged noisily or even treated as a failure in some runtimes.
Prefer awaiting it and, ideally, handling/logging errors explicitly:
- if (user) { - upsertUser({supabase, user}); - } + if (user) { + await upsertUser({supabase, user}); + }If you intentionally want fire‑and‑forget behavior here, consider at least attaching a
.catchto avoid unhandled rejections.server/services/githubService.ts (1)
597-620: Background refresh condition appears inverted vs comment, leading to stale data.The comment says “When user was queried after 3 hours, update the data in background,” but the condition is
if (!updatedAt || diffHours(updatedAt, today) < 3) { void upsertGithubStats(...); isCachedResult = true; }. As written, you refresh aggressively in the first 3 hours and then never refresh again, so long‑lived users will serve permanently stale stats. This likely wants>= 3(or similar) to trigger refresh when data is older than 3 hours.
🧹 Nitpick comments (5)
src/prisma.ts (1)
41-49: MissingfindUniqueandfindUniqueOrThrowhandlers.The soft-delete filter is not applied to
findUniqueorfindUniqueOrThrowqueries. This means calls likeprisma.users.findUnique({ where: { id: 1 } })can return soft-deleted records, which may not be the expected behavior.count({model, args, query}) { return query(appendNotDeletedFilter(model, args || {})); }, + findUnique({model, args, query}) { + return query(appendNotDeletedFilter(model, args || {})); + }, + findUniqueOrThrow({model, args, query}) { + return query(appendNotDeletedFilter(model, args || {})); + }, delete({model, args}) {app/auth/callback/route.ts (2)
16-35: Align Supabase env handling with other clients (add asserts instead of|| '').Here you’re passing
process.env.NEXT_PUBLIC_SUPABASE_URL || ''and...ANON_KEY || ''intocreateServerClient. If either env var is misconfigured, you’ll get a harder‑to‑diagnose runtime failure inside Supabase rather than an early, explicit error, and this route will still attempt to run.For consistency with
getSupabaseBrowserClientandgetSupabaseServerComponentClient, consider asserting the env vars first and then passing the non‑nullable values intocreateServerClient. Reusing the sameasserthelper you already introduced would keep behavior uniform across all Supabase entry points.
37-40: Preserve the requested locale on auth error redirect.On OAuth failure you always redirect to
/${i18n.defaultLocale}/sign-in. To keep behavior consistent with the success path and user expectations, consider preferringlangParamwhen present, e.g./${langParam ?? i18n.defaultLocale}/sign-in?error=auth_failed.proxy.ts (1)
70-75: Make locale redirect fully defensive against undefined locale.
getLocaleis typed as returningstring | undefined. While the currentmatchLocalecall passesi18n.defaultLocaleand should always return something at runtime, you could make the redirect more robust and self-documenting by defaulting explicitly:const locale = getLocale(req) ?? i18n.defaultLocale; return NextResponse.redirect(`${origin}/${locale}/${pathname}`);This also keeps things future-proof if
getLocaleis ever changed.server/services/githubService.ts (1)
325-461: Typed payloads +Promise.allfix the asyncforEachissue; consider minimal error handling.The refactor of
upsertGithubStatsto usePluginRow/UserPluginRow,StatsInsert[], andTrophiesInsertplusPromise.allovertrophiesandstatslooks solid and resolves the earlier un-awaited asyncforEachproblem. You’re still ignoring the{ error }returned from eachupsert, though, so silent failures will be hard to debug; consider at least destructuring the result and logging or short‑circuiting onerrorwhere appropriate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
public/assets/scouter.svgis excluded by!**/*.svgyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
app/auth/callback/route.ts(1 hunks)package.json(1 hunks)pages/api/news-letter.ts(3 hunks)proxy.ts(2 hunks)server/services/githubService.ts(8 hunks)server/supabaseClient.ts(1 hunks)server/supabaseServerClient.ts(1 hunks)src/prisma.ts(1 hunks)src/utils/functions.ts(2 hunks)src/utils/supabase.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/utils/functions.ts
- server/supabaseClient.ts
- pages/api/news-letter.ts
🧰 Additional context used
🧬 Code graph analysis (5)
src/utils/supabase.ts (2)
src/types/supabase.ts (1)
Database(9-318)src/utils/assert.ts (1)
assert(7-14)
proxy.ts (2)
src/utils/assert.ts (1)
assert(7-14)src/types/supabase.ts (1)
Database(9-318)
server/supabaseServerClient.ts (2)
src/types/supabase.ts (1)
Database(9-318)src/utils/assert.ts (1)
assert(7-14)
server/services/githubService.ts (4)
src/types/types.ts (7)
PluginRow(8-8)UserPluginRow(10-10)StatsInsert(13-13)UserPluginInsert(11-11)TrophiesInsert(15-15)Model(5-5)StatsRow(12-12)server/plugins/index.ts (1)
getGithubStatus(66-93)src/types/supabase.ts (1)
Json(1-7)server/plugins/pluginUtils.ts (1)
diffHours(52-57)
app/auth/callback/route.ts (2)
src/types/supabase.ts (1)
Database(9-318)src/i18n.ts (1)
i18n(1-4)
🔇 Additional comments (19)
package.json (8)
8-9: Tailwind CLI flag simplifications look good.The short-form flags (-i, -o, -w) are functionally equivalent to the previous flags and modernize the command syntax. Paths are preserved correctly.
12-13: Verify ESLint 9 configuration compatibility.The lint scripts use the
--extflag with ESLint 9.39.1. Next.js 16.0.5 is a valid release, but verify that the--extflag is still the recommended approach for ESLint 9.x with flat config. Modern ESLint may prefer explicit file patterns over the--extflag. Additionally, lines 12 and 13 appear to be duplicates; clarify if this is intentional or if one should be removed.
19-19: Good practice: Environment variable for Supabase project ID.Using
$SUPABASE_PROJECT_IDmakes the setup environment-agnostic and prevents hard-coded staging references. Ensure this variable is properly defined in all deployment environments (dev, staging, prod).
38-38: Next.js 16.0.5 version is valid and current.The version 16.0.5 is a valid public release, published as the latest stable version. The previous review comment was incorrect—this is a real, stable release. The caret range allows for patches and minor updates, which is appropriate.
41-41: React 19.2.0 versions are appropriate for Next.js 16.Both react and react-dom are pinned to ^19.2.0, which aligns with Next.js 16's React 19.2 feature set and requirements. The versions are consistent and appropriate.
Also applies to: 43-43
28-28: Verify Prisma 6 compatibility with Next.js 16.Both
@prisma/clientandprismaare bumped to ^6, a major version change. Next.js 16 includes various features and updates, but verify that Prisma 6 has no breaking changes incompatible with Next.js 16, the type system changes, or other dependencies in the project.Also applies to: 80-80
49-49: Clarify TypeScript version pinning strategy.Line 49 pins TypeScript to an exact version (5.9.3) instead of using a caret range like most other dependencies. This is unusual unless there's a specific reason (reproducibility, known issues). Verify this is intentional, as it may complicate future patch updates. Also confirm compatibility with the pinned
@types/react@19.2.7and@types/react-dom@19.2.3on lines 61 and 63.
24-50: Major dependency ecosystem modernization.This PR includes significant version bumps across the stack: Next.js 15→16, React 18→19.2, Prisma 5→6, ESLint 8→9, and updates to testing libraries, Supabase packages, and tooling. While the versions are current and well-maintained, ensure that:
- All breaking changes have been addressed in corresponding code files (Next.js middleware→proxy, ESLint flat config, etc.)
- New packages (e.g.,
lottie-react@^2.4.1,usehooks-ts@^3.1.1) are properly integrated- The Supabase SSR package (
@supabase/ssr@^0.8.0) is correctly configured for Next.js 16The AI summary mentions ESLint flat config migration and Supabase refactoring, which aligns with these dependency changes.
Also applies to: 53-85
src/prisma.ts (5)
1-12: Clean extension setup.Good use of
Prisma.ModelNamefor type-safe model tracking and the Set for O(1) lookups.
14-35: Well-designed filter helper.The helper correctly preserves existing
whereconditions and respects explicitdeleted_atfilters, allowing intentional queries of soft-deleted records when needed.
50-58: Previous issue resolved.The handler now correctly extracts only the
whereclause from delete args. Note that if callers useselectorincludeoptions on delete (rare), those would be lost in the update conversion.
60-69: Previous issue resolved.The handler now correctly constructs the
updateManycall with onlywhereanddataproperties.
74-96: LGTM.The type cast maintains API compatibility, and the singleton pattern correctly prevents multiple client instances during development hot-reloads.
app/auth/callback/route.ts (1)
43-48: Locale-aware success redirect looks good.The
redirectPathcomputation now prefers a safe relativenextpath and otherwise falls back to/${langParam || i18n.defaultLocale}, which addresses the earlier locale preservation concern while avoiding open redirects.src/utils/supabase.ts (1)
7-17: Browser Supabase client helper is consistent and fail-fast.Using
assertforNEXT_PUBLIC_SUPABASE_URL/NEXT_PUBLIC_SUPABASE_ANON_KEYand then constructing a typedcreateBrowserClient<Database>keeps the browser client aligned with your server helpers and will surface misconfigurations clearly at runtime. Looks good.server/supabaseServerClient.ts (1)
10-31: Server component Supabase client migration looks correct.The async
getSupabaseServerComponentClientwith env asserts plus acookiesadapter exposinggetAll/setAllis the idiomatic pattern for@supabase/ssrand aligns with your other Supabase helpers. The generalized return typeSupabaseClient<Database>also simplifies downstream typing without losing safety.proxy.ts (1)
29-55: Verify this proxy is actually wired as Next.js middleware.The
proxyfunction plus exportedconfig.matcherhas the shape of a Next.js middleware, but this file is namedproxy.ts. Next will only auto-run middleware frommiddleware.ts(in the root orsrc/). Please ensure you either:
- Have a
middleware.tsthat re-exports thisproxyfunction andconfig, or- Rename/move this file so that Next picks it up directly.
Otherwise, the Supabase/session and locale logic here won’t execute.
server/services/githubService.ts (2)
5-15: Good move centralizing Supabase typings.Importing
Jsonand the row/insert aliases fromsrc/typeskeeps this service aligned with your generated DB types and removes pressure to fall back toas any.
513-535: Supabase query typings are consistent and type‑safe.The explicit
{ data: plugin },{ data: userPlugin }, and{ data: stats }typings usingModel['plugins']['Row'],UserPluginRow, andStatsRow[]match Supabase’s response shapes and make downstream usage ofplugin,userPlugin, andstatsmuch clearer.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Performance & Infrastructure
✏️ Tip: You can customize this high-level summary in your review settings.