Skip to content
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

Add create_signal_from_stream function #545

Merged
merged 4 commits into from
Aug 12, 2024

Conversation

sgued
Copy link
Contributor

@sgued sgued commented Aug 10, 2024

I think this is a better implementation of create_signal_from_tokio_channel:

  • This implementation properly handles the effect callback being dropped (when the parent scope is disposed?) by dropping the stream
  • This implementation does not need to spawn a new task, and therefore is runtime independant
  • This implementation is generic over streams (including futures::channel::Receiver)
  • This implementation avoids having the value wrapped in an Option (if no initial value is available, it is easy to map the stream to wrap values in a Some with StreamExt::map
  • This implementation does not require the value to be Send

Overall is there a plan for async interop in floem? I wonder if I should implement an API like Leptos' resource API next.

  • add example ( a version of the timer that instead uses a tokio stream)

  • add test for the infinite loop case I'm worried about:
    Basically it's checking that using the following stream implementation in tokio-timer does not lead to an infinite loop, but I'm not sure how to make it into a test. It works with the current version, but I'm afraid that it would be possible that a future change to the reactive system could break this.

struct StreamWrapper {
    inner: IntervalStream,
    should_yield: bool,
}

impl Stream for StreamWrapper {
    type Item = Instant;
    fn poll_next(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Instant>> {
        let mut this = self.as_mut();
        let should_yield = this.should_yield;
        let should_yield = mem::replace(&mut this.should_yield, !should_yield);
        if should_yield {
            cx.waker().wake_by_ref();
            return Poll::Pending;
        }

        Pin::new(&mut this.inner).poll_next(cx)
    }

    fn size_hint(&self) -> (usize, Option<usize>) {
        self.inner.size_hint()
    }
}

@sgued sgued marked this pull request as draft August 10, 2024 18:28
@sgued sgued marked this pull request as ready for review August 10, 2024 21:06
I think this is a better implementation of create_signal_from_tokio_channel:

- This implementation properly handles the effect callback being dropped (when the parent scope is disposed?) by dropping the stream
- This implementation does not need to spawn a new task, and therefore is runtime independant
- This implementation is generic over streams (including futures::channel::Receiver)
- This implementation avoids having the value wrapped in an `Option` (if no initial value is available, it is easy to map the stream to wrap values in a `Some` with [`StreamExt::map`](https://docs.rs/futures/latest/futures/stream/trait.StreamExt.html#method.map)
- This implementation does not require the value to be `Send`
… an effect

for a trigger for an external event.

The locks were held for longer than necessary. This lead to a deadlock if an
external event re-registered itself directly in an effect for itself.

This should also improve performance as the contention should be lower, and the lock
is locked only once in the idle state.
The ext-event-handlers registers triggers to be handled in the idle of the app,
so the effect can never run recursively
@dzhou121
Copy link
Contributor

This does look really nice thanks!

As in async interop, I think that's desirable. And feel free to explore it.

@dzhou121 dzhou121 merged commit 3564120 into lapce:main Aug 12, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants