re-rxjs / react-rxjs

React bindings for RxJS
https://react-rxjs.org
MIT License
549 stars 19 forks source link

feat(utils): createSignal & createKeyedSignal #172

Closed josepot closed 3 years ago

josepot commented 3 years ago

A couple things in this PR:

codecov[bot] commented 3 years ago

Codecov Report

Merging #172 (5aec029) into main (e372450) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #172   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           24        26    +2     
  Lines          366       389   +23     
  Branches        44        48    +4     
=========================================
+ Hits           366       389   +23     
Impacted Files Coverage Δ
packages/utils/src/createKeyedSignal.ts 100.00% <100.00%> (ø)
packages/utils/src/createListener.ts 100.00% <100.00%> (ø)
packages/utils/src/createSignal.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e372450...5aec029. Read the comment docs.

iprado78 commented 3 years ago

A couple things in this PR:

  • Deprecating createListener in favor of createSignal, b/c I really think that it's a much better name.
  • Adding a createKeyedSignal. I really think that this addition should allow us to avoid using split/collect.

Just a data point, but coming to the library having worked some with RxJS--but not having worked in a lot of message architectures--"producer" would have been the word that immediately told me what I was getting back in the first element of the tuple and could use for sourcing events. "Signal" is more accurate for the whole tuple--and composes better with "keyed"--but also could mean more things. Probably just something to handle in the docs :) .

josepot commented 3 years ago

A couple things in this PR:

  • Deprecating createListener in favor of createSignal, b/c I really think that it's a much better name.
  • Adding a createKeyedSignal. I really think that this addition should allow us to avoid using split/collect.

Just a data point, but coming to the library having worked some with RxJS--but not having worked in a lot of message architectures--"producer" would have been the word that immediately told me what I was getting back in the first element of the tuple and could use for sourcing events. "Signal" is more accurate for the whole tuple--and composes better with "keyed"--but also could mean more things. Probably just something to handle in the docs :) .

Thanks for the feedback @iprado78 . Yeah, I'm still unsure what the best name would be...

In reality createSignal/createListener is just sugar for splitting the Observer and the Observable from the underlying Subject. That's it. In the sense that a Subject implements both interfaces, but in some cases (when you don't want to complete or error from the Subject), it can be convenient to separate them... Specially when using the mapper function.

Now, the new addition createKeyedSignal does a bit more of magic behind the scenes, but it can make our lives a lot easier when working with keyed observables.

It's true that the term "signal" is a bit ambiguous, maybe a more accurate name would have been createInputSignal, but I find it too verbose... Also, it's not like we will ever have a createOutputSignal :slightly_smiling_face:. I've seen the term signal being used quite a lot in different kinds of reactive libraries like bacon.js, solidjs... And IMO it's always used for modeling the "raw" external signals that are being captured.

IMO the term "producer" is a lot less ambiguous, at least in the context of RxJS. I'm pretty sure that in the context of RxJS the term producer is used for describing the entity that is responsible for emitting the "signals". For instance, a way to explain the difference between a hot and a cold observable is to say that hot Observables are those that the producer is defined outside of the Observable, while a cold observable has the producer defined inside the Observable. Basically, the producer is the one responsible for calling the different methods of the Observer. All this is to say that IMO the first element of the tuple doesn't return a producer, it returns the Observable that captures the values of the producer, but not the producer itself.

iprado78 commented 3 years ago

Fair enough, the way I’ve used it, it’s “a replacement for creating a Subject and manually calling next on it” in a lot of contexts where you might want to do that. I associate that with defining your own producer but I guess it’s really more wrapping a producer that you can’t statically define in an Observable.

On Tue, Mar 23, 2021 at 6:02 PM Josep M Sobrepere @.***> wrote:

Merged #172 https://github.com/re-rxjs/react-rxjs/pull/172 into main.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/re-rxjs/react-rxjs/pull/172#event-4498756750, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGAPRRB3RC4ELKIM52IMEDTFEFWJANCNFSM4ZV6EWDA .