vitaly-t / sub-events

Lightweight, strongly-typed events, with monitored subscriptions.
https://vitaly-t.github.io/sub-events
MIT License
38 stars 2 forks source link

Add last-value/immediate option #29

Open suterma opened 2 days ago

suterma commented 2 days ago

Hi and first of all thank you for this great event handling framework.

One issue I came across, was that it does not support the receipt of the "last emitted value" at subscription time, like e.g. the "immediate" feature VueJs-Watchers do have.

Context: I have a handler for stuff, that emits various events, some of the events regularly, some seldom. New users (clients) of such handlers register to the various events, but do not get the "current" state of the respective property automatically.

So, right after subscription, the clients need to get the current state, for each event, via an explicit call. This adds quite some boilerplate code, eg. below, for each event/property:

onDurationChangedSubscription = handler.onDurationChanged.subscribe(
    (duration) => {
        updateDuration(duration); // update the emitted state
    },
);
updateDuration(handler.duration); // get current state

The preferred solution for this would be an additional option to the subscription that initially calls the "updateDuration" function with the last ever emitted value (making the last line in the above example obsolete).

vitaly-t commented 18 hours ago

That is effectively the kind of feature available in RXJS, whereas this library is more of a replacement for the standard EventEmitter, which does not support any previous state broadcast.

I do see the value in adding such feature though, so I will consider it.

The preferred solution for this would be an additional option to the subscription that initially calls the "updateDuration" function with the last ever emitted value

In RXJS such a functional distinction is made at construction time, and so I might follow the same pattern here, by extending IEventOptions. Need to think about that. What do you think?

suterma commented 17 hours ago

Performance-wise the use of an event option at construction time might be best. Thus, you only need to keep a value around, when it's actually intended.

However a subscriber (I like that term better than Observer, like with RXJS), will never know then, because it will not be able to willfully set or even retrieve, this option for itself, when it actually subscribes. So from a usage standpoint, it will be best, if the subscriber can determine at subscription time. I would not go so far as to actually provide the old value for reference, or even a flag whether this is a "retained" value. This would be unhelpful most of the time I guess.

For me, the first, your, idea would be most straightforward, since I control both the emitter and subscriber, thus will be able to handle the situation optimally for each event type anyway.

vitaly-t commented 17 hours ago

Ok, so I'm thinking now of adding emitLast: boolean flag to ISubOptions interface.

However, it is not clear then what should we do inside once in this case, which takes the same options. Should we treat the last value as the only value or wait for one after, before auto-unsubscribing? 😸

@suterma Any idea?

UPDATE(s)

The more I think about it, the more I believe that if you pass emitLast: true into once method, it should unsubscribe immediately, and not wait for another event to occur. That would seem consistent, logic-wise.

More problems - method toPromise would need to be extended, for consistency, to also support emitLast.

Not such a simple feature after all 😄

suterma commented 15 hours ago

Yeah, to keep things simple, one needs to put a lot of thinking into it :-)

Good luck. I have solved my problem at hand, just so you know, for the time being, using my workaround from the initial post. It's doing fine and maybe, just maybe, is the most simple and straightforward way after all. :-)

vitaly-t commented 14 hours ago

This library hasn't been updated in ages. I will need to update everything before adding any new feature.

Started with this PR.

vitaly-t commented 13 hours ago

That PR has been merged, a new version 1.9.1 released with tons of updates and changes, except no code changes, hence the minor version increment. This way we now have a good base to start looking at new features ;)

vitaly-t commented 12 hours ago

I have started adding this feature in this PR.

vitaly-t commented 10 hours ago

This feature seems to create a lot of logical problems - race conditions between emitting events, subscribing, notifying and providing the start value. I've been trying to figure it all out, and at the moment I do not see how this will work correctly...

I cannot just send the last value when calling subscribe, because, for example inside once we try to immediately unsubscribe when we get the first value, which in this case would be triggered before a subscription is even created, which is a logical contradiction.

I am now thinking of maybe simplifying it, by not having any new option, and instead have SubEvent support lastEvent read-only property to always expose the last emitted event. This way things will get super-simple and reliable, no issues.

UPDATE

After a bit more thinking, the latter is also not without problems. If I set this property after the event has been emitted to all subscribers, then new subscribers may miss it even though the emit for the event has already been called. And if I set it at the start of the emit, then lastEvent may contain event that your subscriber is about to receive, thus causing event duplication.

suterma commented 10 hours ago

About immediate unsubscription: Could you not return an already cancelled subscription in this case? This is how I return promises, when there is nothing actually to wait for. E.g. here in my app: https://github.com/suterma/replayer-pwa/blob/de02fc5878831b28b28936dd9ced25ef6a9eb936/src/code/media/AudioFader.ts#L370C1-L372C17

If you have a "lastEvent" property, this might be fine, but would not solve my problem, I still would need to have the additional line of code. Querying the event, instead of the emitter object, but effectively no savings for me.

In any case, don't feel obliged to implement this for me personally. It's no deal-breaker for me.

vitaly-t commented 10 hours ago

Could you not return an already cancelled subscription in this case?

It is not that simple, subscriptions have events they provide, like when you unsubscribe, which isn't possible to do before a subscription is even created.

As per the above UPDATE, adding lastEvent is also problematic, as adding it at the start of emission will result in event duplication, while setting it at the end of emission may result in missed events for new subscribers.

In other words, I am stuck with this feature for the moment, it does not fit into the existing concurrency model, it creates logical/concurrency problems.

P.S. Looking on the bright side, at least I have made a major update of the library with the previous PR, for v1.9.1

vitaly-t commented 10 hours ago

If you have a "lastEvent" property, this might be fine, but would not solve my problem, I still would need to have the additional line of code. Querying the event, instead of the emitter object, but effectively no savings for me.

Kind of, but you would be able to get the last event before creating a new subscription, and not change the code inside the subscription :wink: That way, I think, it is still quite useful, if you think about it, no need re-emitting anything.

vitaly-t commented 10 hours ago

I have just published 2.0.0-beta.1 beta version, with lastEvent added to SubEvent. You can try and play with, see if it helps you at all :wink:

vitaly-t commented 7 hours ago

Released v1.10.0, with lastEvent added, plus fixed bug #33 in it. This might be it, as far as this feature goes.