rxdart icon indicating copy to clipboard operation
rxdart copied to clipboard

add ValueObservable.combineLatestN + tests

Open lukepighetti opened this issue 4 years ago • 5 comments

Quick PR just to see what your appetite is for ValueObservable specific methods to preserve ValueObservable type

lukepighetti avatar Sep 26 '19 12:09 lukepighetti

I actually really like the idea behind this change. It's something I'd considered in the past as well!

One question about the implementation that I'd love some feedback on: It currently uses shareValueSeeded to achieve the effect. That will mean the Observable will become unusable after the final listener unsubscribes to it. Would that be the expected behavior for peeps using this code?

brianegan avatar Oct 14 '19 11:10 brianegan

I was not aware that was a side-effect of using shareValueSeeded. The purpose for using it was to try to preserve value at all costs.

lukepighetti avatar Oct 14 '19 17:10 lukepighetti

Heya @lukepighetti -- how are ya feeling about this change?

brianegan avatar Nov 22 '19 13:11 brianegan

If you can provide a quick and clear request for the change I'd be happy to implement it and push. My main concern is making sure this is consistent with your vision for rxdart

lukepighetti avatar Nov 25 '19 21:11 lukepighetti

One thing I was considering: I don't think most people want shareValueSeeded (from the Rx spec), which will shut down when there are no more listeners. Rather, I think most folks want an operator that works like asBroadcastStream but also captures the latest value. I was thinking of adding two new operators:

asBroadcastValueStream & asBroadcastValueStreamSeeded. These would work exactly like asBroadcastStream -- in fact, they'd use that method under the hood, but would also provide access to the latest emitted value.

Perhaps we could use that base for this change?

brianegan avatar Nov 26 '19 08:11 brianegan