From fd489c29f6f0f6a3e6cd89fa3241ef804c1d5623 Mon Sep 17 00:00:00 2001 From: Maciej Hirsz Date: Thu, 25 May 2023 11:24:25 +0200 Subject: [PATCH 1/2] Make bound event listeners `Copy` --- Cargo.lock | 6 +- crates/kobold/Cargo.toml | 4 +- crates/kobold/src/event.rs | 18 ++++ crates/kobold/src/lib.rs | 4 +- crates/kobold/src/stateful.rs | 2 +- crates/kobold/src/stateful/hook.rs | 106 +++++++++++++++++----- crates/kobold_macros/Cargo.toml | 2 +- crates/kobold_macros/src/gen/element.rs | 2 +- crates/kobold_macros/src/gen/transient.rs | 1 + crates/kobold_qr/Cargo.toml | 4 +- 10 files changed, 115 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dd2d9042..189e64fd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -153,7 +153,7 @@ dependencies = [ [[package]] name = "kobold" -version = "0.8.1" +version = "0.9.0" dependencies = [ "console_error_panic_hook", "itoa", @@ -207,7 +207,7 @@ dependencies = [ [[package]] name = "kobold_macros" -version = "0.8.0" +version = "0.9.0" dependencies = [ "arrayvec", "beef", @@ -225,7 +225,7 @@ dependencies = [ [[package]] name = "kobold_qr" -version = "0.8.0" +version = "0.9.0" dependencies = [ "fast_qr", "kobold", diff --git a/crates/kobold/Cargo.toml b/crates/kobold/Cargo.toml index 0214dc40..02441304 100644 --- a/crates/kobold/Cargo.toml +++ b/crates/kobold/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "kobold" -version = "0.8.1" +version = "0.9.0" authors = ["Maciej Hirsz "] edition = "2021" license = "MPL-2.0" @@ -19,7 +19,7 @@ stateful = [] wasm-bindgen = "0.2.84" wasm-bindgen-futures = "0.4.34" itoa = "1.0.6" -kobold_macros = { version = "0.8.0", path = "../kobold_macros" } +kobold_macros = { version = "0.9.0", path = "../kobold_macros" } console_error_panic_hook = "0.1.7" rlsf = { version = "0.2.1", optional = true } diff --git a/crates/kobold/src/event.rs b/crates/kobold/src/event.rs index 11e25443..b63239d1 100644 --- a/crates/kobold/src/event.rs +++ b/crates/kobold/src/event.rs @@ -83,6 +83,24 @@ event! { MouseEvent, } +pub trait IntoListener { + type Listener: Listener; + + fn into_listener(self) -> Self::Listener; +} + +impl IntoListener for L +where + L: Listener, + E: EventCast, +{ + type Listener = L; + + fn into_listener(self) -> L { + self + } +} + pub trait Listener where E: EventCast, diff --git a/crates/kobold/src/lib.rs b/crates/kobold/src/lib.rs index 0033cb83..b95f6952 100644 --- a/crates/kobold/src/lib.rs +++ b/crates/kobold/src/lib.rs @@ -585,7 +585,7 @@ macro_rules! class { /// let increment = move |_| *count += 1; /// let decrement = move |_| *count -= 1; /// } -/// # fn throwaway(_: impl kobold::event::Listener) {} +/// # fn throwaway(_: kobold::stateful::Bound) {} /// # throwaway(increment); /// # throwaway(decrement); /// # } @@ -597,7 +597,7 @@ macro_rules! class { /// # fn test(count: &Hook) { /// let increment = count.bind(move |count, _| *count += 1); /// let decrement = count.bind(move |count, _| *count -= 1); -/// # fn throwaway(_: impl kobold::event::Listener) {} +/// # fn throwaway(_: kobold::stateful::Bound) {} /// # throwaway(increment); /// # throwaway(decrement); /// # } diff --git a/crates/kobold/src/stateful.rs b/crates/kobold/src/stateful.rs index 5c0c0a5e..80232f4b 100644 --- a/crates/kobold/src/stateful.rs +++ b/crates/kobold/src/stateful.rs @@ -31,7 +31,7 @@ mod should_render; use cell::WithCell; use product::{Product, ProductHandler}; -pub use hook::{Hook, Signal}; +pub use hook::{Bound, Hook, Signal}; pub use into_state::IntoState; pub use should_render::{ShouldRender, Then}; diff --git a/crates/kobold/src/stateful/hook.rs b/crates/kobold/src/stateful/hook.rs index 3b5c1b03..76c9708f 100644 --- a/crates/kobold/src/stateful/hook.rs +++ b/crates/kobold/src/stateful/hook.rs @@ -93,7 +93,7 @@ impl Hook { /// Binds a closure to a mutable reference of the state. While this method is public /// it's recommended to use the [`bind!`](crate::bind) macro instead. - pub fn bind(&self, callback: F) -> impl Listener + pub fn bind(&self, callback: F) -> Bound where S: 'static, E: EventCast, @@ -102,25 +102,7 @@ impl Hook { { let inner = &self.inner as *const Inner; - let bound = move |e| { - // ⚠️ Safety: - // ========== - // - // This is fired only as event listener from the DOM, which guarantees that - // state is not currently borrowed, as events cannot interrupt normal - // control flow, and `Signal`s cannot borrow state across .await points. - let inner = unsafe { &*inner }; - let state = unsafe { inner.state.mut_unchecked() }; - - if callback(state, e).should_render() { - inner.update(); - } - }; - - Bound { - bound, - _unbound: PhantomData::, - } + Bound { inner, callback } } pub fn bind_async(&self, callback: F) -> impl Listener @@ -162,12 +144,63 @@ impl Hook { } } -struct Bound { +pub struct Bound { + inner: *const Inner, + callback: F, +} + +impl Bound { + pub fn into_listener(self) -> impl Listener + where + S: 'static, + E: EventCast, + F: Fn(&mut S, E) -> O + 'static, + O: ShouldRender, + { + let Bound { inner, callback } = self; + + let bound = move |e| { + // ⚠️ Safety: + // ========== + // + // This is fired only as event listener from the DOM, which guarantees that + // state is not currently borrowed, as events cannot interrupt normal + // control flow, and `Signal`s cannot borrow state across .await points. + let inner = unsafe { &*inner }; + let state = unsafe { inner.state.mut_unchecked() }; + + if callback(state, e).should_render() { + inner.update(); + } + }; + + BoundListener { + bound, + _unbound: PhantomData::, + } + } +} + +impl Clone for Bound +where + F: Clone, +{ + fn clone(&self) -> Self { + Bound { + inner: self.inner, + callback: self.callback.clone(), + } + } +} + +impl Copy for Bound where F: Copy {} + +struct BoundListener { bound: B, _unbound: PhantomData, } -impl Listener for Bound +impl Listener for BoundListener where B: Listener, E: EventCast, @@ -216,3 +249,32 @@ where (**self).update(p) } } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn bound_callback_is_copy() { + union MockInner { + thin: (*const (), usize), + fat: *const Inner, + } + + // This is horribly wrong, but since this is just a test and we + // never dereference the pointer it should be fine. + let inner: *const Inner = unsafe { + MockInner { thin: (std::ptr::null(), 0) }.fat + }; + + let mock = Bound { + inner, + callback: |state: &mut i32, _: web_sys::Event| { + *state += 1; + } + }; + + // Make sure we can copy the mock twice + drop([mock, mock]); + } +} diff --git a/crates/kobold_macros/Cargo.toml b/crates/kobold_macros/Cargo.toml index 28afa93c..72a007fc 100644 --- a/crates/kobold_macros/Cargo.toml +++ b/crates/kobold_macros/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "kobold_macros" -version = "0.8.0" +version = "0.9.0" authors = ["Maciej Hirsz "] edition = "2021" license = "MPL-2.0" diff --git a/crates/kobold_macros/src/gen/element.rs b/crates/kobold_macros/src/gen/element.rs index 13e0f1bc..253889c0 100644 --- a/crates/kobold_macros/src/gen/element.rs +++ b/crates/kobold_macros/src/gen/element.rs @@ -134,7 +134,7 @@ impl IntoGenerator for HtmlElement { expr.stream, ) } else { - expr.stream + (expr.stream, ".into_listener()").tokenize() }; let value = gen.add_field(coerce).event(event, el.typ).name; diff --git a/crates/kobold_macros/src/gen/transient.rs b/crates/kobold_macros/src/gen/transient.rs index 155cd298..923739c4 100644 --- a/crates/kobold_macros/src/gen/transient.rs +++ b/crates/kobold_macros/src/gen/transient.rs @@ -184,6 +184,7 @@ impl Tokenize for Transient { "\ use ::kobold::dom::Mountable as _;\ use ::kobold::event::ListenerHandle as _;\ + use ::kobold::event::IntoListener as _;\ use ::kobold::reexport::wasm_bindgen;\ ", self.js, diff --git a/crates/kobold_qr/Cargo.toml b/crates/kobold_qr/Cargo.toml index e2f1994d..8eda615b 100644 --- a/crates/kobold_qr/Cargo.toml +++ b/crates/kobold_qr/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "kobold_qr" -version = "0.8.0" +version = "0.9.0" authors = ["Maciej Hirsz "] edition = "2021" license = "MPL-2.0" @@ -10,7 +10,7 @@ description = "QR code component for Kobold" [dependencies] fast_qr = "0.8.5" -kobold = { version = "0.8.0", path = "../kobold" } +kobold = { version = "0.9.0", path = "../kobold" } wasm-bindgen = "0.2.84" [dependencies.web-sys] From fcf28a2fafe5d729f7d2ed8d055108d2dda35cdd Mon Sep 17 00:00:00 2001 From: Maciej Hirsz Date: Thu, 25 May 2023 11:45:08 +0200 Subject: [PATCH 2/2] Satisfy Miri in tests --- crates/kobold/src/stateful/hook.rs | 29 +++++++++++++++++---------- crates/kobold/src/stateful/product.rs | 11 ++++++++++ crates/kobold/src/value.rs | 4 ++-- 3 files changed, 31 insertions(+), 13 deletions(-) diff --git a/crates/kobold/src/stateful/hook.rs b/crates/kobold/src/stateful/hook.rs index 76c9708f..7ec5f628 100644 --- a/crates/kobold/src/stateful/hook.rs +++ b/crates/kobold/src/stateful/hook.rs @@ -252,26 +252,33 @@ where #[cfg(test)] mod test { + use std::cell::UnsafeCell; + use wasm_bindgen::JsCast; + + use crate::stateful::cell::WithCell; + use crate::stateful::product::ProductHandler; + use crate::value::TextProduct; + use super::*; #[test] fn bound_callback_is_copy() { - union MockInner { - thin: (*const (), usize), - fat: *const Inner, - } - - // This is horribly wrong, but since this is just a test and we - // never dereference the pointer it should be fine. - let inner: *const Inner = unsafe { - MockInner { thin: (std::ptr::null(), 0) }.fat + let inner = Inner { + state: WithCell::new(0_i32), + prod: UnsafeCell::new(ProductHandler::mock( + |_, _| {}, + TextProduct { + memo: 0, + node: wasm_bindgen::JsValue::UNDEFINED.unchecked_into(), + }, + )), }; let mock = Bound { - inner, + inner: &inner as *const _, callback: |state: &mut i32, _: web_sys::Event| { *state += 1; - } + }, }; // Make sure we can copy the mock twice diff --git a/crates/kobold/src/stateful/product.rs b/crates/kobold/src/stateful/product.rs index b2acddd4..c28a2481 100644 --- a/crates/kobold/src/stateful/product.rs +++ b/crates/kobold/src/stateful/product.rs @@ -26,6 +26,17 @@ pub struct ProductHandler { _state: PhantomData, } +#[cfg(test)] +impl ProductHandler { + pub(crate) fn mock(updater: F, product: P) -> Self { + ProductHandler { + updater, + product, + _state: PhantomData, + } + } +} + impl ProductHandler { pub fn build(updater: F, view: V, p: In) -> Out where diff --git a/crates/kobold/src/value.rs b/crates/kobold/src/value.rs index 6cbbcc37..ce90a432 100644 --- a/crates/kobold/src/value.rs +++ b/crates/kobold/src/value.rs @@ -59,8 +59,8 @@ impl_value!(bool: bool); impl_value!(f64: u8, u16, u32, usize, i8, i16, i32, isize, f32, f64); pub struct TextProduct { - memo: M, - node: Node, + pub(crate) memo: M, + pub(crate) node: Node, } impl Anchor for TextProduct {