Skip to content

Refactor methods related to localized strings in Date #5411

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.

Sign up for GitHub

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

Open
Vellumic wants to merge 9 commits into
base: main
Choose a base branch
from
+66 −151
Open

Refactor methods related to localized strings in Date #5411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6106701
Extract necessary imports to a single block
Vellumic Jun 17, 2026
bb1796d
Rename `dateObject` variable to `x` as in spec
Vellumic Jun 17, 2026
870c115
Introduce shared implementation
Vellumic Jun 20, 2026
00a536e
Remove `format_date_time_locale`
Vellumic Jun 20, 2026
bd05f96
Update `format_timestamp_with_dtf`
Vellumic Jun 20, 2026
325a176
Gate `to_locale_string_with` behind `intl` flag
Vellumic Jun 21, 2026
7065648
Remove unnecessary `allow` attributes
Vellumic Jun 21, 2026
a8b8216
Improve some doc comments
Vellumic Jun 21, 2026
4766f5f
fix(ci): clear stale runner cache
Vellumic Jun 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 45 additions & 96 deletions core/engine/src/builtins/date/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ use crate::{
use boa_gc::{Finalize, Trace};
use boa_macros::js_str;

#[cfg(feature = "intl")]
use crate::builtins::intl::date_time_format::{
FormatDefaults, FormatType, create_date_time_format, format_timestamp_with_dtf,
};

pub(crate) mod utils;

#[cfg(test)]
Expand Down Expand Up @@ -1636,6 +1641,40 @@ impl Date {
func.call(this, &[], context)
}

/// Shared implementation of `Date.prototype.toLocaleString`,
/// `Date.prototype.toLocaleDateString`, and `Date.prototype.toLocaleTimeString`
/// methods with the corresponding formatting params
#[cfg(feature = "intl")]
#[inline]
fn to_locale_string_with(
this: &JsValue,
args: &[JsValue],
required: FormatType,
defaults: FormatDefaults,
context: &mut Context,
) -> JsResult<JsValue> {
// 1. Let dateObject be the this value.
// 2. Perform ? RequireInternalSlot(dateObject, [[DateValue]]).
// 3. Let x be dateObject.[[DateValue]].
let x = this
.as_object()
.and_then(|obj| obj.downcast_ref::<Date>().as_deref().copied())
.ok_or_else(|| JsNativeError::typ().with_message("'this' is not a Date"))?
.0;

// 4. If x is NaN, return "Invalid Date".
if x.is_nan() {
return Ok(JsValue::new(js_string!("Invalid Date")));
}

// 5. Let dateFormat be ? CreateDateTimeFormat(%Intl.DateTimeFormat%, locales, options, required, defaults).
// 6. Return ! FormatDateTime(dateFormat, x).
let locales = args.get_or_undefined(0);
let options = args.get_or_undefined(1);
let dtf = create_date_time_format(locales, options, required, defaults, context)?;
format_timestamp_with_dtf(&dtf, x, context)
}

/// [`Date.prototype.toLocaleDateString()`][spec].
///
/// The `toLocaleDateString()` method returns the date portion of the given Date instance according
Expand All @@ -1646,48 +1685,18 @@ impl Date {
///
/// [spec]: https://tc39.es/ecma402/#sup-date.prototype.tolocaledatestring
/// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toLocaleDateString
#[allow(
unused_variables,
reason = "`args` and `context` are used when the `intl` feature is enabled"
)]
pub(crate) fn to_locale_date_string(
this: &JsValue,
args: &[JsValue],
context: &mut Context,
) -> JsResult<JsValue> {
#[cfg(feature = "intl")]
{
use crate::builtins::intl::date_time_format::{
FormatDefaults, FormatType, format_date_time_locale,
};
// 1. Let dateObject be the this value.
// 2. Perform ? RequireInternalSlot(dateObject, [[DateValue]]).
// 3. Let x be dateObject.[[DateValue]].
let t = this
.as_object()
.and_then(|obj| obj.downcast_ref::<Date>().as_deref().copied())
.ok_or_else(|| JsNativeError::typ().with_message("'this' is not a Date"))?
.0;
// 4. If x is NaN, return "Invalid Date".
if t.is_nan() {
return Ok(JsValue::new(js_string!("Invalid Date")));
}
// 5. Let dateFormat be ? CreateDateTimeFormat(%Intl.DateTimeFormat%, locales, options, date, date).
// 6. Return ! FormatDateTime(dateFormat, x).
let locales = args.get_or_undefined(0);
let options = args.get_or_undefined(1);
format_date_time_locale(
locales,
options,
FormatType::Date,
FormatDefaults::Date,
t,
context,
)
Self::to_locale_string_with(this, args, FormatType::Date, FormatDefaults::Date, context)
}
#[cfg(not(feature = "intl"))]
{
Self::to_string(this, &[], context)
Self::to_string(this, args, context)
}
}

