typedarray: use locale-sensitive separator in %TypedArray%.prototype.toLocaleString by mrhapile · Pull Request #5089 · boa-dev/boa

mrhapile · 2026-03-15T03:07:07Z

Summary

Update %TypedArray%.prototype.toLocaleString to use a locale-sensitive separator when the intl feature is enabled, aligning its behavior with Array.prototype.toLocaleString.

ref - #5081

Previously, the TypedArray implementation always used the hardcoded separator ", ". When the intl feature is enabled, Array.prototype.toLocaleString already derives the separator using ICU's ListFormatter. This PR updates the TypedArray implementation to use the same locale-aware separator logic.

When the intl feature is disabled, the implementation correctly falls back to ", ".

Changes

Use ICU ListFormatter via the intl_provider to compute a locale-sensitive separator when the intl feature is enabled.
Fall back to the default separator ", " when intl is disabled.
Hoist extraction of locales and options arguments outside the iteration loop to avoid repeated lookups.
Remove the unused utf16 import from the module.

Specification

This aligns the implementation with:

ECMAScript §23.2.3.31
%TypedArray%.prototype.toLocaleString

which specifies that the algorithm should behave the same as:

ECMAScript §23.1.3.32
Array.prototype.toLocaleString

except that TypedArrayLength is used instead of LengthOfArrayLike.

Verification

The following checks were performed:

cargo fmt
cargo clippy
cargo test
Test262 tests for TypedArray locale string conversion:

All existing tests pass and no regressions were introduced.

Notes

The behavior now matches the locale-sensitive list separator behavior used by Array.prototype.toLocaleString when intl support is enabled.

Signed-off-by: mrhapile <allinonegaming3456@gmail.com>

Copilot

Pull request overview

Updates %TypedArray%.prototype.toLocaleString to use the same locale-sensitive list separator logic as Array.prototype.toLocaleString when the intl feature is enabled, keeping behavior aligned with the spec and existing array behavior.

Changes:

Compute a locale-aware separator via ICU ListFormatter behind the intl feature flag.
Fall back to the default ", " separator when intl is disabled.
Hoist locales/options extraction outside the iteration loop and remove the unused utf16 import.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

core/engine/src/builtins/typed_array/builtin.rs

github-actions · 2026-03-15T03:16:29Z

Test262 conformance changes

Test result	main count	PR count	difference
Total	52,963	52,963	0
Passed	50,070	49,935	-135
Ignored	2,072	2,207	+135
Failed	821	821	0
Panics	0	0	0
Conformance	94.54%	94.28%	-0.25%

Tested main commit: baaadfeeaf4a64f34e82cbbb9fee3ed4013495c0
Tested PR commit: b607b69648cb1accf98b6118a27f0715d5b7abfe
Compare commits: baaadfe...b607b69

Preallocate the result buffer to reduce reallocations when concatenating separator and element strings, similar to the Array.prototype.toLocaleString implementation. Signed-off-by: mrhapile <allinonegaming3456@gmail.com>

Copilot

Pull request overview

Updates %TypedArray%.prototype.toLocaleString to use the same locale-sensitive list separator behavior as Array.prototype.toLocaleString when the intl feature is enabled, while keeping the ", " fallback when intl is disabled.

Changes:

Use ICU ListFormatter (via intl_provider) to compute a locale-sensitive separator under cfg(feature = "intl").
Hoist locales/options extraction outside the element iteration and remove the unused utf16 import.
Adjust separator handling to work with JsString (iterating code units) and pre-allocate in join.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

core/engine/src/builtins/typed_array/builtin.rs

Reserve capacity for the result buffer in %TypedArray%.prototype.toLocaleString to reduce repeated reallocations when concatenating separators and element strings. This mirrors the approach used in Array.prototype.toLocaleString. Signed-off-by: mrhapile <allinonegaming3456@gmail.com>

codecov · 2026-03-15T03:49:56Z

Codecov Report

❌ Patch coverage is 51.72414% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.13%. Comparing base (6ddc2b4) to head (b607b69).
⚠️ Report is 865 commits behind head on main.

Files with missing lines	Patch %	Lines
core/engine/src/builtins/typed_array/builtin.rs	7.14%	13 Missing ⚠️
core/engine/src/builtins/intl/number_format/mod.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #5089       +/-   ##
===========================================
+ Coverage   47.24%   59.13%   +11.88%     
===========================================
  Files         476      563       +87     
  Lines       46892    62691    +15799     
