-
Notifications
You must be signed in to change notification settings - Fork 510
fix(intersectionBy): remove duplicates when mapper produces same values #1528
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the intersectionBy function to support finding the intersection of three or more arrays using a mapping function, while maintaining backward compatibility with the existing two-array signature. The enhancement allows developers to find common elements across multiple arrays based on a transformed value, which is useful for complex data filtering scenarios.
Key Changes:
- Modified the function signature to accept a variable number of arrays using rest parameters, with the mapper function as the last argument
- Updated implementation to iteratively filter elements across all provided arrays
- Added comprehensive test coverage for 3+ array scenarios and edge cases
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/array/intersectionBy.ts | Enhanced function to support multiple arrays with iterative filtering logic and updated JSDoc documentation |
| src/array/intersectionBy.spec.ts | Added test cases for 3+ arrays, 4+ arrays, and empty intersection scenarios |
| docs/reference/array/intersectionBy.md | Updated English documentation with multi-array examples and parameter descriptions |
| docs/ko/reference/array/intersectionBy.md | Updated Korean documentation with multi-array examples and parameter descriptions |
| docs/ja/reference/array/intersectionBy.md | Updated Japanese documentation with multi-array examples and parameter descriptions |
| docs/zh_hans/reference/array/intersectionBy.md | Updated Chinese documentation with multi-array examples and parameter descriptions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1528 +/- ##
=======================================
Coverage 99.97% 99.97%
=======================================
Files 474 474
Lines 4492 4493 +1
Branches 1313 1313
=======================================
+ Hits 4491 4492 +1
Misses 1 1 🚀 New features to boost your workflow:
|
|
I'm sorry, but as we mentioned in the original comment, we would like to keep the original interface where we only accept two arrays and a mapper function. (We would not like to accept multiple arrays here since it violates our simplicity principle.) However, we should fix a bug where calls like |
|
Sorry, I seem to have misunderstood your comment. I'll try to fix the bug you mentioned by submitting a PR. |
|
I've fixed the bug and re-created the PR, sorry for the confusion. |
Summary
Fixes: #956
Fixed a bug in
intersectionBywhere it returned duplicate elements from the first array when the mapper function produced identical values for multiple elements. Now correctly returns only the first occurrence of each mapped value.For example,
intersectionBy([2.1, 2.2], [2.3, 3.4], Math.floor)now correctly returns[2.1]instead of[2.1, 2.2].Changes
intersectionByto useuniqByon the first array before filteringComment