Add iterator implementation to Collections #680
Conversation
🦋 Changeset detectedLatest commit: eca1a75 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@codingjoe is attempting to deploy a commit to the Meta Open Source Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
This pull request adds ES6 iterator support to the Collection class, enabling the use of for-of loops when working with Collections. The implementation uses a generator function that delegates to the underlying __paths array.
Key changes:
- Implements
Symbol.iteratoras a generator method in the Collection class - Adds test coverage for the new iterator functionality
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Collection.js | Adds iterator implementation using generator delegation and JSDoc annotations |
| src/tests/Collection-test.js | Adds test case verifying iterator works correctly with for-of loops |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
codingjoe
force-pushed
the
issues/679/collection-iterator
branch
3 times, most recently
from
24499e4 to
c06da00
Compare
ES6 iterators support the new for..of syntax. Since a Collection wraps an Array, we can lean on ECMAScripts new iterator delegation. Resolve facebook#679
codingjoe
force-pushed
the
issues/679/collection-iterator
branch
from
c06da00 to
eca1a75
Compare
| *[Symbol.iterator]() { | ||
| yield* this.__paths; | ||
| } | ||
|
|
There was a problem hiding this comment.
I wonder if we should have a nodes iterator too, like the existing nodes method. It could be called iterateNodes or something similar?
There was a problem hiding this comment.
I looked into the method. Since it returns a native Array, it is already iterable.
e.g., the following code will already work:
for (const node of colletion.nodes()) { console.debug(node) }If you like, I can add a regression test for this behavior, but maybe it's a little extra.
|
Looks pretty good to me! Just left a small comment inline. |
2 participants
ES6 iterators support the new for..of syntax. Since a
Collectionwraps anArray, we can lean on ECMAScript's new iterator delegation.Resolve #679
I am working on esupgrade a tool to upgrade JS/TS code to widely available ECMAScript features. It's built on this wonderful tool. I'd really love to keep esupgrade's code base up to date too, and using for..of loops and generators would help tremendously.
Thanks for your consideration and for publishing this under a free license <3