Skip to content
This repository has been archived by the owner on Jan 10, 2025. It is now read-only.

Update and Map Deprecated Timezone #2842

Merged
merged 3 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/light-students-learn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/dates': patch
---

Correctly support deprecated timezones in older browsers when calling `formatDate()`. For example, modern browsers support both `Europe/Kyiv` and (the now deprecated) `Europe/Kiev`, but browsers as recent as Chrome 131 on MacOS only support `Europe/Kiev`. Note: This is a purely internal change which should not effect the result of calling `formatDate()`.
1 change: 1 addition & 0 deletions packages/dates/src/deprecated-timezones.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const deprecatedTimezones: {[key: string]: string} = {
'Canada/Yukon': 'America/Whitehorse',
'Chile/Continental': 'America/Santiago',
'Chile/EasterIsland': 'Pacific/Easter',
'Europe/Kyiv': 'Europe/Kiev',
Cuba: 'America/Havana',
Egypt: 'Africa/Cairo',
Eire: 'Europe/Dublin',
Expand Down
9 changes: 8 additions & 1 deletion packages/dates/src/utilities/formatDate.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import {mapDeprecatedTimezones} from '../map-deprecated-timezones';

const intl = new Map<string, Intl.DateTimeFormat>();
export function memoizedGetDateTimeFormat(
locales?: string | string[],
Expand Down Expand Up @@ -58,7 +60,12 @@ export function formatDate(
}).format(adjustedDate);
}

return memoizedGetDateTimeFormat(locales, options).format(date);
return memoizedGetDateTimeFormat(locales, {
...options,
timeZone: options.timeZone
? mapDeprecatedTimezones(options.timeZone)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HIII @jesstelford ❤️ SOOOOOOOOOO when we implemented the change inside i18n and tested out the snapshots ... we weren't able to see the changes actually fix the issue 😬 .... so we're trying it out directly in formatDate as parts of web directly call formatDate from @shopify/dates we shipped https://github.com/Shopify/web/pull/152962/files that actually fixes it in web, but if we wanted to do our "due diligence" for other repo's, thought we might as well try this LOL 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! During your testing, did you notice any regressions with this change (trying some different mapped and non-mapped timezones)? If not, I agree we should ship this as a bug fix to ensure we've covered all our bases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HIII JESS!!! do you mean for the i18n testing? Or this current one? SADLY Npm hasn't finished propogating lol, so I'm just waiting to test 😢

: undefined,
}).format(date);
}

export function dateTimeFormatCacheKey(
Expand Down
22 changes: 22 additions & 0 deletions packages/dates/src/utilities/tests/formatDate.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import {formatDate} from '../formatDate';

jest.mock('../../map-deprecated-timezones', () => ({
mapDeprecatedTimezones: jest.fn(),
}));

const mapDeprecatedTimezones: jest.Mock = jest.requireMock(
'../../map-deprecated-timezones',
).mapDeprecatedTimezones;

describe('formatDate', () => {
it('maps the deprecated timezone', () => {
const date = new Date();
const locale = 'en-UA';
const options: Intl.DateTimeFormatOptions = {
timeZone: 'Europe/Kyiv',
};

formatDate(date, locale, options);
expect(mapDeprecatedTimezones).toHaveBeenCalledWith('Europe/Kyiv');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THIS TEST ISN'T GREAT ... but like at least we're testing that the function is actually being called, AND the fact that quilt/packages/dates/src/tests/map-deprecated-timezones.test.ts is testing that it's mapping / converting, feels safe to me - Ideally mocking out a real "bug" would be great here, but since we can't ... this is good enough imo 😄 ❤️

});
});
Loading