===========================================
+ Hits        22154    37071    +14917     
- Misses      24738    25620      +882

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jedel1043 · 2026-03-15T04:24:28Z

core/engine/src/builtins/typed_array/builtin.rs

+                use crate::builtins::intl::locale::default_locale;
+                use icu_list::{
+                    ListFormatter, ListFormatterPreferences, options::ListFormatterOptions,
+                };
+
+                let locale = default_locale(context.intl_provider().locale_canonicalizer()?);
+                let preferences = ListFormatterPreferences::from(&locale);
+                let formatter = ListFormatter::try_new_unit_with_buffer_provider(
+                    context.intl_provider().erased_provider(),
+                    preferences,
+                    ListFormatterOptions::default(),
+                )
+                .map_err(|e| JsNativeError::typ().with_message(e.to_string()))?;
+
+                // Ask ICU for the list pattern literal by formatting two empty elements.
+                // For many locales this yields ", ", but it may differ.
+                js_string!(
+                    formatter.format_to_string(std::iter::once("").chain(std::iter::once("")))
+                )


In this case we kinda don't want to follow the implementation of Array, because it would create one NumberFormat per element, which is extremely inefficient

Instead, create a NumberFormat and a ListFormat then do a format of all numbers followed by a format with ListFormat

Signed-off-by: mrhapile <allinonegaming3456@gmail.com>

jedel1043 · 2026-03-15T06:27:01Z

You didn't address my comment

mrhapile · 2026-03-15T08:05:18Z

@jedel1043 I did attempt the shared NumberFormat + ListFormatter approach, but it caused several Test262 failures (particularly for BigInt TypedArrays). To keep the implementation spec-correct for now, I temporarily removed the NumberFormat change and kept the ListFormatter.

mrhapile · 2026-03-15T08:06:09Z

I'm currently investigating how to correctly integrate a shared NumberFormat while preserving full Test262 conformance , working on it will update sooon!!!

Signed-off-by: mrhapile <allinonegaming3456@gmail.com>

mrhapile · 2026-03-15T08:35:22Z

right now
Results (ECMAScript Next):
Total tests: 39
Passed tests: 19
Ignored tests: 0
Failed tests: 20

Signed-off-by: mrhapile <allinonegaming3456@gmail.com>

mrhapile · 2026-03-15T08:38:15Z

Results (ECMAScript Next):
Total tests: 39
Passed tests: 29
Ignored tests: 0
Failed tests: 10 (0 panics)
Conformance: 74.36%

…g shared NumberFormat optimization Signed-off-by: mrhapile <allinonegaming3456@gmail.com>

…d ListFormatter Avoid creating a NumberFormat per element by reusing a single NumberFormat and ListFormatter instance for the entire typed array. Falls back to the spec path when: - Number.prototype.toLocaleString is overridden - elements contain NaN or Infinity - the typed array uses BigInt Signed-off-by: mrhapile <allinonegaming3456@gmail.com>

jedel1043 · 2026-03-15T12:23:01Z

I see what's happening. The test262 suite specifically tests that you call Number.prototype.toLocaleString for all elements... that's kinda bad

Okay, I guess we'll have to return to the old approach of using the separator and manually calling toLocaleString. I'll think of a way to optimise this in the future.

mrhapile · 2026-03-15T12:25:22Z

@jedel1043 nahh now I have updated it I was just confirming and testing things before making this pr ready for review

mrhapile · 2026-03-15T12:25:46Z

jedel1043 · 2026-03-15T12:34:26Z

I did see that, but I still put my comment becusse if the spec requires you to call Number.prototype.toLocaleString, then I don't think we should do special handling in TypedArray.prototype.toLocaleString. We should instead optimize Number.prototype.toLocaleString directly

mrhapile · 2026-03-15T12:42:52Z

should I revert it back to the simple approach (the first one I proposed) or wait for further instructions??

jedel1043 · 2026-03-15T13:45:46Z

yep, revert back

Signed-off-by: mrhapile <allinonegaming3456@gmail.com>

jedel1043 · 2026-03-16T20:35:25Z

core/engine/src/context/intrinsics.rs

