yukinarit / mapbox-gl-rs

Unofficial Rust/WASM binding for mapbox-gl-js
MIT License
39 stars 14 forks source link

Implement PartialEq for Map or MapFactory #11

Open jryio opened 1 year ago

jryio commented 1 year ago

When using mapboxgl inside of a Yew app, it would be helpful to pass around MapFactory or Map as props to other components so they can manipulate the map in different ways. Yew requires that props implement Properties and PartialEq.

Additionally, in order to take a map instance and pass it into a Yew context provider like use_context, the type must implement ParitalEq (docs for use_context).

Global state management crates for Yew like bounce also require that the shared value be ParitalEq.

This will allow the mapbox gl instance to be used outside of the context where it was initialized.

jryio commented 1 year ago

This is an issue I am happy to take on, but I will need to familiarize myself with how to implement PartialEq on JSValues.

yukinarit commented 1 year ago

Hi @thebearjew

I am not an expert on this area so please correct me if this doesn't make any sense.

I think PartialEq is required to propagate changes on properties between components. Map has its own event listener, so you can hook into events that occurred inside Map. If you want to just pass around Map in components and call Map's methods or properties, It doesn't make sense to implement "deep object comparison" (e.g. How to Compare Objects in JavaScript)? πŸ€”

That said, implementing PartialEq to compare underlying JavaScript Object references is enough? πŸ€”

Handle

impl PartialEq for Handle {
    fn eq(&self, other: &Self) -> bool {
        js_sys::Object::is(self.on_data.as_ref(), other.on_data.as_ref()) &&
        js_sys::Object::is(self.on_styledata.as_ref(), other.on_styledata.as_ref()) &&
        ...
    }
}

Map and MapFactory

#[derive(PartialEq)]
pub struct MapFactory {
    pub map: Arc<Map>,
    handle: Option<Handle>,
}

pub struct Map {
    pub(crate) inner: crate::js::Map,
}

impl PartialEq for Map {
    fn eq(&self, other: &Self) -> bool {
        js_sys::Object::is(&self.inner, &other.inner)
    }
}

NOTE: Above code compiles but wasn't tested at all.

jryio commented 1 year ago

I agree that using JavaScript referential equality (Object::is) is enough, deep object comparison should not be required. I think that two mapbox Map objects would be considered equal if there references were the same map1 === map2 in JavaScript. Your assessment is makes perfect sense.

I am working on a PR for this, but I think it's possible to make passing around a Map object if it has Arc. What do you think @yukinarit?

// Get Clone for free because Arc<crate::js::Map>
#[derive(Clone)]
pub struct Map {
    pub(crate) inner: Arc<crate::js::Map>,
}

pub struct MapFactory {
    map: Map,
    handle: Handle
}

impl MapFactory {
    pub fn set_listener<F: MapEventListener + 'static>(&mut self, f: F) {
        // Arc<Map> here
        // I don't think the Arc will be dropped because calling Handle::new() 
        // will move it into a Closure and that should maintain a reference.
        // I am not certain if this will work, please correct me!
        let map = Arc::new(self.map.clone());

        self.handle = Some(Handle::new(Arc::downgrade(&map), f));

        let handle = self.handle.as_ref().unwrap();

        // ...

    }
}

Here's a concrete example of the scenario I am trying to implement:

I would like to control the behavior of the map via Yew hooks and lifecycle events, rather than respond to Mapbox events like onload and onclick. Additionally, when user interactions like button clicks, route changes, and external data are loaded, I would like to control the map to transition it. All of which require that Map and MapFactory have traits which Yew require like ContextProvider.

Example:


#[function_component(LoadRemoteGeoJson)]
fn load_remote_geojson(props: Props) -> Html {
    let geojson = use_fetch::<MyGeoJsonData>(props.url);
    // Accesses the same object from `use_map`
    // This is helpful because it means we can fetch GeoJson outside of the root component `<App>`
    // Can also update or change the map based on button clicks.
    // Currently this is not possible without Map or MapFactory implementing `Clone` and `PartialEq`.
    let map = use_context::<Map>();

   map.add_geojeon_source("walking", geojson);
   map.add_layer("walking", ...);

   // renders props.children
   html! { ... }
}

fn switch(route: Route) -> Html {
    match route {
        Route::User { id } => html {
            // 
            <LoadRemoteGeoJson url={format!("https://example.com/{id}.geojson")}>
                <UserProfile {id} />
            </LoadRemoteGeoJson>
        },
    }
}

#[function_component(App)]
fn app() -> Html {
    // Initialize the map. copied mapbox-gl-rs example.
    let map  = use_map();
    html! {
        // Make the `MapFactory` or `Map` available to any component in the application.
        // This requires `Clone` and `PartialEq` on `Map` or `MapFactory`
        <ContextProvider<Map> context={map.clone}>
            <BrowserRouter>
                // Load different GeoJson based on route
                <Switch<Route> render={switch} />
            </BrowserRouter>
        </ContextProvider<Map>>
    }

}

I should mention that I am not a Rust expert and could use some guidance and help if I make some basic Rust mistakes. I am more knowledgeable about React / Yew / Frontend concepts.

γ‚γ‚ŠγŒγ¨γ†γ”γ–γ„γΎγ™

yukinarit commented 1 year ago

Hi @thebearjew Thanks for the reply! I am totally okay to wrap Map in a smart pointer. But since JavaScript runtime is single-threaded, I think Rc is enough?

Or perhaps service worker is in mind?