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

Intl.supportedValuesOf('timeZone') not containing complete data #43678

Closed
thernstig opened this issue Jul 4, 2022 · 8 comments
Closed

Intl.supportedValuesOf('timeZone') not containing complete data #43678

thernstig opened this issue Jul 4, 2022 · 8 comments
Labels
feature request Issues that request new features to be added to Node.js. v8 engine Issues and PRs related to the V8 dependency.

Comments

@thernstig
Copy link
Contributor

thernstig commented Jul 4, 2022

What is the problem this feature will solve?

Node.js 18 added support for the Intl.supportedValuesOf(), which in turn came from the V8 v9.9 release. Part of Intl.supportedValuesOf() is e.g. Intl.supportedValuesOf('timeZone').

But that list is not complete when run in Node.js.

Internationalization support is built with full ICU, meaning system-icu is included.

This for example means it is possible to set the TZ variable to something that exists on the system, but is not shown in the output of Intl.supportedValuesOf('timeZone'). This makes Intl.supportedValuesOf('timeZone') somewhat useless as it cannot be used to check if someone e.g. set an allowed TZ value.

What is the feature you are proposing to solve the problem?

Intl.supportedValuesOf() is defined as showing information supported by the implementation. If this is up for interpretation, it would be good if Intl.supportedValuesOf('timeZone') would output all valid values even the ones coming from the system.

What alternatives have you considered?

No response

@thernstig thernstig added the feature request Issues that request new features to be added to Node.js. label Jul 4, 2022
@bnoordhuis
Copy link
Member

Internationalization support is built with full ICU, meaning system-icu is included.

You're misinterpreting what that page says. full-icu and system-icu are different build modes. full-icu uses the bundled copy, system-icu links dynamically against the system ICU.

With that out of the way, I think you're looking for the ICU_TIMEZONE_FILES_DIR environment variable, to tell ICU where to look for additional files. Keep in mind though it only understands its own file format, not the tzdata format

@thernstig
Copy link
Contributor Author

@bnoordhuis I misunderstood that part about ICU then, but the fact still remains I am able to set time zones not visible from Intl.supportedValuesOf('timeZone') as process.env.TZ. And it works.

What I did was compare the output of Intl.supportedValuesOf('timeZone') towards timedatectl list-timezones on my system, and then tried to set TZ to value found in the latter but not the former. And Node.js picks it up properly and understand the TZ I've set.

So Intl.supportedValuesOf('timeZone') is in some way incomplete.

@bnoordhuis
Copy link
Member

bnoordhuis commented Jul 6, 2022

Right, I think I understand what you mean. That's a feature of ICU: it scans the /usr/share/zoneinfo directory (or wherever your system's tzdata lives) for a filename that matches TZ.

It doesn't actually read the timezone file though, it's just a trick to figure out if the timezone name is something the system understands. E.g., if TZ=Iceland and /usr/share/zoneinfo/Iceland exists - match.

Another ICU trick: it scans the zoneinfo directory for the file matching /etc/localtime. No timezone parsing happens, just a byte-by-byte file comparison.

edit: in case I'm not being clear: this is ICU internal behavior that isn't visible to node or V8. There's no way to expose that through Intl.supportedValuesOf().

@thernstig
Copy link
Contributor Author

@bnoordhuis so in effect, this issue should be closed, and there is still no way for Node.js users to get supported time zones?

Having read a bit about Intl.supportedValuesOf() I thought any implementation of it should return all supported values (hence the name). At least that is how I interpret https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/supportedValuesOf and other resources.

@bnoordhuis
Copy link
Member

Reading the spec for AvailableTimeZones, I suppose it's possible V8 calls the wrong ICU API, although I'm not sure what API it should call instead.

If I interpret the CanonicalizeTimeZoneName step correctly, then I would expect "UTC" to show up in the list - but it's not there, even though ICU definitely knows what UTC is.

V8 gets the timezone names from icu::TimeZone::createTimeZoneIDEnumeration(UCAL_ZONE_TYPE_CANONICAL_LOCATION).

Maybe it should be using UCAL_ZONE_TYPE_CANONICAL because that includes zones like Etc/UTC, Etc/GMT+2 and PST8PDT (good) but on my system also includes things like SystemV/PST8PDT (not good.)

I suggest taking it up with the V8 team because it's ultimately an upstream issue, either in V8 or ICU.

@bnoordhuis bnoordhuis added the v8 engine Issues and PRs related to the V8 dependency. label Jul 6, 2022
@bnoordhuis
Copy link
Member

Since there's nothing to do on our side, I'll go ahead and close the issue. Can you post the link to the V8 issue if there is one? Thanks.

@bnoordhuis bnoordhuis closed this as not planned Won't fix, can't repro, duplicate, stale Jul 15, 2022
@thernstig
Copy link
Contributor Author

For cross reference, tc39/proposal-intl-enumeration#47 seems to indicate this as well, so I will write a bug report on v8.

@thernstig
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests

2 participants