fix: refactor i18n loadTranslations and set timeout to 3s
#22878
Conversation
WalkthroughThis change updates the mock implementation of the Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
loadTranslations and set timeout to 3s
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/04/25)1 reviewer was added to this PR based on Keith Williams's automation. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/lib/fetchWithTimeout.ts (1)
1-13: LGTM! Clean timeout implementation with minor enhancement opportunities.The
fetchWithTimeoututility correctly implements timeout functionality usingAbortController. Thefinallyblock ensures timeout cleanup.Consider these enhancements:
-export async function fetchWithTimeout(url: string, options: RequestInit = {}, timeoutMs: number) { +export async function fetchWithTimeout(url: string, options: RequestInit = {}, timeoutMs: number = 5000) { + if (timeoutMs <= 0) { + throw new Error('Timeout must be positive'); + } + const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), timeoutMs); try { const response = await fetch(url, { ...options, signal: controller.signal, }); return response; + } catch (error) { + if (error instanceof Error && error.name === 'AbortError') { + throw new Error(`Request timed out after ${timeoutMs}ms`); + } + throw error; } finally { clearTimeout(timeout); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/web/test/lib/handleChildrenEventTypes.test.ts(1 hunks)packages/features/ee/billing/credit-service.test.ts(1 hunks)packages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.test.ts(1 hunks)packages/features/ee/workflows/lib/reminders/reminderScheduler.test.ts(1 hunks)packages/features/ee/workflows/lib/test/compareReminderBodyToTemplate.test.ts(2 hunks)packages/lib/fetchWithTimeout.ts(1 hunks)packages/lib/server/i18n.ts(1 hunks)packages/lib/server/repository/user.test.ts(1 hunks)packages/lib/server/service/userCreationService.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/ee/workflows/lib/reminders/reminderScheduler.test.tspackages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.test.tspackages/features/ee/billing/credit-service.test.tspackages/features/ee/workflows/lib/test/compareReminderBodyToTemplate.test.tspackages/lib/server/repository/user.test.tspackages/lib/fetchWithTimeout.tspackages/lib/server/i18n.tsapps/web/test/lib/handleChildrenEventTypes.test.tspackages/lib/server/service/userCreationService.test.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/ee/workflows/lib/reminders/reminderScheduler.test.tspackages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.test.tspackages/features/ee/billing/credit-service.test.tspackages/features/ee/workflows/lib/test/compareReminderBodyToTemplate.test.tspackages/lib/server/repository/user.test.tspackages/lib/fetchWithTimeout.tspackages/lib/server/i18n.tsapps/web/test/lib/handleChildrenEventTypes.test.tspackages/lib/server/service/userCreationService.test.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.tsx : Always use `t()` for text localization in frontend code; direct text embedding should trigger a warning
Learnt from: bandhan-majumder
PR: calcom/cal.com#22359
File: packages/lib/server/locales/en/common.json:1336-1339
Timestamp: 2025-07-14T16:31:45.233Z
Learning: When making localization changes for new features, it's often safer to add new strings rather than modify existing ones to avoid breaking existing functionality that depends on the original strings. This approach allows for feature-specific customization while maintaining backward compatibility.
📚 Learning: applies to **/*.tsx : always use `t()` for text localization in frontend code; direct text embedding...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.tsx : Always use `t()` for text localization in frontend code; direct text embedding should trigger a warning
Applied to files:
packages/features/ee/workflows/lib/reminders/reminderScheduler.test.tspackages/features/ee/workflows/lib/test/compareReminderBodyToTemplate.test.tspackages/lib/server/repository/user.test.tspackages/lib/server/i18n.tsapps/web/test/lib/handleChildrenEventTypes.test.tspackages/lib/server/service/userCreationService.test.ts
📚 Learning: when making localization changes for new features, it's often safer to add new strings rather than m...
Learnt from: bandhan-majumder
PR: calcom/cal.com#22359
File: packages/lib/server/locales/en/common.json:1336-1339
Timestamp: 2025-07-14T16:31:45.233Z
Learning: When making localization changes for new features, it's often safer to add new strings rather than modify existing ones to avoid breaking existing functionality that depends on the original strings. This approach allows for feature-specific customization while maintaining backward compatibility.
Applied to files:
packages/lib/server/i18n.ts
🧬 Code Graph Analysis (1)
packages/lib/server/i18n.ts (3)
packages/config/next-i18next.config.d.ts (1)
i18n(2-5)packages/lib/constants.ts (1)
WEBAPP_URL(12-18)packages/lib/fetchWithTimeout.ts (1)
fetchWithTimeout(1-13)
⏰ 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: Detect changes
🔇 Additional comments (12)
packages/features/ee/billing/credit-service.test.ts (1)
28-37: LGTM! Consistent i18n mock implementation.The async
getTranslationmock properly simulates the real i18n behavior by returning a translation function with attachedlocaleandnamespaceproperties. This standardizes testing across the codebase.packages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.test.ts (1)
46-55: LGTM! Consistent i18n mock pattern.The i18n mock follows the same async pattern established across other test files, ensuring consistent behavior when testing organization creation workflows that depend on translations.
packages/features/ee/workflows/lib/reminders/reminderScheduler.test.ts (1)
20-29: LGTM! Standardized i18n mock for workflow testing.The async
getTranslationmock maintains consistency with the established pattern, enabling proper testing of reminder scheduling workflows without translation dependency.packages/features/ee/workflows/lib/test/compareReminderBodyToTemplate.test.ts (2)
2-2: LGTM! Added vi import for mocking.The import update correctly adds
vito support the new i18n mock implementation.
13-22: LGTM! Consistent async i18n mock for template testing.The i18n mock maintains the established async pattern and supports the actual
getTranslationcalls used in the template comparison tests (lines 89, 106, 122, 136, 152).packages/lib/server/repository/user.test.ts (1)
12-16: LGTM! Mock implementation correctly simulates async behavior.The updated mock properly reflects the new asynchronous signature of
getTranslationwithlocaleandnamespaceparameters, while maintaining the test behavior of returning keys unchanged. The addition oflocaleandnamespaceproperties on the returned function aligns with the production implementation structure.apps/web/test/lib/handleChildrenEventTypes.test.ts (1)
29-34: LGTM! Consistent mock implementation across test files.The mock update maintains consistency with other test files in the codebase, properly simulating the new async
getTranslationfunction signature while preserving test functionality.packages/lib/server/service/userCreationService.test.ts (1)
14-18: LGTM! Consistent mock implementation maintained.The mock implementation follows the same pattern as other test files in this refactor, ensuring consistency across the test suite.
packages/lib/server/i18n.ts (4)
3-3: LGTM! Improved imports and constants.The new imports enhance the implementation:
i18nconfig enables proper locale validationfetchWithTimeoutadds timeout capability for reliabilityloggerprovides better error handling than console methodsSUPPORTED_NAMESPACESconstant improves maintainabilityAlso applies to: 6-7, 11-11
19-22: LGTM! Enhanced input validation.The locale and namespace validation logic is well-implemented:
- Handles zh → zh-CN mapping consistently
- Validates against supported locales with fallback to "en"
- Validates namespaces with fallback to "common"
29-36: LGTM! Improved network reliability with timeout.The use of
fetchWithTimeoutwith a 3-second timeout enhances reliability by preventing hanging requests. The cache policy appropriately differs between production and development environments.
38-46: No runtime errors from empty fallback translations
After inspectingloadTranslationsandgetTranslation, returning{}on fetch failure simply initializes i18next with an empty resource object. ThegetFixedTfunction still returns a valid translator and will fall back to returning the key (rather than throwing). There are no code paths that directly index into the raw translations object, so this change won’t cause runtime exceptions—at worst, untranslated keys will surface in the UI.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
| /* eslint-disable @typescript-eslint/no-var-requires */ | ||
| const { i18n } = require("@calcom/config/next-i18next.config"); |
There was a problem hiding this comment.
E2E results are ready! |
What does this PR do?
commonnamespaceMandatory Tasks (DO NOT REMOVE)
How should this be tested?