Pauan / rust-signals

Zero-cost functional reactive Signals for Rust
MIT License
675 stars 37 forks source link

Implementing `Signal` #61

Closed wishawa closed 2 years ago

wishawa commented 2 years ago

Hi! I am making a state management library and I want it to work with futures-signals. I have some questions about implementing Signal on my own types.

Pauan commented 2 years ago

From looking at the source it seems that poll_changed should always return Poll::Ready(Some(value)) on the first poll. i.e. signals are always considered "changed" upon creation. Is this correct?

That's correct. There is an undocumented Signal contract:

https://github.com/Pauan/rust-signals/blob/36585021f8230d74083640609b90480dbf1b6078/src/signal/signal.rs#L16-L23

My types are !Send and !Sync. Would that cause any problem with this library?

That's not an issue, it just means that you have to spawn the Signal using a local spawner like spawn_local:

https://docs.rs/tokio/1.20.1/tokio/task/fn.spawn_local.html

https://docs.rs/futures/0.3.21/futures/task/trait.LocalSpawnExt.html#method.spawn_local

https://docs.rs/async-std/1.12.0/async_std/task/fn.spawn_local.html

https://docs.rs/wasm-bindgen-futures/0.4.32/wasm_bindgen_futures/fn.spawn_local.html

By the way, instead of manually implementing Signal (which is rather tricky), you can instead use channel, which behaves similar to mpsc. Because channel is very fast and lightweight, it's ideal for converting existing APIs (like event listeners) into a Signal:

fn my_signal() -> impl Signal<Item = u32> {
    let (sender, receiver) = channel(0);

    register_event_listener(move |value| {
        sender.send(value).unwrap();
    });

    receiver
}

If you need to clean things up, you can wrap it in a struct:

struct MySignal {
    event_listener: EventListener,
    receiver: Receiver<u32>,
}

// This cleans up the event listener when it's dropped, so it doesn't leak memory
impl Drop for MySignal {
    fn drop(&mut self) {
        self.event_listener.unregister();
    }
}

// This just forwards the implementation to the channel
impl Signal for MySignal {
    type Item = u32;

    #[inline]
    fn poll_change(self: Pin<&mut Self>, cx: &mut Context) -> Poll<Option<Self::Item>> {
        self.receiver.poll_change_unpin(cx)
    }
}

fn my_signal() -> MySignal {
    let (sender, receiver) = channel(0);

    let event_listener = register_event_listener(move |value| {
        sender.send(value).unwrap();
    });

    MySignal {
        event_listener,
        receiver,
    }
}
wishawa commented 2 years ago

Thanks for the quick reply!

That's correct. There is an undocumented Signal contract

This is exactly what I need.

By the way, instead of manually implementing Signal (which is rather tricky), you can instead use channel, which behaves similar to mpsc. Because channel is very fast and lightweight, it's ideal for converting existing APIs (like event listeners) into a Signal

I'll try manual implementation first to avoid allocation, but if that doesn't work out I'll definitely try using channel.

One more question: What would cause Poll::Ready(None) to be returned? One cause I found from looking at the code is when every handle to the backing Mutable has been dropped. Are there any other cause?

Pauan commented 2 years ago

One more question: What would cause Poll::Ready(None) to be returned?

Lots of reasons:

When a Signal returns Poll::Ready(None), the consumer knows that the Signal won't change anymore, so it can stop listening for changes, which is more efficient.

wishawa commented 2 years ago

Okay thanks so much! I think my implementation is complete. I'll share the result once I'm done testing it.