Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom zone formatting support #1377

Merged
merged 3 commits into from
Mar 4, 2023
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
58 changes: 42 additions & 16 deletions src/impl/locale.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { padStart, roundTo, hasRelative } from "./util.js";
import { padStart, roundTo, hasRelative, formatOffset } from "./util.js";
import * as English from "./english.js";
import Settings from "../settings.js";
import DateTime from "../datetime.js";
Expand Down Expand Up @@ -197,9 +197,13 @@ class PolyNumberFormatter {
class PolyDateFormatter {
constructor(dt, intl, opts) {
this.opts = opts;
this.originalZone = undefined;

let z = undefined;
if (dt.zone.isUniversal) {
if (this.opts.timeZone) {
// Don't apply any workarounds if a timeZone is explicitly provided in opts
this.dt = dt;
} else if (dt.zone.type === "fixed") {
// UTC-8 or Etc/UTC-8 are not part of tzdata, only Etc/GMT+8 and the like.
// That is why fixed-offset TZ is set to that unless it is:
// 1. Representing offset 0 when UTC is used to maintain previous behavior and does not become GMT.
Expand All @@ -212,25 +216,23 @@ class PolyDateFormatter {
z = offsetZ;
this.dt = dt;
} else {
// Not all fixed-offset zones like Etc/+4:30 are present in tzdata.
// So we have to make do. Two cases:
// 1. The format options tell us to show the zone. We can't do that, so the best
// we can do is format the date in UTC.
// 2. The format options don't tell us to show the zone. Then we can adjust them
// the time and tell the formatter to show it to us in UTC, so that the time is right
// and the bad zone doesn't show up.
// Not all fixed-offset zones like Etc/+4:30 are present in tzdata so
// we manually apply the offset and substitute the zone as needed.
z = "UTC";
if (opts.timeZoneName) {
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 isn't entirely correct since using timeStyle: "full" will also show the timezone.

this.dt = dt;
} else {
this.dt = dt.offset === 0 ? dt : DateTime.fromMillis(dt.ts + dt.offset * 60 * 1000);
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 throws away any locale information so it would be more correct to just switch the datetime to UTC and apply the offset there.

}
this.dt = dt.offset === 0 ? dt : dt.setZone("UTC").plus({ minutes: dt.offset });
this.originalZone = dt.zone;
}
} else if (dt.zone.type === "system") {
this.dt = dt;
} else {
} else if (dt.zone.type === "iana") {
this.dt = dt;
z = dt.zone.name;
} else {
// Custom zones can have any offset / offsetName so we just manually
// apply the offset and substitute the zone as needed.
z = "UTC";
this.dt = dt.setZone("UTC").plus({ minutes: dt.offset });
this.originalZone = dt.zone;
}

const intlOpts = { ...this.opts };
Expand All @@ -239,11 +241,35 @@ class PolyDateFormatter {
}

format() {
if (this.originalZone) {
// If we have to substitute in the actual zone name, we have to use
// formatToParts so that the timezone can be replaced.
return this.formatToParts()
.map(({ value }) => value)
.join("");
}
return this.dtf.format(this.dt.toJSDate());
}

formatToParts() {
return this.dtf.formatToParts(this.dt.toJSDate());
const parts = this.dtf.formatToParts(this.dt.toJSDate());
if (this.originalZone) {
return parts.map((part) => {
if (part.type === "timeZoneName") {
const offsetName = this.originalZone.offsetName(this.dt.ts, {
locale: this.dt.locale,
format: this.opts.timeZoneName,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The docs currently say the format allows short or long but Intl.DateTimeFormat now supports more options. I think it's more correct to just pass it through as the IANAZone technically allows you to pass any of the valid options. To be more correct, we might also want to consider adding timeStyle as an option since that can also influence the timezone output.

});
return {
...part,
value: offsetName,
};
} else {
return part;
}
});
}
return parts;
}

resolvedOptions() {
Expand Down
70 changes: 68 additions & 2 deletions test/datetime/format.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* global test expect */

import { DateTime } from "../../src/luxon";
import { DateTime, Zone, FixedOffsetZone } from "../../src/luxon";

const dtMaker = () =>
DateTime.fromObject(
Expand All @@ -20,6 +20,50 @@ const dtMaker = () =>
dt = dtMaker(),
invalid = DateTime.invalid("because");

class CustomZone extends Zone {
constructor(name, offset) {
super();
this._name = name;
this._offset = offset;
}

get isUniversal() {
return true;
}

get isValid() {
return true;
}

get name() {
return this._name;
}

get type() {
return "custom";
}

equals(zone) {
return zone instanceof CustomZone && zone._name === this._name && zone._offset === this._offset;
}

offset(_ms) {
return this._offset;
}

offsetName(_ms, { format }) {
if (format === "short") {
return this._name.substring(0, 4);
} else {
return this._name;
}
}

formatOffset(...args) {
return FixedOffsetZone.prototype.formatOffset(...args);
}
}

//------
// #toJSON()
//------
Expand Down Expand Up @@ -398,7 +442,29 @@ test("DateTime#toLocaleString() shows things with UTC if fixed-offset zone with

test("DateTime#toLocaleString() does the best it can with unsupported fixed-offset zone when showing the zone", () => {
expect(dt.setZone("UTC+4:30").toLocaleString(DateTime.DATETIME_FULL)).toBe(
"May 25, 1982 at 9:23 AM UTC"
"May 25, 1982 at 1:53 PM UTC+4:30"
);
});

test("DateTime#toLocaleString() does the best it can with unsupported fixed-offset zone with timeStyle full", () => {
expect(dt.setZone("UTC+4:30").toLocaleString({ timeStyle: "full" })).toBe("1:53:54 PM UTC+4:30");
});

test("DateTime#toLocaleString() shows things in the right custom zone", () => {
expect(dt.setZone(new CustomZone("CUSTOM", 30)).toLocaleString(DateTime.DATETIME_SHORT)).toBe(
"5/25/1982, 9:53 AM"
);
});

test("DateTime#toLocaleString() shows things in the right custom zone when showing the zone", () => {
expect(dt.setZone(new CustomZone("CUSTOM", 30)).toLocaleString(DateTime.DATETIME_FULL)).toBe(
"May 25, 1982 at 9:53 AM CUST"
);
});

test("DateTime#toLocaleString() shows things in the right custom zone with timeStyle full", () => {
expect(dt.setZone(new CustomZone("CUSTOM", 30)).toLocaleString({ timeStyle: "full" })).toBe(
"9:53:54 AM CUSTOM"
);
});

Expand Down