-
Notifications
You must be signed in to change notification settings - Fork 27
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
Timestamps with offset timezones throw on toLocaleString()
#223
Comments
Yep, this is a known issue over in the ECMA-402 (Internationalization) spec : tc39/ecma402#683 This polyfill does not include any localized text, so it's unlikely that we'd polyfill this gap in |
Oh, okay For anyone having the same problem, converting Temporal.ZonedDateTime.from("2020-08-05T20:06:13[+01:00]")
.toPlainDateTime()
.toLocaleString(undefined, {
day: "numeric",
month: "short",
});
// > "5 Aug" |
@zemlanin Your workaround suggests how we could polyfill this, at least for some use cases. In this comment I'll sketch out a possible solution. If you wanted to PR it, we'd welcome the contribution!
Here's a completely untested starting point for this solution: toLocaleString(
locales: Params['toLocaleString'][0] = undefined,
options: Params['toLocaleString'][1] = undefined
): string {
if (!ES.IsTemporalZonedDateTime(this)) throw new TypeError('invalid receiver');
const hasOffsetTimeZone = ES.TestTimeOfsetString(this.offset);
if (!offsetTimeZone) return new DateTimeFormat(locales, options).format(this);
// Pre-Temporal Intl.DateTimeFormat can't handle offset time zones, but if he time zone name isn't visible
// then we can just format as a PlainDateTime instead and avoid this problem.
const formatTimeZoneName = options.timeZoneName || (!hasDateOptions (options) && !hasTimeOptions(options));
if (!formatTimeZoneName) return new DateTimeFormat(locales, options).format(this.toPlainDateTime());
// If we get here, it's an offset time zone and we need to format the time zone name. If the offset zone is
// aligned on an hour boundary, we can substitute an IANA Etc/GMT* zone.
const { offsetNanoseconds } = this;
if (offsetNanoseconds % 3.6e12 !== 0) {
throw new RangeError (`Offset time zone ${this.offset} cannot be formatted with this polyfill because it is not aligned on an hour boundary`);
}
const hours = offsetNanoseconds / 3.6e12;
if (hours > 14 || hours < 12) {
throw new RangeError (`Offset time zone ${this.offset} cannot be formatted with this polyfill because it is not between -12:00 and +14:00`);
}
// IANA's offset zones use reverse signs for POSIX compatibility.
// So an ISO 8601 +05:00 is "GMT-5" in the IANA TZDB.
const ianaOffsetZone = `GMT${hours > 0 '-' : '+'}${hours}`;
const toFormat = this.withTimeZone(ianaOffsetZone);
return new DateTimeFormat(locales, options).format(toFormat);
} |
After thinking about it for a bit, I think this workaround may be better applied in intl.ts as part of the DateTimeFormat part of the polyfill, because then it would work for a wider set of use cases, and would locate the DTF-specific polyfilling together. |
Wouldn't this lead to a value cycle between ecmascript.ts and intl.ts? Probably a "lazy" cycle nonetheless, but that's maybe still a problem. Leaving all that logic within intl.ts sounds reasonable to me. I'm happy to review a PR for this for "typescript correctness", but not for business logic correctness :) |
Hi. Don't know if this a polyfill issue or a spec issue. Sorry if it is the latter
The text was updated successfully, but these errors were encountered: