squint icon indicating copy to clipboard operation
squint copied to clipboard

seq return iterable

Open brandonstubbs opened this issue 2 years ago • 2 comments

Please answer the following questions and leave the below in as part of your PR.

  • [x] This PR corresponds to an issue with a clear problem statement.

  • [x] This PR provides tests for code additions and changes

  • [x] This PR does not contain bonus changes, i.e. changes not discussed in the linked issue. For those, please submit a new issue + PR.

After submitting this PR, please do not force-push your branch as this makes collaboration easier. Push as many new commits to your branch, they will be squashed upon merge.


This PR is addressing issue #92 and discussion #109

brandonstubbs avatar Aug 22 '22 14:08 brandonstubbs

Can you mention the issue in the original message?

borkdude avatar Aug 22 '22 15:08 borkdude

I have done the following changes:

seqable?

Before the following cases would fail:

(seqable? {:a 1})
(seqable? 1)
(seqable? true)

As per our discussion, you can be seqable if you are one of the following:

  1. You implement the Symbol.iterator protocol
  2. You're a POJO
  3. You're nil

I have done the changes to just deal with the base types, as null and Symbol.iterator are object types

NOTE: cases such as new Boolean(true) would have and still will fail (as these are object types and not their primitives), we can add fine-grained checking if we think we need to handle these scenarios. What do you think?

iterable

iterable still returns an iterable even if it's empty. However, now there is a change that it will return undefined if there is a a type that can not be an iterable such as a boolean or number.

seq

Now throws if the coll is not iterable.

es6_iterator

calls seq internally but it will return an iterable even if the collection is empty. Functions that were using iterable before should now use es6_iterator as this is safe to destructure. It also gives the same behaviour clojurescript would for cases such as (first 1) or (map identity 1)

Let me know your thoughts on this and if you are happy I will update the documentation accordingly.

cc @lilactown

brandonstubbs avatar Aug 22 '22 20:08 brandonstubbs

I'm closing this for now as I'd like to only keep things open that are worked on. Feel free to re-open as necessary.

borkdude avatar Oct 03 '22 11:10 borkdude