dioxus icon indicating copy to clipboard operation
dioxus copied to clipboard

Specialize `HistoryProvider::on_update` for web

Open matthunz opened this issue 1 year ago • 1 comments

Removes the need for Send + Sync bounds by specializing the HistoryProvider::on_update with the web feature enabled.

Fixes https://github.com/DioxusLabs/dioxus/issues/1657 by triggering switch_route on the popstate event.

#[cfg(feature = "web")]
fn updater(&mut self, callback: Rc<dyn Fn()>) { ... }

#[cfg(not(feature = "web"))]
fn updater(&mut self, callback: Arc<dyn Fn() + Send + Sync>) { ... }

matthunz avatar Oct 14 '24 16:10 matthunz

#[cfg(feature = "web")] fn updater(&mut self, callback: Rc<dyn Fn()>) { ... }

#[cfg(not(feature = "web"))] fn updater(&mut self, callback: Arc<dyn Fn() + Send + Sync>) { ... }

Specializing based on features is generally problematic because it makes the features nonn-additive. If a library activates the "web" feature on the router and I add that library as a dependency, then my code could break if I do something like this:

let updater = updater();
std::thread::spawn(|| updater);

We already have this issue in a couple different places in the dioxus crates, but we have been slowly working to remove them. For example, the default history for the router is non-additive, but will be removed in #400.

Would it be possible to implement the fix without changing the signature of updater based on feature flags? Potentially by removing the send + sync bound on other platforms or asserting that the web platform is single threaded via thread ids

ealmloff avatar Oct 15 '24 01:10 ealmloff

I think now that the router has moved to be provided by each platform, we can simply implement this in the web crate with 0 configs.

jkelleyrtp avatar Nov 02 '24 00:11 jkelleyrtp

Thanks! After chatting on Discord I totally get why we don't want mututally-exlcusive features.

I have a new version here using an enum to keep the public API the same. There is a hidden #[cfg(target_arch = "wasm32"] though that I hope makes sense in this context 🤔

matthunz avatar Nov 03 '24 20:11 matthunz