fix(intl): resolve and apply the host time zone in DateTimeFormat - #6452
Merged
Conversation
`Intl.DateTimeFormat().resolvedOptions().timeZone` returned `"UTC"` regardless of the host, and formatting always used UTC — so on a non-UTC machine every `DateTimeFormat`/`Date.toLocaleString` result was off by the host offset, and Node's `Europe/Berlin` (etc.) never appeared. (Surfaced by a Next.js app: its next-intl config reads `resolvedOptions().timeZone`, so the hydrated payload differed from Node.) - `date.rs`: `host_time_zone_name()` resolves the IANA id from `TZ` (honored like libc/Node), else the `/etc/localtime` symlink; `zone_offset_seconds()` returns a DST-aware offset — 0 for UTC/GMT, a parsed fixed offset for `±HH:mm`, and the process zone via libc for the host zone. - The `DateTimeFormat` constructor defaults `timeZone` to the host zone (was UTC), `resolvedOptions()` reports it, and the two instance format paths shift the epoch into the configured zone before extracting components. - `Date.prototype.toLocaleString` renders the instant in the resolved zone (the `timeZone` option, else host) instead of UTC. Under `TZ=UTC` (typical CI) the host zone is UTC, so this is a no-op there; the full perry-runtime suite passes on both a UTC and a Europe/Berlin host. Scope: a named zone that is NOT the host zone (e.g. `America/New_York` on a Europe/Berlin host) still needs the OS tz database to resolve — mutating libc's global `TZ` is unsafe in a threaded runtime — so it falls back to UTC for now. The exact cases (default/host zone, UTC, fixed numeric offsets) cover the common usage; a follow-up can wire jiff-tzdb for arbitrary named zones.
📝 WalkthroughWalkthroughChangesHost Time-Zone Formatting
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Date
participant IntlDateTimeFormat
participant DateRuntime
participant LocaleFormatter
Date->>IntlDateTimeFormat: format instant
IntlDateTimeFormat->>DateRuntime: resolve timeZone offset
DateRuntime-->>IntlDateTimeFormat: offset seconds
IntlDateTimeFormat->>LocaleFormatter: format zone-local fields
LocaleFormatter-->>Date: formatted date-time
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-runtime/src/intl.rs`:
- Around line 318-337: In crates/perry-runtime/src/intl.rs:318-337, replace
resolved_date_time_zone with a unified resolver that canonicalizes valid zones,
throws a RangeError for explicitly provided invalid timeZone values, and falls
back to "UTC" when the host default zone is unrecognized. In
crates/perry-runtime/src/intl.rs:1156-1176, remove make_instance’s duplicated
validation and call the unified resolved_date_time_zone helper directly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: af972856-17da-488f-b1cd-f48b172b8dd8
📒 Files selected for processing (5)
crates/perry-runtime/src/date.rscrates/perry-runtime/src/intl.rscrates/perry-runtime/src/intl/date_collator.rscrates/perry-runtime/src/object/date_proto_thunks.rstest-files/test_gap_intl_host_timezone.ts
… on default) + split files
Two correctness bugs this PR introduced, pulling opposite ways (ECMA-402):
* `resolved_date_time_zone` swallowed an invalid explicit `timeZone` option,
returning the raw string — `toLocaleString(x, {timeZone: '+9'})` silently
formatted instead of throwing RangeError.
* `make_instance` threw RangeError when the DEFAULT host zone was
unrecognized. ECMA-402 DefaultTimeZone must ALWAYS yield a valid zone,
falling back to UTC; the constructor must not throw when no option is given.
Consolidated both into a single `resolved_date_time_zone` that throws only on an
explicit invalid zone and falls back to UTC for an unrecognized host default;
`make_instance` now calls it. Verified against node: a malformed explicit zone
(bad offset, empty segment) throws RangeError on both the constructor and
toLocaleString paths; valid named/offset zones and the host default are
unchanged. (A structurally-valid-but-nonexistent named zone like 'Not/AZone'
still passes — Perry has no tz database — a pre-existing limitation unrelated to
this fix.)
The consolidation plus a file split keeps both files under the 2000-line cap:
time-zone helpers → `intl/time_zone.rs`, month/weekday name tables →
`intl/date_names.rs`. All 13 intl/datetime gap tests pass.
Contributor
Author
|
Fixed both — the Consolidated the zone resolution.
One honest limitation: a structurally-valid-but-nonexistent named zone like File-size. The consolidation removed the duplication, and a split keeps both files under the 2000-line cap: time-zone helpers → |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/perry-runtime/src/intl/time_zone.rs (2)
17-29: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid unnecessary
Stringclone.
explicit.clone()clones the innerString(if present) just to checkexplicit.is_some()later. You can avoid this heap allocation by extracting the boolean check first and then consuming the Option directly.♻️ Proposed refactor
- let explicit = get_option_string(options, "timeZone"); - let tz = explicit - .clone() - .unwrap_or_else(|| crate::date::host_time_zone_name().to_string()); + let explicit = get_option_string(options, "timeZone"); + let is_explicit = explicit.is_some(); + let tz = explicit + .unwrap_or_else(|| crate::date::host_time_zone_name().to_string()); let canonical = if matches!(tz.as_bytes().first(), Some(b'+') | Some(b'-')) { is_valid_offset_time_zone(&tz).then(|| canonicalize_offset_time_zone(&tz)) } else { canonicalize_named_time_zone(&tz) }; match canonical { Some(c) => c, - None if explicit.is_some() => { + None if is_explicit => { throw_range_error(&format!("Invalid time zone specified: {tz}")) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/intl/time_zone.rs` around lines 17 - 29, Update the time-zone handling around explicit and tz to record whether explicit is present before consuming it, then consume explicit directly when selecting the fallback host time zone instead of cloning the inner String. Preserve the existing explicit.is_some() behavior in the canonical match error branch.
85-102: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid allocating a
Vecfor string splitting.
tz.split('/').collect()allocates a vector to hold the segments. Because you only need to enforce a minimum segment count and iterate over the characters, you can consume theSplititerator directly and track the segment count, bypassing the heap allocation.♻️ Proposed refactor
- let segments: Vec<&str> = tz.split('/').collect(); - if segments.len() < 2 { - return None; - } let mut has_alpha = false; - for seg in &segments { + let mut segment_count = 0; + for seg in tz.split('/') { + segment_count += 1; if seg.is_empty() { return None; } for b in seg.bytes() { if b.is_ascii_alphabetic() { has_alpha = true; } else if !(b.is_ascii_alphanumeric() || b == b'_' || b == b'+' || b == b'-') { return None; } } } - has_alpha.then(|| tz.to_string()) + if segment_count < 2 { + return None; + } + has_alpha.then(|| tz.to_string())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/intl/time_zone.rs` around lines 85 - 102, Update the timezone validation logic around the split iterator to avoid collecting `tz.split('/')` into a Vec. Consume the iterator directly, track the segment count while validating each segment, and return None unless at least two non-empty segments are processed; preserve the existing character validation and has_alpha behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/perry-runtime/src/intl/time_zone.rs`:
- Around line 17-29: Update the time-zone handling around explicit and tz to
record whether explicit is present before consuming it, then consume explicit
directly when selecting the fallback host time zone instead of cloning the inner
String. Preserve the existing explicit.is_some() behavior in the canonical match
error branch.
- Around line 85-102: Update the timezone validation logic around the split
iterator to avoid collecting `tz.split('/')` into a Vec. Consume the iterator
directly, track the segment count while validating each segment, and return None
unless at least two non-empty segments are processed; preserve the existing
character validation and has_alpha behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f34163dd-24e8-4272-909f-767cf8030c66
📒 Files selected for processing (4)
crates/perry-runtime/src/intl.rscrates/perry-runtime/src/intl/date_collator.rscrates/perry-runtime/src/intl/date_names.rscrates/perry-runtime/src/intl/time_zone.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/perry-runtime/src/intl/date_collator.rs
This was referenced Jul 16, 2026
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.
Summary
Intl.DateTimeFormat().resolvedOptions().timeZonereturned"UTC"regardless of the host machine, and allDateTimeFormat/Date.prototype.toLocaleStringformatting was done in UTC. On any non-UTC host every result was off by the host offset, and Node's host zone (e.g.Europe/Berlin) never appeared.This surfaced in a bundled Next.js app:
next-intlreadsresolvedOptions().timeZone, so the server-rendered hydration payload carriedtimeZone:"UTC"where Node producedtimeZone:"Europe/Berlin".How
date.rs—host_time_zone_name()resolves the IANA id fromTZ(honored like libc/Node) then the/etc/localtimesymlink, falling back to"UTC";zone_offset_seconds()returns a DST-aware offset:0for UTC/GMT, a parsed fixed offset for±HH:mm, and the process zone vialibc::localtimefor the host zone.intl.rs/date_collator.rs— theDateTimeFormatconstructor defaultstimeZoneto the host zone (was UTC),resolvedOptions()reports it, and the two instance format paths shift the epoch into the configured zone before extracting Y/M/D H:M:S.date_proto_thunks.rs—Date.prototype.toLocaleStringrenders the instant in the resolved zone (thetimeZoneoption, else host) instead of UTC.Testing
test_gap_intl_host_timezone.ts(byte-compared to Node on the same host): resolved-zone-is-a-string, resolved matches the global default, UTC formatting, host-zone formatting (host offset applied), fixed+05:00/-03:30offsets, andtoLocaleString({timeZone:'UTC'}). UnderTZ=UTC(typical CI) the host zone is UTC so this is a no-op; the full perry-runtime suite passes on both a UTC and a Europe/Berlin host.Scope / follow-up
A named zone that is not the host zone (e.g.
America/New_Yorkon a Europe/Berlin host) still needs the OS tz database to resolve, and mutating libc's globalTZis unsafe in a threaded runtime, so it currently falls back to UTC. The exact cases — default/host zone, explicit UTC, fixed numeric offsets — cover the common usage; a follow-up can wire the already-vendoredjiff-tzdb(currently gated onTemporal) for arbitrary named zones. The German date pattern (15.01.vs1/15/) is a separate locale-localization gap, not timezone.https://claude.ai/code/session_01NjymZygYh31wM9h3wLnUqv
Summary by CodeRabbit
New Features
Intl.DateTimeFormatnow resolves to the host system time zone (not hardcoded UTC).resolvedOptions().timeZonereflects the effective host zone.Bug Fixes
Intl.DateTimeFormat/Date.prototype.toLocaleStringformatting so non-Temporal inputs apply the selected time zone correctly.Tests