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

Implement PartialEq for Map or MapFactory #11

Open
jryio opened this issue Dec 6, 2022 · 4 comments
Open

Implement PartialEq for Map or MapFactory #11

jryio opened this issue Dec 6, 2022 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@jryio
Copy link
Contributor

jryio commented Dec 6, 2022

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
Copy link
Contributor Author

jryio commented Dec 6, 2022

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
Copy link
Owner

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
Copy link
Contributor Author

jryio commented Dec 9, 2022

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
Copy link
Owner

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?

  • Arc implements Send and Sync which can be shared across threads
  • Rc doesn't implement Send and Sync which can be shared in the same thread

Or perhaps service worker is in mind?

@yukinarit yukinarit added the enhancement New feature or request label Dec 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants