paldepind / flyd

The minimalistic but powerful, modular, functional reactive programming library in JavaScript.
MIT License
1.56k stars 84 forks source link

Fix ending a stream while updating #229

Open nordfjord opened 10 months ago

nordfjord commented 10 months ago

Fixes #228 which highlighted an edge case that happens when a stream is ended while being processed in very specific circumstances. In those circumstances the end stream will be triggered before the listeners because it's updated is queued before the listener updates.

To solve for this we add a toEnd array into which the end stream of the current stream will be pushed to if it's updated while while the main stream is updated (that's a convoluted sentence, it's a convoluted data flow, not sure how to communicate this properly)

nordfjord commented 10 months ago

@paldepind I currently have permissions to merge to the repo, but I'm not able to publish flyd releases. Would appreciate, given you have some spare time, if you could review/merge/publish

paldepind commented 10 months ago

Thanks for fixing this @nordfjord. I'll look at the PR soon 🙂

Please share your npm username then I'll also add you to the npm package to give you publish rights 🙂

nordfjord commented 10 months ago

Thanks for fixing this @nordfjord. I'll look at the PR soon 🙂

Please share your npm username then I'll also add you to the npm package to give you publish rights 🙂

Awesome, thanks mate! My npm username is also nordfjord

paldepind commented 9 months ago

Thank you for looking into this @nordfjord! I haven't completely understood the issue yet. But, I wonder if the test could be simplified further? Do the details inside withLatestFrom matter for instance?

nordfjord commented 9 months ago

But, I wonder if the test could be simplified further?

Absolutely, I've attempted to whittle it down now! I'm honestly not sure how to communicate the issue, the dataflow to trigger it is quite strange

nordfjord commented 9 months ago

as an example of how edge casy this thing is, the issue doesn't manifest (on master) if I only combine a single stream.

Screenshot 2024-02-05 at 15 15 11 Screenshot 2024-02-05 at 15 15 31