Expand All @@ -1700,48 +1709,18 @@ impl Date {
///
/// [spec]: https://tc39.es/ecma402/#sup-date.prototype.tolocalestring
/// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toLocaleString
#[allow(
unused_variables,
reason = "`args` and `context` are used when the `intl` feature is enabled"
)]
pub(crate) fn to_locale_string(
this: &JsValue,
args: &[JsValue],
context: &mut Context,
) -> JsResult<JsValue> {
#[cfg(feature = "intl")]
{
use crate::builtins::intl::date_time_format::{
FormatDefaults, FormatType, format_date_time_locale,
};
// 1. Let dateObject be the this value.
// 2. Perform ? RequireInternalSlot(dateObject, [[DateValue]]).
// 3. Let x be dateObject.[[DateValue]].
let t = this
.as_object()
.and_then(|obj| obj.downcast_ref::<Date>().as_deref().copied())
.ok_or_else(|| JsNativeError::typ().with_message("'this' is not a Date"))?
.0;
// 4. If x is NaN, return "Invalid Date".
if t.is_nan() {
return Ok(JsValue::new(js_string!("Invalid Date")));
}
// 5. Let dateFormat be ? CreateDateTimeFormat(%Intl.DateTimeFormat%, locales, options, any, all).
// 6. Return ! FormatDateTime(dateFormat, x).
let locales = args.get_or_undefined(0);
let options = args.get_or_undefined(1);
format_date_time_locale(
locales,
options,
FormatType::Any,
FormatDefaults::All,
t,
context,
)
Self::to_locale_string_with(this, args, FormatType::Any, FormatDefaults::All, context)
}
#[cfg(not(feature = "intl"))]
{
Self::to_string(this, &[], context)
Self::to_string(this, args, context)
}
}

Expand All @@ -1755,48 +1734,18 @@ impl Date {
///
/// [spec]: https://tc39.es/ecma402/#sup-date.prototype.tolocaletimestring
/// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toLocaleTimeString
#[allow(
unused_variables,
reason = "`args` and `context` are used when the `intl` feature is enabled"
)]
pub(crate) fn to_locale_time_string(
this: &JsValue,
args: &[JsValue],
context: &mut Context,
) -> JsResult<JsValue> {
#[cfg(feature = "intl")]
{
use crate::builtins::intl::date_time_format::{
FormatDefaults, FormatType, format_date_time_locale,
};
// 1. Let dateObject be the this value.
// 2. Perform ? RequireInternalSlot(dateObject, [[DateValue]]).
// 3. Let x be dateObject.[[DateValue]].
let t = this
.as_object()
.and_then(|obj| obj.downcast_ref::<Date>().as_deref().copied())
.ok_or_else(|| JsNativeError::typ().with_message("'this' is not a Date"))?
.0;
// 4. If x is NaN, return "Invalid Date".
if t.is_nan() {
return Ok(JsValue::new(js_string!("Invalid Date")));
}
// 5. Let timeFormat be ? CreateDateTimeFormat(%Intl.DateTimeFormat%, locales, options, time, time).
// 6. Return ! FormatDateTime(timeFormat, x).
let locales = args.get_or_undefined(0);
let options = args.get_or_undefined(1);
format_date_time_locale(
locales,
options,
FormatType::Time,
FormatDefaults::Time,
t,
context,
)
Self::to_locale_string_with(this, args, FormatType::Time, FormatDefaults::Time, context)
}
#[cfg(not(feature = "intl"))]
{
Self::to_string(this, &[], context)
Self::to_string(this, args, context)
}
}

Expand Down
76 changes: 21 additions & 55 deletions core/engine/src/builtins/intl/date_time_format/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,13 +278,7 @@ impl DateTimeFormat {
};

