Skip to content

Commit 004c26d

Browse files
authored
Merge pull request #19 from calidy-com/claude/investigate-issue-18-pH77G
fix: prevent toCalendarSystem() from mutating global locale state
2 parents b886d85 + 9799710 commit 004c26d

2 files changed

Lines changed: 152 additions & 13 deletions

File tree

src/index.js

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -429,22 +429,35 @@ export default (options, dayjsClass, dayjsFactory) => {
429429
// Store the target calendar system in $C
430430
newInstance.$C = calendar;
431431
}
432-
// Update the locale to reflect the new calendar system
433-
dayjsFactory.updateLocale(
434-
newInstance.$L,
435-
calendarSystems[calendar].localeOverride(newInstance.$L)
432+
433+
// Store the calendar-specific locale override on the instance itself.
434+
// IMPORTANT: We intentionally do NOT call dayjsFactory.updateLocale() here.
435+
// Mutating the global locale would cause all dayjs instances to use this
436+
// calendar's month names, breaking the expected behavior where only this
437+
// specific instance should be affected. See GitHub issue #18.
438+
newInstance.$calendarLocale = calendarSystems[calendar].localeOverride(
439+
newInstance.$L
436440
);
437-
// dayjsFactory.locale(
438-
// newInstance.$L,
439-
// {
440-
// ...newInstance.$locale(),
441-
// ...calendarSystems[calendar].localeOverride(newInstance.$L),
442-
// },
443-
// true
444-
// );
441+
445442
return newInstance;
446443
};
447444

