Fix timezone extraction to work in all locales

[sbc100]

Followup to #21585

[jrobichaux]

Could you do something like: https://stackoverflow.com/a/78194184 ?

var timeParts = new Intl.DateTimeFormat(undefined, {timeZoneName: 'short'}).formatToParts(date);
var extractZone = timeParts.find(item => item.type == 'timeZoneName').value;

I don't know if timeZoneName is guaranteed to exist, based on the Mozilla docs on Intl.DateTimeFormat so you might want some error checks around that.

[sbc100]

Could you do something like: https://stackoverflow.com/a/78194184 ?

var timeParts = new Intl.DateTimeFormat(undefined, {timeZoneName: 'short'}).formatToParts(date);
var extractZone = timeParts.find(item => item.type == 'timeZoneName').value;

I don't know if timeZoneName is guaranteed to exist, based on the Mozilla docs on Intl.DateTimeFormat so you might want some error checks around that.

The hours12 solution looks like it should less code and just as reliable.

My reading of the MDN page is that timeZoneName should be available everywhere we support running. I added some assertions in case we do run into any places where its not. I think we should avoid adding fallbacks/checking in release builds until we know we need it.

[sbc100]

Could you do something like: https://stackoverflow.com/a/78194184 ?

var timeParts = new Intl.DateTimeFormat(undefined, {timeZoneName: 'short'}).formatToParts(date);
var extractZone = timeParts.find(item => item.type == 'timeZoneName').value;

I don't know if timeZoneName is guaranteed to exist, based on the Mozilla docs on Intl.DateTimeFormat so you might want some error checks around that.

The hours12 solution looks like it should less code and just as reliable.

My reading of the MDN page is that timeZoneName should be available everywhere we support running. I added some assertions in case we do run into any places where its not. I think we should avoid adding fallbacks/checking in release builds until we know we need it.

I tested this code going back to node v14.1.0 and also in the d8 shell and it seems to work fine.

[jrobichaux]

One more thing and I promise I'll leave you alone. :)

You might want to have a test with a time in a timezone with a 15 or 30 minute offset. For example:

new Date().toLocaleTimeString(undefined, {hour12:false, timeZoneName:'short', timeZone:'Asia/Kathmandu'}).split(' ')[1]

returns GMT+5:45 for me, which would break your 6 character TZNAME_MAX.

I am SO sorry this blew up into so much work. Time zones and locales are HARD.

[sbc100]

One more thing and I promise I'll leave you alone. :)

You might want to have a test with a time in a timezone with a 15 or 30 minute offset. For example:

new Date().toLocaleTimeString(undefined, {hour12:false, timeZoneName:'short', timeZone:'Asia/Kathmandu'}).split(' ')[1]

returns GMT+5:45 for me, which would break your 6 character TZNAME_MAX.

Do you know how POSIX/glibc handles this case? Should we just silently truncate to 6 chars since this is such an outlier case?

[sbc100]

One more thing and I promise I'll leave you alone. :)
You might want to have a test with a time in a timezone with a 15 or 30 minute offset. For example:
new Date().toLocaleTimeString(undefined, {hour12:false, timeZoneName:'short', timeZone:'Asia/Kathmandu'}).split(' ')[1]
returns GMT+5:45 for me, which would break your 6 character TZNAME_MAX.

Do you know how POSIX/glibc handles this case? Should we just silently truncate to 6 chars since this is such an outlier case?

Answering my own question it looks like glibc prints +0545 in this case rather than a 3 letter timezone name.

[sbc100]

Updated TZNAME_MAX to be 16 in order to handle 'Asia/Kathmandu'. Also added test for this particular timezone.

[dschuff]

This test is failing on mac on the Chromium CI bots: https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8752412488426808241/+/u/Emscripten_testsuite__other_/stdout

[sbc100]

Oh way, I disabled it under macOS on github CI, but maybe I just need to disable it completely on macOS.

@dschuff can you confirm if fails for you locally on your mac? Do you think is reasonable to simpley not support the locale testing on mac for this reason?

[dschuff]

Yes, this fails locally for me with

Fatal Python error: config_get_locale_encoding: failed to get the locale encoding: nl_langinfo(CODESET) failed
Python runtime state: preinitialized

Which is the same failure on the Chromium bots, and I guess is the same failure you saw on Circle, right (i.e. python just doesn't work when you ask it for a locale that's not on the system?)

[sbc100]

Do you know how locales work on macOS? Is this s limitation of the OS, or of the python executable that we ship?

[dschuff]

No idea how locales work really. The python executable I'm using locally is not the one we ship though.

[sbc100]

OK I think we will just have to disable that test on mac in all cases then.

[sbc100]

#21624