// 5. Return ? FormatDateTime(dtf, x).
// A.O 11.5.6 PartitionDateTimePattern: 1. TimeClip(x). 2. If NaN throw. Then ToLocalTime and format.
let x = time_clip(x);
if x.is_nan() {
return Err(js_error!(RangeError: "formatted date cannot be NaN"));
}
let result = format_timestamp_with_dtf(dtf.borrow().data(), x, context)?;
Ok(JsValue::from(result))
format_timestamp_with_dtf(dtf.borrow().data(), x, context)
},
dtf_clone,
),
Expand Down Expand Up @@ -826,39 +820,39 @@ pub(crate) fn create_date_time_format(

/// Formats a timestamp (epoch milliseconds) using the given [`DateTimeFormat`] internals.
///
/// This is the shared implementation used by:
/// - the bound `format` function created in `get_format`, and
/// - [`format_date_time_locale`] used by `Date.prototype.toLocaleString` (and friends).
///
/// It corresponds to the *post*-`TimeClip` portion of
/// [`FormatDateTime(dtf, x)`](https://tc39.es/ecma402/#sec-formatdatetime),
/// and the `ToLocalTime` / `PartitionDateTimePattern` logic from
/// It corresponds the `ToLocalTime` / `PartitionDateTimePattern` logic from
/// [11.5.6](https://tc39.es/ecma402/#sec-partitiondatetimepattern) and
/// [11.5.12](https://tc39.es/ecma402/#sec-tolocaltime).
///
/// Callers must have already applied `TimeClip` and `NaN` check
/// (`FormatDateTime` steps 1–2). This helper implements:
/// This helper implements:
///
/// 11.5.6 `PartitionDateTimePattern` ( dtf, x )
/// 1. Let x be TimeClip(x). (Done by caller)
/// 2. If x is `NaN`, throw a `RangeError` exception. (Done by caller)
/// 1. Let x be TimeClip(x).
/// 2. If x is `NaN`, throw a `RangeError` exception.
/// 3. Let epochNanoseconds be ℤ(ℝ(x) × 10^6).
/// 4. Let timeZone be dtf.[[`TimeZone`]].
/// 5. Let offsetNs be GetOffsetNanosecondsFor(timeZone, epochNanoseconds).
/// 6. Let tz be 𝔽(ℝ(x) + ℝ(offsetNs) / 10^6).
///
/// Then calls `ToLocalTime::from_local_epoch_milliseconds` to obtain calendar fields,
/// and formats the resulting `ZonedDateTime` with ICU4X.
fn format_timestamp_with_dtf(
pub(crate) fn format_timestamp_with_dtf(
dtf: &DateTimeFormat,
timestamp: f64,
context: &mut Context,
) -> JsResult<JsString> {
) -> JsResult<JsValue> {
// PartitionDateTimePattern ( dtf, x ) step 1:
// 1. Let x be TimeClip(x).
let x = time_clip(timestamp);

// PartitionDateTimePattern ( dtf, x ) step 2:
// 2. If x is `NaN`, throw a `RangeError` exception.
if x.is_nan() {
return Err(js_error!(RangeError: "formatted date cannot be NaN"));
}
// PartitionDateTimePattern ( dtf, x ) step 3:
// Let epochNanoseconds be ℤ(ℝ(x) × 10^6).
//
// NOTE: `timestamp` is already `TimeClip`'d by the caller and represents *UTC epoch milliseconds*.
let epoch_ns = timestamp as i128 * 1_000_000;
let epoch_ns = x as i128 * 1_000_000;

// PartitionDateTimePattern ( dtf, x ) step 4:
// Let timeZone be dtf.[[`TimeZone`]].
Expand All @@ -884,7 +878,7 @@ fn format_timestamp_with_dtf(

// PartitionDateTimePattern ( dtf, x ) step 6:
// Let tz be 𝔽(ℝ(x) + ℝ(offsetNs) / 10^6).
let tz = timestamp + f64::from(time_zone_offset_seconds * 1_000);
let tz = x + f64::from(time_zone_offset_seconds * 1_000);
let fields = ToLocalTime::from_local_epoch_milliseconds(tz)?;
let dt = fields.to_formattable_datetime()?;
let tz_info = time_zone.to_time_zone_info();
Expand All @@ -895,7 +889,7 @@ fn format_timestamp_with_dtf(
zone: tz_info_at_time,
};
let result = dtf.formatter.format(&zdt).to_string();
Ok(JsString::from(result))
Ok(JsString::from(result).into())
}

fn date_time_style_format(
Expand Down Expand Up @@ -940,22 +934,19 @@ fn best_fit_date_time_format(format_options: &FormatOptions) -> JsResult<Composi
.map_err(|e| JsNativeError::range().with_message(e.to_string()).into())
}

/// Represents the `required` and `defaults` arguments in the abstract operation
/// `toDateTimeOptions`.
/// Identifies a specific category of date-time components.
#[derive(Debug, Clone, Copy, PartialEq)]
pub(crate) enum FormatType {
Date,
Time,
Any,
}

/// Indicates which default fields should be applied when `ToDateTimeOptions`
/// determines defaults are needed. `All` applies both date and time defaults.
/// Identifies which default values to use when none are specified.
#[derive(Debug, Clone, Copy, PartialEq)]
pub(crate) enum FormatDefaults {
Date,
Time,
/// Apply both date and time defaults (e.g. for `toLocaleString`).
All,
}

Expand Down Expand Up @@ -1010,28 +1001,3 @@ fn unwrap_date_time_format(
.with_message("object was not an initialized `Intl.DateTimeFormat` object")
.into())
}

/// Shared helper used by Date.prototype.toLocaleString,
/// Date.prototype.toLocaleDateString, and Date.prototype.toLocaleTimeString.
/// Applies `ToDateTimeOptions` defaults, calls [`create_date_time_format`], and formats
/// the timestamp via [`format_timestamp_with_dtf`] without allocating a JS object.
#[allow(clippy::too_many_arguments)]
pub(crate) fn format_date_time_locale(
locales: &JsValue,
options: &JsValue,
format_type: FormatType,
defaults: FormatDefaults,
timestamp: f64,
context: &mut Context,
) -> JsResult<JsValue> {
let options = coerce_options_to_object(options, context)?;
let options_value = options.into();
let dtf = create_date_time_format(locales, &options_value, format_type, defaults, context)?;
// FormatDateTime steps 1–2: TimeClip and NaN check (format_timestamp_with_dtf does ToLocalTime + format only).
let x = time_clip(timestamp);
if x.is_nan() {
return Err(js_error!(RangeError: "formatted date cannot be NaN"));
}
let result = format_timestamp_with_dtf(&dtf, x, context)?;
Ok(JsValue::from(result))
}
Loading