Skip to content
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

Add test case for Object.groupBy / Map.groupBy with strings? #4036

Open
sosukesuzuki opened this issue Apr 1, 2024 · 3 comments · Fixed by #4038
Open

Add test case for Object.groupBy / Map.groupBy with strings? #4036

sosukesuzuki opened this issue Apr 1, 2024 · 3 comments · Fixed by #4038

Comments

@sosukesuzuki
Copy link
Contributor

Object.groupBy and Map.groupBy take an iterable, such as an array, as their first argument. This can be any iterable, so they can also accept a string.

const str = "abcd";
const grouped = Object.groupBy(str, (char) => char < 'c' ? "x" : "y");

However, it was discovered that in some implementations, a TypeError was thrown when the first argument was not of object type, so this was not working1.

What do you think about adding test cases to test262 to cover this?

Footnotes

  1. https://bugs.webkit.org/show_bug.cgi?id=271524

@ljharb
Copy link
Member

ljharb commented Apr 1, 2024

Sounds great. I'd hope basically all implementation bugs end up with test262 regression tests :-)

@sosukesuzuki
Copy link
Contributor Author

Thank you, I will try to submit a PR.

@ptomato
Copy link
Contributor

ptomato commented Apr 4, 2024

Following up from #4038, I think it might help to have additional coverage for ASCII strings (to help debuggability in case an implementation gets single characters right but surrogate pairs wrong) and grapheme clusters (like 👨‍👦 being split into 👨, ZWJ, and 👦.)

@ptomato ptomato reopened this Apr 4, 2024
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 a pull request may close this issue.

3 participants