Skip to content

feat(ecmascript): Array and TypedArray prototype toLocaleString #745

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.

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

Conversation

yossydev
Copy link
Contributor

@yossydev yossydev commented Jun 1, 2025

The toLocaleString method should ideally conform to ECMA-402, but according to the specification, if ECMA-402 is not supported, it’s acceptable to implement it as specified in ECMA-262:

If an ECMAScript implementation does not include the ECMA-402 API, the following specification of this method is used.

In this PR, the priority was to pass test262, so I implemented toLocaleString for Array and TypedArray.

@yossydev yossydev marked this pull request as ready for review June 1, 2025 12:57
aapoalas
aapoalas previously approved these changes Jun 6, 2025
Copy link
Member

@aapoalas aapoalas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of GC related nitpicks but LGTM, nice! <3

@yossydev yossydev force-pushed the feat/array-and-typedarray-prototype-to-locale-string branch from 00404f0 to 1fa0de5 Compare June 11, 2025 10:07
@yossydev
Copy link
Contributor Author

@aapoalas
Sorry for the delay! Thanks for the review!
I’ve made all the necessary corrections! 1fa0de5

@yossydev yossydev requested a review from aapoalas June 11, 2025 10:08
aapoalas
aapoalas previously approved these changes Jun 11, 2025
Copy link
Member

@aapoalas aapoalas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, though a few nitpicks related to binding still. They don't matter really, the code is fine from GC point of view, but it's again from a "leave a good example" kind of view better to fix them.

@yossydev
Copy link
Contributor Author

@aapoalas
Thank you! I’m making corrections! 🙇‍♂️ ebce347

@aapoalas aapoalas force-pushed the feat/array-and-typedarray-prototype-to-locale-string branch from cf7a758 to aaf5823 Compare June 16, 2025 11:40
@aapoalas aapoalas merged commit 5ba850f into trynova:main Jun 16, 2025
4 checks passed
Copy link
Member

@aapoalas aapoalas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you and great work! <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants