-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Curried Kefir.combine #5
Comments
Hmm... Does the first argument need to be an array and what does the output look like with How about naming this combine variant slightly differently. Perhaps something like: U.seq(value,
U.andCombine([foo]),
U.andCombinePassive([bar])) What would the output be like (a nested array?) in the above with two uses? There could be a similar variant for merge (named BTW, in Partial Lenses there is currently an |
Yeah, the first argument would have to be an array (unless you want to support multiple types), and the output would be I like the idea of slightly different naming. So basically we'd have: U.combine
U.andCombine
U.andCombinePassive Of which the latter 2 are curried and But hmm... if you can already use partial.lenses to do similar things, maybe it's worth consideration to not add these variants of combine and only have non-curried version of |
(I just mentioned the partial lenses naming as it is similar (variations of the same operation for different use cases). It doesn't do the same thing.) |
Ah alright. 👍 So, which option would be the best? 3 variants? |
Hmm... Thinking about this I realized that the lifted functions U.seq(value,
U.of,
U.concat(['foo'])) Of course, lifted functions have slightly different semantics (they give properties that skip identical values and perform deep lifting of the arguments) from plain Kefir combinators, but it might also make sense to provide flipped versions of those. |
Been sick for a week so I haven't done anything about this yet. That's not exactly the same, since it doesn't support passive properties. You could also use Still unsure what would be best approach. |
Before doing a PR about this feature, I'd like to discuss how curried Kefir.combine should be implemented in Karet Util.
Personally, I believe this would be the best approach:
But I'd like to double-check if this approach is OK with you @polytypic ? It's worth discussing over because:
There is no longer a single function you can combine active and passive values with. Personally I think it's not a problem.
The bigger issue might be that it's inconsistent with how
Kefir.merge
is currently implemented asU.parallel
, it's not curriable and therefore not directly usable when piping functions like withU.seq
. But on the other hand, you can't simply writeU.combine([...observables])
with this suggested approach.Any opinions?
The text was updated successfully, but these errors were encountered: