feat: add icu::set_default_time_zone / get_default_time_zone#2000
Open
bartlomieju wants to merge 2 commits into
Open
feat: add icu::set_default_time_zone / get_default_time_zone#2000bartlomieju wants to merge 2 commits into
bartlomieju wants to merge 2 commits into
Conversation
Expose ICU's default time zone so embedders can install a specific IANA zone (e.g. from the TZ environment variable) instead of relying solely on host detection via DateTimeConfigurationChangeNotification(Redetect). On Windows, ICU's host detection reads the time zone from the OS and ignores TZ, so Redetect cannot honor TZ there. set_default_time_zone wraps icu::TimeZone::adoptDefault(createTimeZone(id)) so the requested zone is resolved on every platform. get_default_time_zone returns the current default's IANA id. WIP: not yet built/tested against a full V8 build locally.
This was referenced Jun 2, 2026
The test mutates the process-wide ICU default time zone, which affects how other tests render Date values, so it must hold the exclusive lock (sequential_test) rather than the shared one. Also restore the original zone afterwards so test ordering doesn't matter.
bartlomieju
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Exposes ICU's process-wide default time zone to embedders:
v8::icu::set_default_time_zone(&str)wrapsicu::TimeZone::adoptDefault(icu::TimeZone::createTimeZone(id)).v8::icu::get_default_time_zone() -> Stringreturns the currentdefault's IANA id.
Mirrors the existing
set_default_locale/get_language_tagbindingpair in
src/icu.rsandsrc/binding.cc.Motivation
V8 derives its
Datetime zone from ICU. ICU's host detection reads theTZenvironment variable on Unix but not on Windows, where it readsthe OS/registry instead. So
DateTimeConfigurationChangeNotification(Redetect)cannot make V8 honorTZon Windows. Withset_default_time_zone, an embedder can install thezone explicitly from
TZand then notify isolates withTimeZoneDetection::Skip, getting consistent cross-platform behavior thatmatches Node. Downstream Deno change: denoland/deno#34705 (fixes
denoland/deno#15370).
Testing
Adds
icu_default_time_zone, which sets the default zone, asserts thegetter round-trips, and checks that a fresh isolate's
Dateresolves theinstalled zone. It uses
sequential_test()because it mutatesprocess-global ICU state.
The new C++ in
binding.ccwas compiled and run against ICU 77 headersstandalone, confirming the API usage and that
set/getround-trips forAmerica/New_YorkandUTC. (A full from-sourcecargo testwasn't runlocally; CI builds V8 from source and will exercise the binding.)