+    /// [spec]: https://tc39.es/ecma262/#sec-number.prototype.tolocalestring
+    #[inline]
+    #[must_use]
+    pub fn number_prototype_to_locale_string(&self) -> JsFunction {


We don't need this anymore since the changes were reverted

typedarray: use locale-sensitive separator in toLocaleString

a366a6b

Signed-off-by: mrhapile <allinonegaming3456@gmail.com>

mrhapile requested a review from a team as a code owner March 15, 2026 03:07

Copilot AI review requested due to automatic review settings March 15, 2026 03:07

github-actions bot added C-Builtins PRs and Issues related to builtins/intrinsics Waiting On Review Waiting on reviews from the maintainers labels Mar 15, 2026

github-actions bot added this to the v1.0.0 milestone Mar 15, 2026

Copilot started reviewing on behalf of mrhapile March 15, 2026 03:07 View session

Copilot AI reviewed Mar 15, 2026

View reviewed changes

core/engine/src/builtins/typed_array/builtin.rs Show resolved Hide resolved

typedarray: preallocate buffer capacity in toLocaleString

fd22edc

Preallocate the result buffer to reduce reallocations when concatenating separator and element strings, similar to the Array.prototype.toLocaleString implementation. Signed-off-by: mrhapile <allinonegaming3456@gmail.com>

mrhapile requested a review from Copilot March 15, 2026 03:28

Copilot started reviewing on behalf of mrhapile March 15, 2026 03:29 View session

Copilot AI reviewed Mar 15, 2026

View reviewed changes

core/engine/src/builtins/typed_array/builtin.rs Outdated Show resolved Hide resolved

jedel1043 requested changes Mar 15, 2026

View reviewed changes

typedarray: fix toLocaleString spec compliance

264f79c

Signed-off-by: mrhapile <allinonegaming3456@gmail.com>

mrhapile requested a review from nekevss as a code owner March 15, 2026 05:47

github-actions bot added the C-Intl Changes related to the `Intl` implementation label Mar 15, 2026

mrhapile requested a review from jedel1043 March 15, 2026 05:48

implemented number formatter but failing test case

6c96bf7

Signed-off-by: mrhapile <allinonegaming3456@gmail.com>

mrhapile marked this pull request as draft March 15, 2026 08:34

optimizing

5e5b7d7

Signed-off-by: mrhapile <allinonegaming3456@gmail.com>

typedarray: preserve observable toLocaleString semantics while keepin…

b78e36f

…g shared NumberFormat optimization Signed-off-by: mrhapile <allinonegaming3456@gmail.com>

mrhapile force-pushed the fix/typedarray-tolocalestring-intl-separator branch from 3b43a7c to b78e36f Compare March 15, 2026 11:26

mrhapile marked this pull request as ready for review March 15, 2026 12:31

revert(typedarray): restore spec-compliant toLocaleString implementation

b607b69

Signed-off-by: mrhapile <allinonegaming3456@gmail.com>

jedel1043 reviewed Mar 16, 2026

View reviewed changes

jedel1043 added Waiting On Author Waiting on PR changes from the author and removed Waiting On Review Waiting on reviews from the maintainers labels Mar 16, 2026

Uh oh!

Conversation

mrhapile commented Mar 15, 2026

Summary

ref - #5081

Changes

Specification

Verification

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

github-actions bot commented Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test262 conformance changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

codecov bot commented Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jedel1043 Mar 15, 2026

Choose a reason for hiding this comment

Uh oh!

jedel1043 commented Mar 15, 2026

Uh oh!

mrhapile commented Mar 15, 2026

Uh oh!

mrhapile commented Mar 15, 2026

Uh oh!

mrhapile commented Mar 15, 2026

Uh oh!

mrhapile commented Mar 15, 2026

Uh oh!

jedel1043 commented Mar 15, 2026

Uh oh!

mrhapile commented Mar 15, 2026

Uh oh!

mrhapile commented Mar 15, 2026

Uh oh!

jedel1043 commented Mar 15, 2026

Uh oh!

mrhapile commented Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jedel1043 commented Mar 15, 2026

Uh oh!

jedel1043 Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Mar 15, 2026 •

edited

Loading

codecov bot commented Mar 15, 2026 •

edited

Loading

mrhapile commented Mar 15, 2026 •

edited

Loading