test(proxySet): add test for 'intersection' with Set-like object using 'forEach' fallback#1211
test(proxySet): add test for 'intersection' with Set-like object using 'forEach' fallback#1211sukvvon wants to merge 1 commit intopmndrs:mainfrom
Conversation
…g 'forEach' fallback
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
commit: |
|
|
||
| it('.intersection with Set-like object that has forEach but is not iterable', () => { | ||
| const odds = proxySet([1, 3, 5, 7, 9]) | ||
| const setLike = { |
There was a problem hiding this comment.
What's the reason for this instead of native Set?
There was a problem hiding this comment.
@dai-shi Native Set and proxySet have Symbol.iterator, so they pass the isIterable check and skip the hasForEach path (line 92, 96-99). A Set-like object without Symbol.iterator is needed to cover the forEach fallback in asIterable.
There was a problem hiding this comment.
I see. I'll defer the review to copilot and @overthemike .
There was a problem hiding this comment.
Pull request overview
Adds regression coverage for proxySet().intersection when given a non-iterable Set-like object that supports forEach, ensuring the hasForEach detection and forEach fallback path are exercised.
Changes:
- Add a new test case for
.intersectionusing a non-iterable Set-like object withforEach.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return [1, 4, 9].values() | ||
| }, | ||
| forEach(cb: (v: number) => void) { | ||
| ;[1, 4, 9].forEach(cb) |
There was a problem hiding this comment.
The leading semicolon before the array literal is unnecessary here (the statement starts at the beginning of the block) and is the only occurrence of this pattern in the repo. Consider removing it to keep test style consistent and avoid confusing readers about ASI hazards that don’t apply in this context.
| ;[1, 4, 9].forEach(cb) | |
| [1, 4, 9].forEach(cb) |
There was a problem hiding this comment.
Copilot doesn't know prettier. 😢
Summary
intersectionwith a Set-like object that hasforEachbut is not iterablehasForEachfunction (line 92) andforEachfallback path (lines 96-99) insrc/vanilla/utils/proxySet.tsCheck List
pnpm run fixfor formatting and linting code and docs