Closed mlrv closed 3 years ago
@mlrv I'm not sure if it's worth investing time as for now there is a decision to drop support for List
and NEL
in version 1.0.0
in favour of some small library allowing integration with https://github.com/immutable-js/immutable-js collections.
But anywy - it's good to have it for now. And It would be great fi you can also add nth()
that would return value as is ( undefined
if it's >=
than length ). That gives end user enough flexibility :)
But anywy - it's good to have it for now. And It would be great fi you can also add
nth()
that would return value as is (undefined
if it's>=
than length ). That gives end user enough flexibility :)
Ah sure, makes sense 👍 It's not a lot of work, so it shouldn't be a big deal if this stuff gets removed in a later release.
In terms of nth
, I'm not really sure, isn't that against the fundamental principles of this (or any other FP) library? It's trivial to implement of course, but I wonder if it's not just a way to help developers shoot themselves in the foot. Happy to be contradicted of course :)
I'm not really sure, isn't that against the fundamental principles of this (or any other FP) library?
I think it's good to allow users to opt-out
of wrapping null
/ undefined
elements (this is the cost of the decision made to not allow Some
to contain null | undefined
🙃 ).
Mmmh I can see the CI failing, makes sense. Should we rather have nth
return null
? Not that it makes much difference, as long as it's documented, but given the nature of the function and how close it is to just a normal index access, I would expect it to return undefined
in those circumstances
Yes, undefined is ok. I believe you can suppress eslint for those 2 lines, as it's a valid place to ignore the rules :)
Add
List.lookup(i)
andNEL.lookup(i)
for safe access at index, should be good enough to close #229.Note that I decided to treat
undefined
values as missing, not sure whether that's the right decision or not, happy to hear feedback on this.