build: update i18n version and routing #568
Conversation
|
@Technologic101 is attempting to deploy a commit to the al1abb-team Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe pull request introduces comprehensive changes to the project's internationalization (i18n) implementation using the Changes
Sequence DiagramsequenceDiagram
participant Client
participant Routing
participant Middleware
participant Layout
participant Locales
Client->>Routing: Request page
Routing->>Middleware: Validate locale
Middleware->>Routing: Return valid locale
Routing->>Locales: Fetch messages
Locales-->>Layout: Provide messages
Layout->>Client: Render internationalized page
Poem
Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
i18n/request.ts (2)
9-11: Consider improving type safetyThe type assertion
as anycould be replaced with a more specific type to maintain better type safety.- if (!locale || !routing.locales.includes(locale as any)) { + if (!locale || !routing.locales.includes(locale as string)) {
13-16: Consider implementing message cachingThe dynamic import of locale messages on each request could impact performance. Consider implementing a caching mechanism for frequently accessed locales.
i18n/routing.ts (1)
8-8: Consider explicit locale mappingWhile
locale.codeworks, consider making the locale mapping more explicit for better maintainability.- locales: LOCALES.map((locale) => locale.code), + locales: LOCALES.map(({code}) => code),app/[locale]/layout.tsx (1)
88-88: Consider adding timeZone prop for consistent date handlingWhen using next-intl v3, it's recommended to specify the timeZone prop for consistent date handling across different locales.
- <NextIntlClientProvider messages={messages}> + <NextIntlClientProvider + messages={messages} + timeZone="UTC" + >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
app/[locale]/layout.tsx(3 hunks)app/components/reusables/LanguageSelector.tsx(1 hunks)i18n/request.ts(1 hunks)i18n/routing.ts(1 hunks)middleware.ts(1 hunks)next.config.js(2 hunks)package.json(3 hunks)
🔇 Additional comments (7)
middleware.ts (1)
1-4: Well-structured middleware implementation!The changes effectively centralize the routing configuration by importing from
@/i18n/routing, following next-intl v3 best practices. This approach improves maintainability by keeping locale configuration in a single location.i18n/routing.ts (1)
16-17: Well-structured navigation exports!The exported navigation utilities provide a clean API while maintaining internationalization support.
next.config.js (2)
24-24: Verify plugin composition orderThe current plugin composition looks correct. However, ensure that
withNextIntldoesn't interfere withwithBundleAnalyzer's functionality. Consider monitoring bundle size reports after this change.
Line range hint
7-9: Verify puppeteer-core compatibilitySince the PR mentions updating Puppeteer version, ensure that the
serverComponentsExternalPackagesconfiguration is still required and compatible with the new version.✅ Verification successful
Configuration is correct and necessary
The
serverComponentsExternalPackagesconfiguration for puppeteer-core is still required as it's actively used in server-side code (services/invoice/server/generatePdfService.ts) for PDF generation in production. The configuration is compatible with the updated version 22.5.0.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check package.json for puppeteer version rg -A 1 '"puppeteer(-core)?":\s*"[^"]*"' package.json # Check if other packages require similar external configuration rg -l 'require\(["\']puppeteer(-core)?["\']\)' .Length of output: 390
Script:
#!/bin/bash # Search for puppeteer-core imports/requires rg -l "from ['\"]puppeteer-core['\"]|require\\(['\"]puppeteer-core['\"]\\)" . # Search for files with .server suffix or "use server" directive that use puppeteer rg -l "puppeteer(-core)?" --glob "**/*.server.{js,ts,jsx,tsx}" . rg -l "^['\"](server|use server)['\"]\s*;.*puppeteer(-core)?" . # Check if there are any server components using puppeteer rg -C 2 "puppeteer(-core)?" --glob "**/*.server.{js,ts,jsx,tsx}" .Length of output: 291
Script:
#!/bin/bash # Check for any puppeteer usage in app directory rg -l "puppeteer(-core)?" app/ # Look for API routes using puppeteer rg -l "puppeteer(-core)?" --glob "app/api/**/*.{js,ts}" . rg -l "puppeteer(-core)?" --glob "pages/api/**/*.{js,ts}" . # Search for any puppeteer usage patterns rg -C 2 "puppeteer(-core)?" .Length of output: 12294
app/components/reusables/LanguageSelector.tsx (1)
6-6: Verify the routing configuration setupThe import path has been updated to use a custom routing configuration. Ensure that
@/i18n/routingis properly set up with the new next-intl v3 routing configuration.Consider adding documentation about the new routing setup in the README or a dedicated i18n documentation file.
✅ Verification successful
Import change is correctly implemented ✅
The custom routing setup at
i18n/routing.tsis properly configured using next-intl v3'sdefineRoutingandcreateNavigation. The import change in LanguageSelector.tsx is valid and consistent with the application's i18n architecture.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the routing configuration exists and is properly set up rg -l "defineRouting|createSharedPathnamesNavigation" "app/i18n"Length of output: 113
Script:
#!/bin/bash # Find i18n related files and directories fd -t f "i18n" --exec rg -l "defineRouting|createSharedPathnamesNavigation|useRouter" {}Length of output: 90
Script:
#!/bin/bash # Find all i18n related files echo "=== i18n related files ===" fd "i18n" echo -e "\n=== Files containing routing setup ===" rg -l "routing|navigation" --type ts --type tsxLength of output: 249
Script:
#!/bin/bash # List contents of i18n directory echo "=== Contents of i18n directory ===" ls -la i18n/ echo -e "\n=== Files containing routing setup ===" rg -t ts "routing|navigation" --glob "*.{ts,tsx}"Length of output: 2442
package.json (2)
66-66: Verify Puppeteer and Chromium version compatibilityThe PR updates both Puppeteer and adds @sparticuz/chromium. Ensure these versions are compatible and the removal of the postinstall script doesn't affect the setup.
Consider documenting the Puppeteer setup and any environment-specific requirements in the README.
Also applies to: 75-75
41-41: Verify next-intl v3 migration changesThe update from next-intl v2 to v3 is a major version bump that includes breaking changes. Ensure all breaking changes have been addressed:
- Routing configuration updates
- Message loading changes
- Client/Server components separation
✅ Verification successful
next-intl v3 migration appears correctly implemented ✅
The codebase already follows the v3 patterns:
- Correct usage of NextIntlClientProvider
- Server-side message loading with getMessages
- Proper app directory [locale] routing structure
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for potential missed migration updates rg "useTranslations|useLocale|useNow|useTimeZone" "app/"Length of output: 56
Script:
#!/bin/bash # Find next-intl usage and config files rg "from 'next-intl'" -A 2 rg "from \"next-intl\"" -A 2 # Look for i18n configuration files fd -g "*i18n*.{js,ts,tsx,json}" # Look for messages or translations fd -g "*messages*.{js,ts,tsx,json}" fd -g "*translations*.{js,ts,tsx,json}"Length of output: 531
|
|
||
| const messages = await getMessages() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for message loading
The error handling for message loading has been removed. Consider adding error handling to gracefully handle cases where messages fail to load.
- const messages = await getMessages()
+ try {
+ const messages = await getMessages()
+ } catch (error) {
+ console.error('Failed to load messages:', error);
+ notFound();
+ }Committable suggestion skipped: line range outside the PR's diff.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Refactors routing to use i18n v3, bumps puppeteer version
Summary by CodeRabbit
Release Notes
New Features
Dependency Updates
next-intlto version 3.26.3puppeteerto version 22.5.0@sparticuz/chromiumdependencyConfiguration Changes
Chores
postinstallscript from project configuration