fix(intl): #5582 — Temporal-aware DateTimeFormat range + timeZoneName part (19 test262 cases)#5770
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds a ChangesIntl.DateTimeFormat Temporal validation and range coercion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
… part (19 test262 cases)
Builds on main's single-value Temporal support (`temporal_to_epoch_ms`) and
extends it to the range methods, the `timeZoneName` part, and the
spec-mandated `TypeError`s the deterministic formatter still missed.
- `formatRange`/`formatRangeToParts` coerced both endpoints through `ToNumber`
(`js_number_coerce`), raising "Cannot convert a Temporal value to a number"
for any Temporal argument. `date_time_range_clip` now routes each endpoint
through the same Temporal-aware `date_arg_to_clipped_ms` as the single-value
path, so a plain Temporal value (PlainDate/PlainDateTime/PlainYearMonth/
PlainMonthDay/PlainTime) decodes to its epoch instant and is formatted
instead of throwing.
- `PartitionDateTimeRangePattern`: the two endpoints must be the same brand —
mixing brands (e.g. PlainDate + PlainTime, or Date + a Temporal value) is a
`TypeError` (`range_type_tag`). This preserves the existing
`fails-on-distinct-temporal-types` / dissimilar-types throws that previously
came for free from `ToNumber`.
- `Temporal.ZonedDateTime` is now rejected with a `TypeError` in
`date_arg_to_clipped_ms` (ECMA-402 HandleDateTimeValue), covering both the
single-value (`format`/`formatToParts`) and same-brand range cases.
- `formatToParts` emits a `{type:"timeZoneName"}` part when the option is set
and the input denotes a real instant (`Date`/number); a Temporal plain value
carries no zone and is suppressed.
test262 intl402/DateTimeFormat: runtime-fail 97 → 78 (pass 126 → 145), no
regressions. Fixes temporal-*-formatting-timezonename (x15),
temporal-zoneddatetime-not-supported (format/formatToParts), and the
temporal-objects-no-time-clip range cases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
proggeramlug
force-pushed
the
fix/5582-dtf-temporal-range
branch
from
ad11553 to
512983e
Compare
proggeramlug
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.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/date_collator.rs`:
- Around line 124-126: The early return in date_collator logic is too broad: it
skips all Temporal values and prevents Temporal.Instant from rendering a
requested timeZoneName. Update the guard in the date collator path so only the
plain Temporal types (PlainDate, PlainTime, PlainDateTime, PlainYearMonth,
PlainMonthDay) return early, while Temporal.Instant continues through the
formatting flow and can include the zone label. Use the existing temporal type
checks around the relevant collator/formatting branch to narrow this condition.
🪄 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: 44d9f752-1ca4-402c-a874-c71f208cda80
📒 Files selected for processing (1)
crates/perry-runtime/src/intl/date_collator.rs
Comment on lines
+124
to
+126
| if crate::temporal::is_temporal_value(value) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== date_collator.rs (focused) ==\n'
nl -ba crates/perry-runtime/src/intl/date_collator.rs | sed -n '100,170p'
printf '\n== temporal/mod.rs (focused) ==\n'
nl -ba crates/perry-runtime/src/temporal/mod.rs | sed -n '1,260p'
printf '\n== helper usages ==\n'
rg -n "fn is_temporal_value|fn temporal_kind|enum TemporalKind|is_temporal_value\(" crates/perry-runtime/src/temporal crates/perry-runtime/src -g '!**/target/**'Repository: PerryTS/perry
Length of output: 224
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== date_collator.rs helper call sites ==\n'
sed -n '1,220p' crates/perry-runtime/src/intl/date_collator.rs | grep -n -C 4 -E 'append_time_zone_name_part|date_time_format_to_parts_value|is_temporal_value|temporal_kind'
printf '\n== temporal module symbols ==\n'
rg -n "enum TemporalKind|is_temporal_value|temporal_kind|PlainDate|PlainTime|PlainDateTime|PlainYearMonth|PlainMonthDay|Instant" crates/perry-runtime/src/temporal -g '!**/target/**'Repository: PerryTS/perry
Length of output: 40351
Don’t suppress timeZoneName for Temporal.Instant. This guard skips every Temporal value, including Temporal.Instant; only the plain Temporal types (PlainDate, PlainTime, PlainDateTime, PlainYearMonth, PlainMonthDay) should return early so an Instant can still render a zone label when requested.
🤖 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/date_collator.rs` around lines 124 - 126, The
early return in date_collator logic is too broad: it skips all Temporal values
and prevents Temporal.Instant from rendering a requested timeZoneName. Update
the guard in the date collator path so only the plain Temporal types (PlainDate,
PlainTime, PlainDateTime, PlainYearMonth, PlainMonthDay) return early, while
Temporal.Instant continues through the formatting flow and can include the zone
label. Use the existing temporal type checks around the relevant
collator/formatting branch to narrow this condition.
proggeramlug
added a commit
that referenced
this pull request
Jun 28, 2026
…matToParts (#5774) Follow-up to #5770 (CodeRabbit review). The `timeZoneName`-part guard in `formatToParts` skipped *every* Temporal value, including `Temporal.Instant`. An `Instant` is anchored to the timeline, so — like a `Date`/number — it must render a zone label when `timeZoneName` is requested; only the *plain* Temporal kinds (PlainDate/PlainTime/PlainDateTime/PlainYearMonth/PlainMonthDay) carry no zone and stay suppressed. (`ZonedDateTime`/`Duration` are rejected upstream by `date_arg_to_clipped_ms` and never reach this path.) Behavior-preserving for the test262 intl402/DateTimeFormat suite (pass 145, runtime-fail 78, unchanged); the in-scope timezonename cases use only plain Temporal types. Co-authored-by: Ralph <ralph@skelpo.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Merged
4 tasks
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.
#5582 —
Intl.DateTimeFormatTemporal range +timeZoneNameBuilds on main's single-value Temporal support (
temporal::temporal_to_epoch_ms) and extends it to the parts that the deterministic formatter still missed: the range methods, thetimeZoneNamepart, and the spec-mandatedTypeErrors.Problem
formatRange/formatRangeToPartscoerced both endpoints throughToNumber, so any Temporal argument threwTypeError: Cannot convert a Temporal value to a number.formatToPartsnever emitted atimeZoneNamepart, so the cases that read.find(p => p.type === 'timeZoneName').valuehitundefined.value.Intl.DateTimeFormat.prototype.format(zonedDateTime)formatted instead of throwing the requiredTypeError.Changes (single file:
crates/perry-runtime/src/intl/date_collator.rs)date_time_range_cliproutes each endpoint through the same Temporal-awaredate_arg_to_clipped_msas the single-value path; a plain Temporal value decodes to its epoch instant and is formatted instead of throwing.range_type_tag—PartitionDateTimeRangePattern: the two endpoints must be the same brand; mixing brands (e.g.PlainDate+PlainTime, orDate+ a Temporal value) is aTypeError. This preserves thefails-on-distinct-temporal-types/ dissimilar-types throws that previously came for free fromToNumber.date_arg_to_clipped_msrejectsTemporal.ZonedDateTimewith aTypeError(ECMA-402 HandleDateTimeValue), covering the single-value and same-brand range cases.formatToPartsemits a{type:"timeZoneName"}part when the option is set and the input denotes a real instant (Date/number); a Temporal plain value carries no zone and is suppressed.Reuses main's
temporal_to_epoch_ms— no duplicate epoch logic.Validation
scripts/test262_subset.py --dir intl402/DateTimeFormat(pinned corpus), measured against currentmain:0 regressions. 19 cases flip green:
format/formatRange/formatRangeToParts×temporal-{plaindate,plaindatetime,plainmonthday,plaintime,plainyearmonth}-formatting-timezonename(15)format/formatToPartstemporal-zoneddatetime-not-supported(2)formatRange/formatRangeToPartstemporal-objects-no-time-clip(2)cargo test -p perry-runtime(date module) green.Scope
Perry ships no CLDR / Temporal calendar engine (out of scope per
CLAUDE.md), so the renderedtimeZoneNamelabel and extreme-date calendar output stay best-effort; the in-scope cases observe only part presence and (non-)throwing behavior. Remainingintl402/DateTimeFormatfailures (calendar systems, dayPeriod, fractionalSecondDigits, …) still need a real formatting engine and stay tracked in #5582.🤖 Generated with Claude Code
Summary by CodeRabbit
Intl.DateTimeFormatnow rejectsTemporal.ZonedDateTimeinputs with a clearTypeErrorinstead of treating them like epoch-based dates.formatToParts()now includes atimeZoneNamepart when requested for real instants, improving output consistency.