vitaly-t / iter-ops

Basic operations on iterables
https://vitaly-t.github.io/iter-ops
MIT License
136 stars 5 forks source link

feat: add support for converting an array-like to an iterable #216

Closed RebeccaStevens closed 1 year ago

RebeccaStevens commented 1 year ago

I've set this up to work for the sync case. The async case will be a little more complicated.

However, I'm not sure if this is worth pursuing as it can lead to false matches users will not be expecting. The only way we can test if something is array-like is by seeing if it has a length property and that it is an integer. However, it is quite plausible for an object to meet that criteria without being array-like. We'd need to add an escape hatch for this case, which would be more work than it's worth imo.

vitaly-t commented 1 year ago

Also, the conversion is inefficient. It should produce a proper iterable, not convert it into array, similar to how reverse was done, but just forward, not backwards.

vitaly-t commented 1 year ago

So I have refactored the implementation to return a proper iterable, and I moved it up, into object condition, because any ArrayLike is always an object, as far as I know.

We still need a test for it.

And, I'm thinking, should we not add the same into toAsync? :wink:

codecov-commenter commented 1 year ago

Codecov Report

Base: 99.75% // Head: 99.45% // Decreases project coverage by -0.30% :warning:

Coverage data is based on head (ed2b0a6) compared to base (56daaad). Patch coverage: 25.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #216 +/- ## ========================================== - Coverage 99.75% 99.45% -0.31% ========================================== Files 49 49 Lines 1639 1647 +8 Branches 337 340 +3 ========================================== + Hits 1635 1638 +3 - Misses 0 4 +4 - Partials 4 5 +1 ``` | [Impacted Files](https://codecov.io/gh/vitaly-t/iter-ops/pull/216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [src/helpers.ts](https://codecov.io/gh/vitaly-t/iter-ops/pull/216/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2hlbHBlcnMudHM=) | `90.19% <0.00%> (-7.64%)` | :arrow_down: | | [src/typeguards.ts](https://codecov.io/gh/vitaly-t/iter-ops/pull/216/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3R5cGVndWFyZHMudHM=) | `96.42% <66.66%> (-3.58%)` | :arrow_down: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

vitaly-t commented 1 year ago

I have added a test also.

vitaly-t commented 1 year ago

@RebeccaStevens Please review my changes, and review/refactor, as you wish.

vitaly-t commented 1 year ago

Ok, I'm merging it as is. I also think that we should not do anything for async here.

If you see a need for improvement, please follow up with another PR ;)