Add next-intl internationalization and remove legacy routes#36
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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. WalkthroughAdds full internationalization: routing, middleware, request/message loading, locale-aware layouts/pages/components, language switcher, message bundles for six locales, updated proxy/redirect logic, Next.js config plugin integration, and new localized pricing UI. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as User/Browser
participant Middleware as next-intl Middleware (proxy.ts)
participant ProxyRedirect as proxy-redirect (lib/proxy-redirect.ts)
participant NextApp as Next.js App Router
participant LocaleLayout as app/[locale]/layout.tsx
participant RequestCfg as i18n/request.ts
participant MsgFS as messages/*.json
participant UI as Page/Components (useTranslations)
Browser->>Middleware: GET /pt/some-path
Middleware->>ProxyRedirect: check path, skip API/TRPC
ProxyRedirect->>NextApp: route to /[locale] with params.locale="pt"
NextApp->>LocaleLayout: render with params { locale: "pt" }
LocaleLayout->>RequestCfg: setRequestLocale + getRequestConfig
RequestCfg->>MsgFS: dynamic import messages/pt.json (fallback to en)
MsgFS-->>RequestCfg: messages object
RequestCfg-->>LocaleLayout: { locale, messages }
LocaleLayout->>UI: Wrap children with NextIntlClientProvider(messages)
UI->>UI: useTranslations(...) -> localized strings
UI-->>Browser: render localized response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 9
🧹 Nitpick comments (4)
lib/proxy-redirect.ts (1)
3-6: Import LOCALES from the i18n routing configuration to avoid duplication.The
LOCALESarray defined in lib/proxy-redirect.ts duplicates the locales list from i18n/routing.ts. Maintaining separate definitions risks inconsistency when locales are added or removed. Importrouting.localesfrom i18n/routing.ts instead.app/layout.tsx (1)
11-39: Localize metadata for supported locales.The app supports 6 locales (en, pt, de, zh, es, nl) via
app/[locale]/routing.tsx, butapp/layout.tsxserves hardcoded English metadata across all locale routes. Either generate metadata dynamically inapp/layout.tsxusing locale params, or add locale-specific metadata exports toapp/[locale]/layout.tsx. Search engines rely on localized metadata to properly index multilingual content.components/marketing/founders-letter.tsx (1)
44-49: Consider using the i18n-aware Link component for internal navigation.The feedback link was changed from Next.js
Linkto a standard anchor tag. If this is an internal route that should support locale prefixes, consider using the i18n-awareLinkfrom@/i18n/navigationinstead. However, ifhttps://smryai.userjot.com/is an external service, the current implementation is correct.components/marketing/faq.tsx (1)
45-57: Consider using next-intl's rich text formatting for cleaner code interpolation.The current string split/map pattern to inject the
<code>element is fragile because it relies on the translator preserving the exact{code}placeholder text. If a translator modifies or removes this marker in the translation file, the code will break silently or display incorrectly.
next-intlsupports rich text formatting that provides a more robust solution for interpolating React components into translated strings.🔎 Recommended approach using rich text
Update the translation call to use component interpolation:
<li> - {t("a8Option1", { - code: "http://smry.ai/", - example: "http://smry.ai/https://www.wsj.com/..." - }).split("{code}").map((part, i) => - i === 0 ? part : ( - <span key={i}> - <code className="rounded bg-yellow-100 px-1 py-0.5 font-mono text-xs text-neutral-800 dark:bg-yellow-900 dark:text-neutral-200"> - http://smry.ai/ - </code> - {part} - </span> - ) - )} + {t.rich("a8Option1", { + code: (chunks) => ( + <code className="rounded bg-yellow-100 px-1 py-0.5 font-mono text-xs text-neutral-800 dark:bg-yellow-900 dark:text-neutral-200"> + {chunks} + </code> + ) + })} </li>Then update the translation in
messages/en.json:{ "faq": { "a8Option1": "Prepend <code>http://smry.ai/</code> before the URL (e.g., http://smry.ai/https://www.wsj.com/...)" } }This approach is type-safe, doesn't break if translators reword the text, and is the recommended pattern for component interpolation in
next-intl.Reference: next-intl rich text formatting
📜 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 (26)
app/[locale]/layout.tsx(1 hunks)app/[locale]/page.tsx(9 hunks)app/[locale]/pricing/page.tsx(1 hunks)app/globals.css(1 hunks)app/layout.tsx(2 hunks)app/pricing/page.tsx(0 hunks)components/layout/site-footer.tsx(6 hunks)components/marketing/banner.tsx(2 hunks)components/marketing/bookmarklet.tsx(2 hunks)components/marketing/custom-pricing-table.tsx(1 hunks)components/marketing/faq.tsx(2 hunks)components/marketing/founders-letter.tsx(2 hunks)components/shared/language-switcher.tsx(1 hunks)i18n/navigation.ts(1 hunks)i18n/request.ts(1 hunks)i18n/routing.ts(1 hunks)lib/proxy-redirect.ts(2 hunks)messages/de.json(1 hunks)messages/en.json(1 hunks)messages/es.json(1 hunks)messages/nl.json(1 hunks)messages/pt.json(1 hunks)messages/zh.json(1 hunks)next.config.js(2 hunks)package.json(1 hunks)proxy.ts(2 hunks)
💤 Files with no reviewable changes (1)
- app/pricing/page.tsx
🧰 Additional context used
🧬 Code graph analysis (6)
components/shared/language-switcher.tsx (2)
i18n/routing.ts (2)
Locale(9-9)routing(3-7)components/ui/select.tsx (4)
Select(170-170)SelectTrigger(171-171)SelectValue(172-172)SelectItem(175-175)
app/[locale]/layout.tsx (1)
i18n/routing.ts (1)
routing(3-7)
proxy.ts (2)
i18n/routing.ts (1)
routing(3-7)lib/proxy-redirect.ts (1)
buildProxyRedirectUrl(87-149)
app/[locale]/pricing/page.tsx (2)
components/marketing/custom-pricing-table.tsx (1)
CustomPricingTable(15-105)components/marketing/founders-letter.tsx (1)
FoundersLetter(9-55)
i18n/navigation.ts (1)
i18n/routing.ts (1)
routing(3-7)
i18n/request.ts (1)
i18n/routing.ts (1)
routing(3-7)
🔇 Additional comments (27)
messages/nl.json (1)
1-145: LGTM!The Dutch localization file is well-structured with complete translations across all namespaces. Currency is appropriately localized to euros (line 68).
lib/proxy-redirect.ts (2)
33-52: LGTM!The locale-aware routing logic correctly handles locale-prefixed paths, exact locale matches, and falls back appropriately for non-locale paths.
54-70: LGTM!Clean extraction of the route-matching logic into a reusable helper. The root path skip is correctly placed since it's handled by the caller.
messages/zh.json (1)
1-145: LGTM!The Chinese localization file is complete with properly structured Simplified Chinese translations across all namespaces.
app/globals.css (1)
222-250: LGTM!The Clerk checkout button styles integrate well with the existing design system by using CSS variables, ensuring consistent theming in both light and dark modes.
package.json (1)
57-57: next-intl 4.6.1 is compatible with Next.js 16.0.7 and React 19.2.1.next-intl is designed for Next.js App Router, Server Components, and static rendering. Next.js 16 includes React 19.2 features, and a blog post from November 2025 explores the compatibility between next-intl and Next.js 16's 'use cache' feature, noting that next-intl is already prepared for this transition. No known incompatibilities exist for this version combination.
components/marketing/banner.tsx (2)
1-4: LGTM! Correct i18n setup.The addition of "use client" directive is required for the
useTranslationshook, and the import is properly placed.
82-90: LGTM! Translation integration is correct.The heading text is properly internationalized using the "banner" namespace. The translation hook is correctly initialized and used.
i18n/routing.ts (1)
1-9: LGTM! Clean i18n routing configuration.The routing setup follows next-intl best practices. The
localePrefix: 'as-needed'setting means the default locale (en) will be accessible without a URL prefix (e.g.,/instead of/en), while other locales will require prefixes (e.g.,/pt,/de).components/marketing/bookmarklet.tsx (2)
4-7: LGTM! Proper i18n integration.The
useTranslationshook is correctly imported and initialized for the "bookmarklet" namespace.
28-31: LGTM! UI text properly internationalized.Both the title tooltip and link text are correctly using translation keys from the bookmarklet namespace.
components/layout/site-footer.tsx (2)
1-12: LGTM! Correct client component conversion for i18n.The component is properly converted to a client component with the "use client" directive, which is required for the
useTranslationshook. Using two separate translation namespaces ("footer" and "common") is a good practice for organizing translations.
22-74: LGTM! All UI strings properly internationalized.All static text has been correctly replaced with translation keys from the appropriate namespaces, maintaining the component's existing structure and functionality.
app/layout.tsx (1)
9-9: LGTM! Correct async pattern for locale resolution.Converting RootLayout to async and using
getLocale()from next-intl/server is the correct pattern for Next.js 15. This allows the root HTML element's lang attribute to be set dynamically based on the resolved locale.Also applies to: 41-46
components/shared/language-switcher.tsx (2)
51-51: ThealignItemWithTriggerprop is valid and correctly supported. The project uses Base UI's Select component, where this prop is properly defined in theSelectPopupfunction (aliased asSelectContent) and passed through to the underlyingSelectPrimitive.Positionercomponent with correct TypeScript types.Likely an incorrect or invalid review comment.
15-31: Locale coverage is correctly synchronized.The 6 locales (en, pt, de, zh, es, nl) defined in
i18n/routing.tshave complete and matching entries in bothlanguageNamesandlanguageFlagsinlanguage-switcher.tsx. Coverage remains synchronized.i18n/navigation.ts (1)
1-5: LGTM!The navigation setup correctly creates locale-aware navigation utilities from the routing configuration. This provides a clean abstraction for i18n-aware navigation throughout the application.
proxy.ts (2)
13-16: LGTM! API route exclusion is correctly implemented.The early return for API and TRPC routes prevents unnecessary i18n processing and ensures these routes bypass locale detection. This is the correct approach for keeping API routes locale-agnostic.
45-54: Matcher configuration is correct and follows best practices—no conflict exists.The patterns work as intended: the matcher config allows full regex so matching like negative lookaheads or character matching is supported. Line 51's negative lookahead pattern excludes
/apifrom the general i18n routing catch-all, while line 53's explicit/(api|trpc)(.*)pattern ensures API routes still match the matcher for Clerk authentication. In Next.js, matcher allows you to filter Middleware to run on specific paths using OR logic—any matching pattern triggers the proxy function once. The early return on lines 13-16 then skips i18n middleware for API routes, preventing duplicate processing. This pattern matches Clerk's recommended configuration: a negative lookahead pattern combined with an explicit API/trpc pattern.messages/es.json (1)
1-145: All locale files have consistent translation keys across all languages.Verification confirms that en.json, es.json, pt.json, de.json, zh.json, and nl.json each contain exactly 119 translation keys with no missing or extra keys between any locale files. No runtime errors related to key inconsistency will occur.
Likely an incorrect or invalid review comment.
next.config.js (1)
1-4: next-intl@4.6.1 is fully compatible with Next.js 15 and React 19.The latest version 4.6.1 was published 3 days ago, and production-ready implementations confirm next-intl works seamlessly with Next.js 15. Early RC-stage compatibility issues from May 2024 have been resolved in stable releases. The plugin integration in your configuration is correct.
messages/en.json (1)
46-48: Verify interpolation variable handling in FAQ components.Lines 46-48 contain interpolation variables like
{code}and{example}. Ensure the FAQ component consuming these translations provides the required values using next-intl's pattern of passing an object with values, e.g.,t('key', { code: value, example: value }).app/[locale]/layout.tsx (1)
11-31: LGTM! Locale layout structure follows next-intl best practices.The implementation correctly:
- Generates static params for all supported locales
- Validates the locale parameter and returns 404 for invalid locales
- Sets the request locale for next-intl's server-side utilities
- Provides messages to client components via NextIntlClientProvider
components/marketing/faq.tsx (1)
9-66: LGTM! FAQ successfully migrated to use translations.The component correctly integrates
next-intlfor all FAQ content, including questions, answers, and footer text. The translation keys are well-structured and the dynamic content generation preserves the original functionality.app/[locale]/page.tsx (3)
38-56: LGTM! SupportLink correctly uses translations.The component properly integrates the translation hook while maintaining the existing visibility logic for premium users and loading states.
58-248: LGTM! Home page successfully internationalized.The main page component has been properly migrated to use
next-intl:
- Translation hooks for both "home" and "common" namespaces
- Locale-aware
Linkcomponent fromi18n/navigationLanguageSwitcheradded to the header for user language selection- All user-facing strings replaced with translation calls
- Error messages and validation text properly translated
The implementation maintains the existing functionality while adding comprehensive i18n support.
24-26: Translation key verification complete.All translation keys referenced in the home page (support, validationError, tagline, tryIt, placeholder, by, prepend, toAnyUrl, bookmarkletTip, bookmarkletInstructions from "home" namespace and smryLogo from "common" namespace) exist in every locale message file (en.json, pt.json, de.json, zh.json, es.json, nl.json). No runtime errors will occur from missing translations.
Greptile SummaryThis PR successfully integrates Key Changes
Previous Issues AddressedAll issues from previous review threads have been resolved:
Remaining ConsiderationRoot layout metadata (app/layout.tsx:11) remains hardcoded in English for SEO purposes, which is acceptable for a single canonical version but could be enhanced with per-locale metadata in the future. Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Middleware as proxy.ts
participant ClerkAuth as Clerk Middleware
participant ProxyRedirect as buildProxyRedirectUrl
participant IntlMiddleware as next-intl Middleware
participant LocaleLayout as [locale]/layout.tsx
participant Page as [locale]/page.tsx
User->>Middleware: Request /{locale}/path or /domain.com
Middleware->>ClerkAuth: Process auth
ClerkAuth-->>Middleware: Auth complete
alt API/TRPC route
Middleware-->>User: NextResponse.next()
else URL slug (e.g., /nytimes.com)
Middleware->>ProxyRedirect: buildProxyRedirectUrl(pathname, search, origin)
ProxyRedirect->>ProxyRedirect: Check isAppRoute() with locale awareness
ProxyRedirect->>ProxyRedirect: Parse & normalize URL
ProxyRedirect-->>Middleware: /proxy?url=...
Middleware-->>User: Redirect to proxy route
else Regular app route
Middleware->>IntlMiddleware: Handle locale routing
IntlMiddleware->>IntlMiddleware: Detect/validate locale
IntlMiddleware-->>User: Redirect or continue with locale
User->>LocaleLayout: Render layout
LocaleLayout->>LocaleLayout: Load messages for locale
LocaleLayout->>Page: Render with NextIntlClientProvider
Page->>Page: useTranslations() hooks
Page-->>User: Localized content
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export default function PricingPage() { | ||
| return ( | ||
| <main className="flex min-h-screen flex-col bg-background"> | ||
| {/* Header */} | ||
| <header className="border-b border-border bg-background/80 backdrop-blur-xl"> | ||
| <div className="mx-auto flex max-w-4xl items-center justify-between px-4 py-4"> | ||
| <Link | ||
| href="/" | ||
| className="flex items-center gap-2 text-sm text-muted-foreground hover:text-foreground transition-colors" | ||
| > | ||
| <ArrowLeft className="size-4" /> | ||
| Back to SMRY | ||
| </Link> | ||
| <div className="flex items-center gap-4"> | ||
| <SignedIn> | ||
| <UserButton | ||
| appearance={{ | ||
| elements: { | ||
| avatarBox: "size-8" | ||
| } | ||
| }} | ||
| /> | ||
| </SignedIn> | ||
| <Link href="/"> | ||
| <Image | ||
| src="/logo.svg" | ||
| width={80} | ||
| height={80} | ||
| alt="smry logo" | ||
| className="dark:invert" | ||
| /> | ||
| </Link> | ||
| </div> | ||
| </div> | ||
| </header> | ||
|
|
||
| {/* Content */} | ||
| <div className="flex flex-1 flex-col items-center px-4 py-10"> | ||
| {/* Hero Section */} | ||
| <div className="mb-8 w-full max-w-lg"> | ||
| <div className="p-0.5 bg-accent rounded-[14px]"> | ||
| <div className="bg-card rounded-xl p-6 text-center"> | ||
| <div className="mx-auto mb-4 flex size-10 items-center justify-center rounded-full bg-accent"> | ||
| <Zap className="size-4 text-foreground/70" /> | ||
| </div> | ||
| <h1 className="text-xl font-semibold">Read Any Article, Instantly</h1> | ||
| <p className="mt-2 text-sm text-muted-foreground"> | ||
| Stop paying $50+/month for multiple subscriptions. Get unlimited access to articles from NYT, WSJ, Bloomberg, and 1000+ sites. | ||
| </p> | ||
| <div className="mt-4 flex items-center justify-center gap-2 text-xs text-muted-foreground"> | ||
| <span className="px-2 py-1 bg-accent rounded-md font-medium">7-day free trial</span> | ||
| <span>·</span> | ||
| <span>Cancel anytime</span> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| {/* Pricing Options */} | ||
| <div className="w-full max-w-xl mb-10"> | ||
| <CustomPricingTable /> | ||
| </div> | ||
|
|
||
| {/* Benefits */} | ||
| <div className="w-full max-w-2xl mb-8"> | ||
| <div className="grid grid-cols-1 sm:grid-cols-3 gap-3"> | ||
| <div className="p-0.5 bg-accent rounded-[14px]"> | ||
| <div className="bg-card rounded-xl p-4 h-full"> | ||
| <div className="flex items-center gap-2 mb-2"> | ||
| <Zap className="size-4 text-muted-foreground" /> | ||
| <span className="text-sm font-medium">Unlimited Summaries</span> | ||
| </div> | ||
| <p className="text-xs text-muted-foreground"> | ||
| No daily limits. Read as much as you want, whenever you want. | ||
| </p> | ||
| </div> | ||
| </div> | ||
| <div className="p-0.5 bg-accent rounded-[14px]"> | ||
| <div className="bg-card rounded-xl p-4 h-full"> | ||
| <div className="flex items-center gap-2 mb-2"> | ||
| <Search className="size-4 text-muted-foreground" /> | ||
| <span className="text-sm font-medium">Full History</span> | ||
| </div> | ||
| <p className="text-xs text-muted-foreground"> | ||
| Never lose an article. Search and revisit everything you've read. | ||
| </p> | ||
| </div> | ||
| </div> | ||
| <div className="p-0.5 bg-accent rounded-[14px]"> | ||
| <div className="bg-card rounded-xl p-4 h-full"> | ||
| <div className="flex items-center gap-2 mb-2"> | ||
| <Sparkles className="size-4 text-muted-foreground" /> | ||
| <span className="text-sm font-medium">Clean Reading</span> | ||
| </div> | ||
| <p className="text-xs text-muted-foreground"> | ||
| No ads, no distractions. Just the content you came for. | ||
| </p> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Value Comparison */} | ||
| <div className="w-full max-w-2xl mb-8"> | ||
| <div className="p-0.5 bg-accent rounded-[14px]"> | ||
| <div className="bg-card rounded-xl p-4"> | ||
| <p className="text-xs font-medium text-muted-foreground uppercase tracking-wide mb-3"> | ||
| The math | ||
| </p> | ||
| <div className="space-y-2 text-sm"> | ||
| <div className="flex justify-between items-center py-1 border-b border-border"> | ||
| <span className="text-muted-foreground">NYT</span> | ||
| <span className="line-through text-muted-foreground">$17/mo</span> | ||
| </div> | ||
| <div className="flex justify-between items-center py-1 border-b border-border"> | ||
| <span className="text-muted-foreground">WSJ</span> | ||
| <span className="line-through text-muted-foreground">$12/mo</span> | ||
| </div> | ||
| <div className="flex justify-between items-center py-1 border-b border-border"> | ||
| <span className="text-muted-foreground">The Atlantic</span> | ||
| <span className="line-through text-muted-foreground">$6/mo</span> | ||
| </div> | ||
| <div className="flex justify-between items-center py-1 border-b border-border"> | ||
| <span className="text-muted-foreground">Bloomberg</span> | ||
| <span className="line-through text-muted-foreground">$35/mo</span> | ||
| </div> | ||
| <div className="flex justify-between items-center pt-2"> | ||
| <span className="font-medium">SMRY Premium</span> | ||
| <span className="font-medium text-foreground">All of the above</span> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Founder's Letter */} | ||
| <div className="w-full max-w-2xl mb-8"> | ||
| <FoundersLetter /> | ||
| </div> | ||
|
|
||
| {/* Trust signals */} | ||
| <div className="flex flex-wrap items-center justify-center gap-4 text-xs text-muted-foreground mb-8"> | ||
| <span>7-day free trial</span> | ||
| <span className="text-border">•</span> | ||
| <span>Cancel anytime</span> | ||
| <span className="text-border">•</span> | ||
| <span>No questions asked</span> | ||
| </div> | ||
| </div> | ||
| </main> | ||
| ); | ||
| } |
There was a problem hiding this comment.
logic: Pricing page is not using next-intl translations
All text in this file is hardcoded in English. Should use useTranslations("pricing") hook and translation keys from the messages files like other pages do.
| {/* Header */} | ||
| <header className="border-b border-border bg-background/80 backdrop-blur-xl"> | ||
| <div className="mx-auto flex max-w-4xl items-center justify-between px-4 py-4"> | ||
| <Link |
There was a problem hiding this comment.
logic: Using next/link instead of locale-aware @/i18n/navigation Link
Should use Link from @/i18n/navigation to maintain locale context when navigating.
| pt: "Portugues", | ||
| de: "Deutsch", | ||
| zh: "中文", | ||
| es: "Espanol", | ||
| nl: "Nederlands", |
There was a problem hiding this comment.
syntax: Missing diacritics in language names
"Portugues" should be "Português", "Espanol" should be "Español"
lib/proxy-redirect.ts
Outdated
| /** | ||
| * Supported locales for i18n routing. | ||
| */ | ||
| export const LOCALES = ["en", "pt", "de", "zh", "es", "nl"]; |
There was a problem hiding this comment.
logic: Duplicate locale definition creates maintenance risk
LOCALES array here duplicates the locales defined in i18n/routing.ts. Should import from routing config to maintain single source of truth.
| export const LOCALES = ["en", "pt", "de", "zh", "es", "nl"]; | |
| import { routing } from "@/i18n/routing"; | |
| export const LOCALES = routing.locales; |
proxy.ts
Outdated
| // Locale prefixed paths | ||
| '/(pt|de|zh|es|nl)/:path*', |
There was a problem hiding this comment.
style: Hardcoded locale list in matcher duplicates routing config
Locale list '/(pt|de|zh|es|nl)/:path*' is hardcoded and excludes default locale 'en', duplicating config from i18n/routing.ts. If locales are added/removed in routing config, this must be manually updated.
- Use env var for Patron plan ID (dev/prod) - Show upgrade button for free users, manage for paid - Update publications list to lower legal risk - Fix header z-index for Clerk drawer overlay 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
lib/proxy-redirect.ts
Outdated
| /** | ||
| * Supported locales for i18n routing. | ||
| */ | ||
| export const LOCALES = ["en", "pt", "de", "zh", "es", "nl"]; |
There was a problem hiding this comment.
logic: Duplicate locale definition
LOCALES duplicates the config from i18n/routing.ts:line 4. Import routing.locales instead to maintain single source of truth.
| export const LOCALES = ["en", "pt", "de", "zh", "es", "nl"]; | |
| import { routing } from "@/i18n/routing"; | |
| /** | |
| * Supported locales for i18n routing. | |
| */ | |
| export const LOCALES = routing.locales; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| // Root path | ||
| '/', | ||
| // Locale prefixed paths | ||
| '/(pt|de|zh|es|nl)/:path*', |
There was a problem hiding this comment.
style: Hardcoded locale matcher excludes default locale
Pattern '/(pt|de|zh|es|nl)/:path*' hardcodes non-default locales and would need manual updates if locales change in i18n/routing.ts. The default locale 'en' is intentionally excluded due to localePrefix: 'as-needed' strategy, but this creates maintenance burden.
Consider dynamically generating this pattern or documenting the manual sync requirement clearly.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
app/[locale]/pricing/page.tsx
Outdated
| import { SignedIn, SignedOut, SignInButton, UserButton } from "@clerk/nextjs"; | ||
| import { CheckoutButton, useSubscription, SubscriptionDetailsButton } from "@clerk/nextjs/experimental"; | ||
| import { Check, X, ChevronDown, ArrowLeft } from "lucide-react"; | ||
| import Link from "next/link"; |
There was a problem hiding this comment.
syntax: Missing i18n imports
Pricing page is under locale route but doesn't import next-intl or locale-aware navigation.
| import Link from "next/link"; | |
| import Link from "next/link"; | |
| import { useTranslations } from "next-intl"; | |
| import { Link as I18nLink } from "@/i18n/navigation"; |
| <h1 className="text-4xl sm:text-5xl font-bold tracking-tight text-center"> | ||
| Read without limits. | ||
| </h1> |
There was a problem hiding this comment.
logic: Hardcoded English text in locale route
Text "Read without limits." is hardcoded despite page being under [locale] route. Should use translation key.
Example implementation:
| <h1 className="text-4xl sm:text-5xl font-bold tracking-tight text-center"> | |
| Read without limits. | |
| </h1> | |
| <h1 className="text-4xl sm:text-5xl font-bold tracking-tight text-center"> | |
| {t("heroTitle")} | |
| </h1> |
| <Link | ||
| href="/" | ||
| className="flex items-center gap-2 text-sm text-muted-foreground hover:text-foreground transition-colors" | ||
| > | ||
| <ArrowLeft className="size-4" /> | ||
| <span className="hidden sm:inline">Back to SMRY</span> | ||
| </Link> |
There was a problem hiding this comment.
logic: Using standard Link instead of locale-aware Link
Should use Link from @/i18n/navigation to maintain locale context when navigating to home.
|
|
||
| const languageNames: Record<Locale, string> = { | ||
| en: "English", | ||
| pt: "Portugues", |
There was a problem hiding this comment.
syntax: Missing diacritics
Should be "Português" not "Portugues"
| pt: "Portugues", | |
| pt: "Português", |
| pt: "Portugues", | ||
| de: "Deutsch", | ||
| zh: "中文", | ||
| es: "Espanol", |
There was a problem hiding this comment.
syntax: Missing diacritics
Should be "Español" not "Espanol"
| es: "Espanol", | |
| es: "Español", |
| @@ -37,17 +38,16 @@ export const metadata: Metadata = { | |||
| }, | |||
| }; | |||
There was a problem hiding this comment.
style: Metadata not localized for international users
Root layout metadata (title, description, OG tags) remains hardcoded in English. Consider localizing metadata per locale for better international SEO.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
messages/de.json (1)
1-195: German translations look good!The file now contains proper German umlauts (ä, ö, ü) and special characters (ß) throughout. The structure mirrors other locale files, and the translations appear complete and correctly formatted.
proxy.ts (1)
48-59: Documentation helps, but consider a build-time validation.The added comments (lines 44-46) clearly document the sync requirement with
i18n/routing.ts. Since Next.js middleware config requires static patterns, this is a reasonable approach.To further reduce drift risk, consider adding a build-time or CI check that verifies the matcher pattern matches
routing.locales.#!/bin/bash # Verify that the locale pattern in proxy.ts matches the routing config echo "Checking locale sync between proxy.ts and i18n/routing.ts..." # Extract locales from routing.ts routing_locales=$(grep -oP "locales:\s*\[\K[^\]]+(?=\])" i18n/routing.ts | tr -d "' " | tr ',' '\n' | sort) echo "Routing locales: $routing_locales" # Extract locales from proxy.ts matcher (excluding 'en' which is default) proxy_locales=$(grep -oP "\(pt\|[^)]+\)" proxy.ts | tr -d '()' | tr '|' '\n' | sort) echo "Proxy matcher locales: $proxy_locales" # Check if they match (excluding 'en' from routing) routing_without_en=$(echo "$routing_locales" | grep -v '^en$') if [ "$routing_without_en" = "$proxy_locales" ]; then echo "✓ Locales are in sync" else echo "✗ Locale mismatch detected!" echo "Expected (from routing.ts minus 'en'): $routing_without_en" echo "Found (in proxy.ts): $proxy_locales" exit 1 fimessages/pt.json (1)
1-195: Missing Portuguese diacritical marks throughout the file.As noted in the previous review, the Portuguese translations are still missing essential diacritical marks (accents, tildes, cedillas) required for correct spelling. Examples include:
- Line 3: "Gratis" → "Grátis"
- Line 11: "graca" → "graça"
- Line 16: "tambem" → "também"
- Line 27: "duvidas" → "dúvidas"
- Line 31: "nao" → "não"
- Line 38: "Quao" → "Quão"
These are not stylistic choices—Portuguese orthography requires these marks for correct spelling.
Based on the previous review, consider using a Portuguese spell-checker or native speaker review to ensure all diacritical marks are correctly applied.
🧹 Nitpick comments (4)
app/globals.css (1)
253-269: z-index rules are functional but line 265 is fragile.The high z-index with
!importantis necessary for portal overlay behavior. However, the selector on line 265 targeting inline styles (div[style*="position: fixed"]) is brittle and could break if Clerk changes their internal styling implementation.Consider adding a comment noting this dependency, or reach out to Clerk for recommended styling approaches.
components/features/summary-form.tsx (2)
21-40: Optimize date computation in useDailyUsage hook.The
todayconstant is recomputed on every render (line 31), which is unnecessary overhead. Consider memoizing the date string computation or restructuring the logic to derive it only when needed.🔎 Proposed refactor
function useDailyUsage() { const [usage, setUsage] = useLocalStorage<{ count: number; date: string }>( "summary-daily-usage", { count: 0, date: new Date().toDateString() } ); - // Reset if it's a new day - const today = new Date().toDateString(); - const currentCount = usage.date === today ? usage.count : 0; - const increment = useCallback(() => { + const today = new Date().toDateString(); const newCount = usage.date === today ? usage.count + 1 : 1; setUsage({ count: newCount, date: today }); - }, [usage, today, setUsage]); + }, [usage, setUsage]); + + // Reset if it's a new day + const today = new Date().toDateString(); + const currentCount = usage.date === today ? usage.count : 0; return { count: currentCount, increment, limit: DAILY_LIMIT }; }
276-291: Update comment to match logic.The comment on line 276 says "Soft upgrade prompt at 50%+ usage," but the condition
usageCount >= 10(line 70) is actually 50% of the limit (10/20). While technically correct, the comment would be clearer if it stated "at 10+ summaries" or explicitly mentioned "50% of daily limit."🔎 Proposed fix
- {/* Soft upgrade prompt at 50%+ usage */} + {/* Soft upgrade prompt at 50% of daily limit (10+ summaries) */} {showSoftUpgrade && (i18n/request.ts (1)
7-7: Type assertion can be made type-safe.The
as Localeassertion is better thanas any, but still bypasses TypeScript's type checking. SinceLocaleis defined as(typeof routing.locales)[number]in routing.ts, you can make this check fully type-safe.🔎 Proposed refactor
- if (!locale || !routing.locales.includes(locale as Locale)) { + if (!locale || !(routing.locales as readonly string[]).includes(locale)) { locale = routing.defaultLocale; }Or narrow the type explicitly:
+ const isValidLocale = (loc: string | undefined): loc is Locale => + !!loc && (routing.locales as readonly string[]).includes(loc); + - if (!locale || !routing.locales.includes(locale as Locale)) { + if (!isValidLocale(locale)) { locale = routing.defaultLocale; }
📜 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 (21)
.env.development(1 hunks)app/[locale]/layout.tsx(1 hunks)app/[locale]/page.tsx(9 hunks)app/[locale]/pricing/page.tsx(1 hunks)app/globals.css(1 hunks)components/features/proxy-content.tsx(2 hunks)components/features/summary-form.tsx(7 hunks)components/marketing/ad-spot.tsx(1 hunks)components/marketing/custom-pricing-table.tsx(1 hunks)components/shared/language-switcher.tsx(1 hunks)i18n/request.ts(1 hunks)lib/proxy-redirect.ts(2 hunks)messages/de.json(1 hunks)messages/en.json(1 hunks)messages/es.json(1 hunks)messages/nl.json(1 hunks)messages/pt.json(1 hunks)messages/zh.json(1 hunks)next.config.js(2 hunks)package.json(2 hunks)proxy.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- components/marketing/custom-pricing-table.tsx
- package.json
- app/[locale]/layout.tsx
- next.config.js
- messages/nl.json
- messages/zh.json
- messages/es.json
- app/[locale]/pricing/page.tsx
🧰 Additional context used
🧬 Code graph analysis (5)
components/shared/language-switcher.tsx (2)
i18n/routing.ts (2)
Locale(9-9)routing(3-7)components/ui/select.tsx (4)
Select(170-170)SelectTrigger(171-171)SelectValue(172-172)SelectItem(175-175)
lib/proxy-redirect.ts (1)
i18n/routing.ts (1)
routing(3-7)
i18n/request.ts (1)
i18n/routing.ts (2)
routing(3-7)Locale(9-9)
components/features/summary-form.tsx (1)
lib/hooks/use-is-premium.ts (1)
useIsPremium(19-34)
proxy.ts (2)
i18n/routing.ts (1)
routing(3-7)lib/proxy-redirect.ts (1)
buildProxyRedirectUrl(88-150)
🪛 dotenv-linter (4.0.0)
.env.development
[warning] 2-2: [UnorderedKey] The NEXT_PUBLIC_CLERK_PATRON_PLAN_ID key should go before the NEXT_PUBLIC_URL key
(UnorderedKey)
⏰ 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: Greptile Review
🔇 Additional comments (17)
.env.development (1)
1-2: LGTM!The new Clerk plan ID environment variable is correctly prefixed with
NEXT_PUBLIC_for client-side access. The alphabetical ordering suggestion from the linter is a minor style preference and can be safely ignored for a 2-line file.lib/proxy-redirect.ts (3)
2-7: LGTM - Single source of truth for locales!Good fix importing
LOCALESfrom the routing config rather than duplicating the locale list. This ensures consistency across the codebase.
34-53: Locale routing logic looks correct.The implementation properly handles:
- Root path
/(line 36)- Bare locale paths like
/pt,/de(line 45)- Locale-prefixed app routes like
/pt/pricing(lines 47-49)- Non-locale paths delegated to helper (line 52)
55-71: Helper function is well-structured.Clean separation of concerns by extracting the non-locale route checking into a dedicated helper.
proxy.ts (2)
5-8: i18n middleware integration looks correct.The
intlMiddlewareis properly created from the routing config, ensuring consistent locale handling.
10-27: Middleware flow is well-structured.The order of operations is correct:
- Skip i18n for API/TRPC routes (line 14)
- Check for proxy redirect (line 19)
- Apply i18n middleware for remaining routes (line 26)
app/globals.css (2)
223-252: Clerk button styling is well-structured.Good use of CSS custom properties for theming consistency. The styling cleanly targets Clerk's rendered buttons within the wrapper classes.
271-364: Tweet card styling is comprehensive and well-organized.Good approach using CSS custom properties for theme integration. The element hiding selectors are specific to the react-tweet library structure.
The
:has()selectors (lines 298, 313, 335) are supported in all major browsers (Chrome, Firefox, Safari, Edge), with full support in Chrome 106+, Firefox 122+, Safari 15.5+, and Edge 105+. These selectors are appropriate for this progressive enhancement use case.components/features/summary-form.tsx (2)
157-163: LGTM!The usage tracking logic correctly increments the count only when a new summary completes for non-premium users. The use of
prevCompletionRefeffectively prevents duplicate counts during streaming.
256-274: LGTM!The usage counter UI is well-implemented with appropriate color-coding to signal usage levels. The visibility logic correctly hides it for premium users.
i18n/request.ts (1)
11-17: LGTM!The error handling with try-catch and fallback to default locale messages is a solid improvement. This prevents the application from crashing when a translation file is missing.
components/marketing/ad-spot.tsx (1)
371-391: LGTM!The branding update from "Support" to "Go Pro" with gradient styling is consistent with the broader UI updates across the PR. The implementation is clean and maintains the existing click tracking logic.
components/features/proxy-content.tsx (1)
298-303: LGTM!The branding update to "Go Pro" with consistent gradient styling across both desktop and mobile views maintains visual consistency with the broader UI changes in this PR.
Also applies to: 413-418
components/shared/language-switcher.tsx (1)
15-22: LGTM!The language names are correctly spelled with proper diacritical marks (Português, Español). The implementation correctly uses next-intl's locale handling and provides a clean user experience with both desktop (name) and mobile (flag) displays.
messages/en.json (1)
1-195: LGTM!The English translation file is well-structured and comprehensive, providing a solid baseline for the internationalization system. The content is clear, consistent, and covers all UI areas of the application.
app/[locale]/page.tsx (2)
37-55: LGTM!The refactored
SupportLinkcomponent correctly shows the "Go Pro" link only for non-premium signed-in users. The loading check prevents UI flash, and the premium check ensures supporters don't see redundant CTAs.
60-61: LGTM!The internationalization integration is clean and comprehensive. All user-facing strings are properly wrapped with translation hooks (
t()andtCommon()), and the integration with next-intl follows best practices.Also applies to: 75-76, 129-129, 136-136, 141-141, 156-156, 201-201, 224-224, 228-228, 233-233
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
proxy.ts (1)
48-51: Documentation addresses previous feedback on hardcoded locales.The comments now clearly document the manual sync requirement with
i18n/routing.ts. This was previously flagged in reviews and the documentation is a reasonable solution given Next.js matcher patterns require static strings.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
i18n/routing.ts(1 hunks)proxy.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- i18n/routing.ts
🧰 Additional context used
🧬 Code graph analysis (1)
proxy.ts (2)
i18n/routing.ts (1)
routing(6-10)lib/proxy-redirect.ts (1)
buildProxyRedirectUrl(88-150)
⏰ 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: Greptile Review
🔇 Additional comments (3)
proxy.ts (3)
5-8: LGTM!Clean integration of the next-intl middleware. Creating the middleware at module level is appropriate for Next.js edge middleware.
25-26: LGTM!Correctly applies i18n middleware as the fallback when no proxy redirect is needed.
53-55: Matcher config is correct, but depends on the middleware fix above.The
apiexclusion in line 53 combined with the explicit API route inclusion in line 55 is intentional for Clerk auth. However, this relies on the middleware correctly distinguishing actual API routes from domain slugs likeapi.com—ensure the fix for lines 14-16 is applied.
| // Skip i18n for API routes - just let them through | ||
| if (pathname.startsWith('/api') || pathname.startsWith('/trpc')) { | ||
| return NextResponse.next(); | ||
| } |
There was a problem hiding this comment.
API route check is too broad—breaks proxying for domains like api.com.
The pathname.startsWith('/api') check incorrectly matches URL slugs like /api.com, /api.github.com, or /api-sports.io. Combined with the matcher on line 55, these domains would bypass buildProxyRedirectUrl and never get proxied.
🔎 Proposed fix
// Skip i18n for API routes - just let them through
- if (pathname.startsWith('/api') || pathname.startsWith('/trpc')) {
+ if (pathname === '/api' || pathname.startsWith('/api/') ||
+ pathname === '/trpc' || pathname.startsWith('/trpc/')) {
return NextResponse.next();
}📝 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.
| // Skip i18n for API routes - just let them through | |
| if (pathname.startsWith('/api') || pathname.startsWith('/trpc')) { | |
| return NextResponse.next(); | |
| } | |
| // Skip i18n for API routes - just let them through | |
| if (pathname === '/api' || pathname.startsWith('/api/') || | |
| pathname === '/trpc' || pathname.startsWith('/trpc/')) { | |
| return NextResponse.next(); | |
| } |
🤖 Prompt for AI Agents
In proxy.ts around lines 13 to 16, the current check uses
pathname.startsWith('/api') which erroneously matches paths like /api.com and
prevents them from being proxied; change the condition to only match the exact
segment or a segment prefix boundary — e.g., replace pathname.startsWith('/api')
with (pathname === '/api' || pathname.startsWith('/api/')) and do the same for
'/trpc' (pathname === '/trpc' || pathname.startsWith('/trpc/')); this ensures
only true API route segments are skipped and preserves proxying for domains like
api.com referenced by the matcher on line 55.
1 participant
next-intlfor internationalizationnext-intlplugingetLocaleusage inRootLayoutto set<html lang>dynamicallyLOCALESlist and locale-aware app route detection inproxy-redirectnext-intl/middlewareinproxy.tswith updated matcher rulesSiteFooternow usesnext-intltranslations for copy and logo alt textBanner,BookmarkletLink,FAQ, andFoundersLetterconverted to client components with translation hooksapp/page.tsx,app/pricing/page.tsx, andapp/history/page.tsxapp/proxy/page.tsx,layout.tsx,loading.tsx,error.tsx) in favor of new architecturenext-intl@^4.6.1to project dependenciesNotes
Summary by CodeRabbit
New Features
Updated
Removed
✏️ Tip: You can customize this high-level summary in your review settings.