feat(i18n): add Persian (fa) locale with Jalali-aware date formatting#29495
feat(i18n): add Persian (fa) locale with Jalali-aware date formatting#29495Har2yQn78 wants to merge 2 commits into
Conversation
Introduce optional Persian/Shamsi support as a display-only foundation: - Register `fa` as a locale target so it appears in the language picker (RTL direction and language label are derived automatically). - Add a partial `fa/common.json`; missing keys fall back to English. - Route Persian locales through `Intl` (Jalali-aware) in dateTimeFormatter so date/time/weekday output can never silently render as Gregorian via the dayjs fallback. Non-Persian locales are unchanged. - Add `getWeekStartForLocale` defaulting Persian to Saturday. Booking math, timezone logic, storage, and API payloads are untouched. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Welcome to Cal.diy, @Har2yQn78! Thanks for opening this pull request. A few things to keep in mind:
A maintainer will review your PR soon. Thanks for contributing! |
|
Ready to act? Review this PR in Change Stack to turn feedback into patch suggestions you can inspect and refine. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR adds complete Persian (Jalali) calendar locale support to the application. It registers the "fa" locale in i18n configuration, adds Persian translations for common UI labels and date/time terminology, and implements Persian-calendar-aware datetime formatting. The core change routes Persian locales through the 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/lib/weekday.test.ts (1)
75-87: ⚡ Quick winConsider adding edge case test coverage.
The current tests cover the main scenarios well. Adding tests for edge cases would strengthen the suite and help catch potential issues:
- Empty array input:
getWeekStartForLocale([])- Empty string input:
getWeekStartForLocale("")- Multi-locale array with non-Persian first:
getWeekStartForLocale(["en", "fa"])These cases would validate the normalization logic and ensure consistent fallback behavior.
📋 Suggested additional test cases
it("defaults to Sunday (0) for non-Persian locales", () => { expect(getWeekStartForLocale("en")).toBe(0); expect(getWeekStartForLocale("ar")).toBe(0); expect(getWeekStartForLocale(undefined)).toBe(0); + expect(getWeekStartForLocale([])).toBe(0); + expect(getWeekStartForLocale("")).toBe(0); + expect(getWeekStartForLocale(["en", "fa"])).toBe(0); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/lib/weekday.test.ts` around lines 75 - 87, Add edge-case tests for getWeekStartForLocale to ensure normalization and fallback behavior: add assertions for an empty array input (getWeekStartForLocale([]) -> expect 0), an empty string input (getWeekStartForLocale("") -> expect 0), and a multi-locale array where the first entry is non-Persian but a later entry is Persian (getWeekStartForLocale(["en","fa"]) -> expect 6); place them alongside the existing describe("fn: getWeekStartForLocale") tests so they validate handling of falsy/empty inputs and array precedence in the locale normalization logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/lib/dateTimeFormatter.ts`:
- Around line 40-46: resolveFormatterLocale currently appends the string
"-u-ca-persian" and can produce invalid BCP-47 tags (e.g.,
fa-u-nu-latn-u-ca-persian); change the implementation to stop mutating the
locale string and instead ensure callers pass { calendar: "persian" } to
Intl.DateTimeFormat when isPersianCalendarLocale(locale) returns true, so update
resolveFormatterLocale (and any callers) to return the original locale unchanged
and add logic where DateTimeFormat is constructed to include calendar: "persian"
for Persian locales (keep references to resolveFormatterLocale and
isPersianCalendarLocale to locate changes).
In `@packages/lib/weekday.ts`:
- Around line 12-15: getWeekStartForLocale treats an empty array differently
than undefined because Array.isArray(locale) ? locale[0] : locale || "en" yields
undefined for [], leading to inconsistent behavior; update the normalization in
getWeekStartForLocale to defensively handle empty arrays by using the first
element only if present (e.g., fallback to "en" when locale is [] or locale[0]
is undefined) before calling isPersianCalendarLocale, so the function
consistently returns SATURDAY or SUNDAY for [], undefined, or string inputs.
---
Nitpick comments:
In `@packages/lib/weekday.test.ts`:
- Around line 75-87: Add edge-case tests for getWeekStartForLocale to ensure
normalization and fallback behavior: add assertions for an empty array input
(getWeekStartForLocale([]) -> expect 0), an empty string input
(getWeekStartForLocale("") -> expect 0), and a multi-locale array where the
first entry is non-Persian but a later entry is Persian
(getWeekStartForLocale(["en","fa"]) -> expect 6); place them alongside the
existing describe("fn: getWeekStartForLocale") tests so they validate handling
of falsy/empty inputs and array precedence in the locale normalization logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 21210bc0-56ec-415e-91bf-b4a92f389427
📒 Files selected for processing (6)
i18n.jsonpackages/i18n/locales/fa/common.jsonpackages/lib/dateTimeFormatter.test.tspackages/lib/dateTimeFormatter.tspackages/lib/weekday.test.tspackages/lib/weekday.ts
| /** Makes the Persian calendar explicit so behavior is stable across ICU versions. */ | ||
| function resolveFormatterLocale(locale: string): string { | ||
| if (isPersianCalendarLocale(locale) && !locale.toLowerCase().includes("-ca-")) { | ||
| return `${locale}-u-ca-persian`; | ||
| } | ||
| return locale; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Intl.DateTimeFormat support the calendar option (e.g. calendar: "persian") and in which engine/Node versions?
💡 Result:
Yes, the Intl.DateTimeFormat constructor supports the calendar option [1][2][3]. You can specify a calendar, such as "persian", "chinese", or "gregory", in the options object [1][2]. This option can also be set via the Unicode extension key "ca" in the locale string (e.g., "en-u-ca-persian") [1][3]. When both are provided, the options property takes precedence [1][3]. Regarding support: - Engines: The calendar option is part of the standard ECMAScript Internationalization API (ECMA-402) and is supported in all modern web browsers and server-side JavaScript environments that include ICU (International Components for Unicode) data [1][2][4][5]. - Node.js Versions: Support for this feature in Node.js depends on the version of the ICU library bundled with the Node.js release [4][5]. Since Node.js 13+, the default builds include full ICU data, which supports these calendar types [4][5]. If you are using a custom build or an older version of Node.js, you may need to ensure full-icu or system-icu data is available to support non-Gregorian calendars; otherwise, it may fall back to default behavior [4][5]. Important Note: When using the calendar option, keep in mind that the calendar must be supported for the specified locale. If you pass an invalid or unsupported calendar, the environment typically ignores it or reverts to the locale's default calendar [6][7][8]. You can verify which calendars are supported for a given environment using Intl.supportedValuesOf('calendar') [1][3]. Additionally, third-party polyfills, such as those found in some older versions of FormatJS, might not implement full non-Gregorian calendar support [9].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/DateTimeFormat/DateTimeFormat
- 2: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/DateTimeFormat
- 3: https://github.com/mdn/content/blob/main/files/en-us/web/javascript/reference/global_objects/intl/datetimeformat/datetimeformat/index.md
- 4: https://nodejs.org/docs/latest-v23.x/api/intl.html
- 5: http://nodejs.org/docs/latest/api/intl.html
- 6: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/DateTimeFormat/resolvedOptions
- 7: https://stackoverflow.com/questions/60734281/intl-datetimeformat-calendar-and-numberingsystem-options-never-work
- 8: https://github.com/mdn/content/blob/main/files/en-us/web/javascript/reference/global_objects/intl/datetimeformat/resolvedoptions/index.md?plain=1
- 9: https://formatjs.github.io/docs/polyfills/intl-datetimeformat/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and show the relevant portions of packages/lib/dateTimeFormatter.ts
FILE="packages/lib/dateTimeFormatter.ts"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE"
exit 1
fi
# Show around the referenced lines (40-46, plus a bit more for context)
nl -ba "$FILE" | sed -n '1,120p'Repository: calcom/cal.diy
Length of output: 102
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Additionally, quickly search for resolveFormatterLocale / isPersianCalendarLocale usage
rg -n "resolveFormatterLocale|isPersianCalendarLocale|DateTimeFormatter|calendar" packages/lib/dateTimeFormatter.ts packages/lib -S || trueRepository: calcom/cal.diy
Length of output: 48398
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/lib/dateTimeFormatter.ts"
echo "== dateTimeFormatter.ts (relevant section) =="
sed -n '1,220p' "$FILE" | sed -n '20,120p'
echo
echo "== dateTimeFormatter.ts (rest around resolve/build options) =="
sed -n '100,210p' "$FILE"
TEST="packages/lib/dateTimeFormatter.test.ts"
echo
echo "== dateTimeFormatter.test.ts (Persian section) =="
# show around the Persian describe block and any tests for -u- / locale strings
sed -n '250,360p' "$TEST"Repository: calcom/cal.diy
Length of output: 9464
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'NODE'
function tryLocale(locale){
try{
const fmt = new Intl.DateTimeFormat(locale, { timeZone: "UTC" });
const ro = fmt.resolvedOptions();
// Some engines may not expose calendar; keep it best-effort
console.log(JSON.stringify({ locale, ok: true, calendar: ro.calendar, numberingSystem: ro.numberingSystem }));
}catch(e){
console.log(JSON.stringify({ locale, ok: false, name: e && e.name, message: e && e.message }));
}
}
console.log("=== duplicate -u- locale behavior ===");
tryLocale("fa");
tryLocale("fa-IR");
tryLocale("fa-u-nu-latn");
tryLocale("fa-u-nu-latn-u-ca-persian"); // what resolveFormatterLocale currently emits
tryLocale("en-US-u-ca-persian");
console.log("\n=== options.calendar precedence ===");
try{
const fmt = new Intl.DateTimeFormat("fa-u-nu-latn", { timeZone:"UTC", calendar:"persian" });
console.log(JSON.stringify({ localeBase:"fa-u-nu-latn", ok:true, calendar: fmt.resolvedOptions().calendar, numberingSystem: fmt.resolvedOptions().numberingSystem }));
}catch(e){
console.log(JSON.stringify({ ok:false, name: e && e.name, message: e && e.message }));
}
NODERepository: calcom/cal.diy
Length of output: 672
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'NODE'
function tryLocale(locale){
try{
const fmt = new Intl.DateTimeFormat(locale, { timeZone: "UTC" });
const ro = fmt.resolvedOptions();
console.log(JSON.stringify({ locale, ok:true, calendar: ro.calendar, numberingSystem: ro.numberingSystem }));
}catch(e){
console.log(JSON.stringify({ locale, ok:false, name: e && e.name, message: e && e.message }));
}
}
tryLocale("fa-u-nu-latn-ca-persian");
tryLocale("fa-u-nu-latn-u-ca-persian");
NODERepository: calcom/cal.diy
Length of output: 281
Fix resolveFormatterLocale to avoid generating invalid BCP-47 tags (duplicate -u-)
resolveFormatterLocale appends -u-ca-persian whenever the locale is Persian per isPersianCalendarLocale and doesn’t already contain -ca-. For inputs like fa-u-nu-latn, it produces fa-u-nu-latn-u-ca-persian, which throws RangeError: Invalid language tag and breaks Intl-based formatting for that locale.
A more robust approach is to set the calendar via Intl.DateTimeFormat options (calendar: "persian") instead of mutating the locale string.
🛡️ Minimal hardening of the string approach
function resolveFormatterLocale(locale: string): string {
- if (isPersianCalendarLocale(locale) && !locale.toLowerCase().includes("-ca-")) {
- return `${locale}-u-ca-persian`;
- }
- return locale;
+ const lower = locale.toLowerCase();
+ if (isPersianCalendarLocale(locale) && !lower.includes("-ca-")) {
+ return lower.includes("-u-") ? `${locale}-ca-persian` : `${locale}-u-ca-persian`;
+ }
+ return locale;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lib/dateTimeFormatter.ts` around lines 40 - 46,
resolveFormatterLocale currently appends the string "-u-ca-persian" and can
produce invalid BCP-47 tags (e.g., fa-u-nu-latn-u-ca-persian); change the
implementation to stop mutating the locale string and instead ensure callers
pass { calendar: "persian" } to Intl.DateTimeFormat when
isPersianCalendarLocale(locale) returns true, so update resolveFormatterLocale
(and any callers) to return the original locale unchanged and add logic where
DateTimeFormat is constructed to include calendar: "persian" for Persian locales
(keep references to resolveFormatterLocale and isPersianCalendarLocale to locate
changes).
| export function getWeekStartForLocale(locale: string | string[] | undefined): 0 | 6 { | ||
| const normalizedLocale = Array.isArray(locale) ? locale[0] : locale || "en"; | ||
| return isPersianCalendarLocale(normalizedLocale) ? SATURDAY : SUNDAY; | ||
| } |
There was a problem hiding this comment.
Consider handling empty array input defensively.
If an empty array is passed, locale[0] evaluates to undefined, which is then passed to isPersianCalendarLocale(undefined). However, when undefined is passed directly, the normalization defaults to "en". This creates inconsistent behavior between getWeekStartForLocale([]) and getWeekStartForLocale(undefined).
While empty arrays may be unlikely in practice, adding a fallback would make the function more robust and consistent.
🛡️ Proposed defensive fix
export function getWeekStartForLocale(locale: string | string[] | undefined): 0 | 6 {
- const normalizedLocale = Array.isArray(locale) ? locale[0] : locale || "en";
+ const normalizedLocale = Array.isArray(locale) ? (locale[0] || "en") : (locale || "en");
return isPersianCalendarLocale(normalizedLocale) ? SATURDAY : SUNDAY;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function getWeekStartForLocale(locale: string | string[] | undefined): 0 | 6 { | |
| const normalizedLocale = Array.isArray(locale) ? locale[0] : locale || "en"; | |
| return isPersianCalendarLocale(normalizedLocale) ? SATURDAY : SUNDAY; | |
| } | |
| export function getWeekStartForLocale(locale: string | string[] | undefined): 0 | 6 { | |
| const normalizedLocale = Array.isArray(locale) ? (locale[0] || "en") : (locale || "en"); | |
| return isPersianCalendarLocale(normalizedLocale) ? SATURDAY : SUNDAY; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lib/weekday.ts` around lines 12 - 15, getWeekStartForLocale treats
an empty array differently than undefined because Array.isArray(locale) ?
locale[0] : locale || "en" yields undefined for [], leading to inconsistent
behavior; update the normalization in getWeekStartForLocale to defensively
handle empty arrays by using the first element only if present (e.g., fallback
to "en" when locale is [] or locale[0] is undefined) before calling
isPersianCalendarLocale, so the function consistently returns SATURDAY or SUNDAY
for [], undefined, or string inputs.
|
This PR is intentionally scoped to locale + formatting only. A follow-up PR is planned to add Jalali-aware DatePicker grid rendering while keeping all booking logic, storage, and timezone calculations unchanged (Gregorian/ISO). If this direction is accepted, I will proceed with the DatePicker implementation in a separate PR. |
|
@Har2yQn78 this is a feature request. Can u please create an issue for it first? Also please add visual demo of ur changes |
bandhan-majumder
left a comment
bandhan-majumder
left a comment
There was a problem hiding this comment.
changes requested above
|
|
Added the requested issue reference and visual demo. This PR is intentionally scoped to the foundational locale + formatting layer first, and not the complete Persian/Jalali experience yet. Some UI translations are still incomplete and currently fall back to English where keys are missing. Before expanding the implementation further, I wanted to first validate whether this overall direction and architecture are acceptable. If this approach is approved, I plan to follow up with a separate PR focused on the Jalali-aware DatePicker/calendar grid while keeping all booking logic, timezone handling, and storage Gregorian/ISO internally. |
github-actions
Bot
added
✨ feature
|
2 participants
Summary
Adds foundational Persian (
fa) locale support with Jalali-aware date formatting usingIntl.This PR intentionally focuses on locale infrastructure and formatting behavior only. Booking logic, slot generation, timezone calculations, storage, and API payloads remain unchanged and Gregorian internally.
Changes
faas a locale target so it appears in the language pickerfa/common.jsonlocale scaffolding (missing keys fall back to English)Intlto ensure Jalali-aware date/time/weekday outputdayjsformatting pathsgetWeekStartForLocalewith Saturday as the default Persian week startNon-goals
This PR does NOT:
Follow-up
A future PR will introduce Jalali-aware date-picker grid rendering while preserving Gregorian internal scheduling semantics.
Fixed #29510