445+
// Override $locale() to return instance-specific calendar locale when available.
446+
// This ensures format() uses the correct month names for the calendar system
447+
// without mutating the global locale. See GitHub issue #18.
448+
const old$locale = dayjsClass.prototype.$locale;
449+
dayjsClass.prototype.$locale = function () {
450+
const baseLocale = old$locale.call(this);
451+
// If this instance has calendar-specific locale overrides, merge them
452+
if (this.$calendarLocale) {
453+
return {
454+
...baseLocale,
455+
...this.$calendarLocale,
456+
};
457+
}
458+
return baseLocale;
459+
};
460+
448461
// Handle locale inconsistencies if any:
449462
function handleLocale(preset, object, calendarSystem) {
450463
const buggyLocales = [
@@ -472,7 +485,16 @@ export default (options, dayjsClass, dayjsFactory) => {
472485
const oldLocale = dayjsClass.prototype.locale;
473486
dayjsClass.prototype.locale = function (preset, object) {
474487
object = handleLocale(preset, object, this.$C || "gregory");
475-
return oldLocale.bind(this)(preset, object);
488+
const result = oldLocale.bind(this)(preset, object);
489+
// When locale changes on an instance with a calendar system,
490+
// regenerate the instance-specific calendar locale for the new locale.
491+
// This ensures month names are correct for the new locale.
492+
if (this.$C && this.$C !== "gregory" && calendarSystems[this.$C]) {
493+
result.$calendarLocale = calendarSystems[this.$C].localeOverride(
494+
result.$L
495+
);
496+
}
497+
return result;
476498
};
477499

478500
const oldDayjsFactoryLocale = dayjsFactory.locale;

test/instanceIsolation.test.js

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
/**
2+
* Regression tests for GitHub issue #18
3+
* https://github.com/calidy-com/dayjs-calendarsystems/issues/18
4+
*
5+
* Verifies that calling toCalendarSystem() on an instance does NOT
6+
* mutate global state or affect other dayjs instances.
7+
*/
8+
import dayjs from "dayjs";
9+
import "dayjs/locale/ar";
10+
import calendarSystems from "../src/index";
11+
import GregoryCalendarSystem from "../src/calendarSystems/GregoryCalendarSystem";
12+
import PersianCalendarSystem from "../src/calendarSystems/PersianCalendarSystem";
13+
import HijriCalendarSystem from "../src/calendarSystems/HijriCalendarSystem";
14+
import HebrewCalendarSystem from "../src/calendarSystems/HebrewCalendarSystem";
15+
16+
describe("Instance isolation - GitHub issue #18", () => {
17+
beforeAll(() => {
18+
dayjs.extend(calendarSystems);
19+
dayjs.registerCalendarSystem("gregory", new GregoryCalendarSystem());
20+
dayjs.registerCalendarSystem("persian", new PersianCalendarSystem());
21+
dayjs.registerCalendarSystem("islamic", new HijriCalendarSystem());
22+
dayjs.registerCalendarSystem("hebrew", new HebrewCalendarSystem());
23+
});
24+
25+
test("toCalendarSystem on instance should NOT affect new instances", () => {
26+
// Create a Gregorian date
27+
const gregorianDate = dayjs("2023-05-14");
28+
expect(gregorianDate.format("MMMM")).toEqual("May");
29+
expect(gregorianDate.$C).toBeUndefined();
30+
31+
// Convert to Islamic calendar
32+
const islamicDate = gregorianDate.toCalendarSystem("islamic");
33+
expect(islamicDate.$C).toEqual("islamic");
34+
expect(islamicDate.format("MMMM")).toEqual("Shawwal");
35+
36+
// Create a NEW dayjs instance - it should still be Gregorian
37+
const newGregorianDate = dayjs("2023-05-14");
38+
expect(newGregorianDate.$C).toBeUndefined();
39+
expect(newGregorianDate.format("MMMM")).toEqual("May");
40+
});
41+
42+
test("multiple calendar conversions should not interfere with each other", () => {
43+
const baseDate = dayjs("2023-05-14");
44+
45+
// Convert to different calendars
46+
const persian = baseDate.toCalendarSystem("persian");
47+
const islamic = baseDate.toCalendarSystem("islamic");
48+
const hebrew = baseDate.toCalendarSystem("hebrew");
49+
50+
// Each should have its own calendar system
51+
expect(persian.$C).toEqual("persian");
52+
expect(islamic.$C).toEqual("islamic");
53+
expect(hebrew.$C).toEqual("hebrew");
54+
55+
// Each should format with its own month names
56+
expect(persian.format("MMMM")).toEqual("Ordibehesht");
57+
expect(islamic.format("MMMM")).toEqual("Shawwal");
58+
expect(hebrew.format("MMMM")).toEqual("Iyar");
59+
60+
// Original should still be Gregorian
61+
expect(baseDate.$C).toBeUndefined();
62+
expect(baseDate.format("MMMM")).toEqual("May");
63+
});
64+
65+
test("new instances after calendar conversion should be Gregorian", () => {
66+
// Convert an instance to Islamic
67+
dayjs("2023-01-15").toCalendarSystem("islamic");
68+
69+
// Create multiple new instances - all should be Gregorian
70+
const dates = [
71+
dayjs("2023-01-15"),
72+
dayjs("2023-06-20"),
73+
dayjs("2023-12-25"),
74+
];
75+
76+
dates.forEach((date) => {
77+
expect(date.$C).toBeUndefined();
78+
expect(date.format("MMMM")).toMatch(
79+
/^(January|February|March|April|May|June|July|August|September|October|November|December)$/
80+
);
81+
});
82+
});
83+
84+
test("calendar instance should preserve locale when cloned", () => {
85+
const islamicDate = dayjs("2023-05-14").toCalendarSystem("islamic");
86+
const cloned = islamicDate.clone();
87+
88+
expect(cloned.$C).toEqual("islamic");
89+
expect(cloned.format("MMMM")).toEqual("Shawwal");
90+
});
91+
92+
test("changing locale on calendar instance should update month names", () => {
93+
const islamicDate = dayjs("2023-04-10").toCalendarSystem("islamic");
94+
expect(islamicDate.format("MMMM")).toEqual("Ramadan");
95+
96+
// Change to Arabic locale
97+
const arabicIslamicDate = islamicDate.locale("ar");
98+
expect(arabicIslamicDate.format("MMMM")).toEqual("رمضان");
99+
100+
// Original should still have English month names
101+
expect(islamicDate.format("MMMM")).toEqual("Ramadan");
102+
});
103+
104+
test("factory method vs instance method distinction", () => {
105+
// Instance method should only affect that instance
106+
const instance1 = dayjs("2023-05-14");
107+
const islamicInstance = instance1.toCalendarSystem("islamic");
108+
109+
expect(islamicInstance.$C).toEqual("islamic");
110+
expect(instance1.$C).toBeUndefined();
111+
112+
// New instances should still default to Gregorian
113+
const instance2 = dayjs("2023-05-14");
114+
expect(instance2.$C).toBeUndefined();
115+
expect(instance2.format("MMMM")).toEqual("May");
116+
});
117+
});

0 commit comments

Comments
 (0)