chinedufn / percy

Build frontend browser apps with Rust + WebAssembly. Supports server side rendering.
https://chinedufn.github.io/percy/
Apache License 2.0
2.26k stars 84 forks source link

Router: why not pass `Provided<T>` to RouteHandler through `view()` #133

Closed njam closed 4 years ago

njam commented 4 years ago

Currently the Router keeps arbitrary application state called "provided" (with an Rc of Any), then passes a reference to that state to the RouteHandler with set_provided(), to finally pass it to the route-handler function, for example:

#[route(path = "/foo")]
fn route_foo(state: Provided<Rc<RefCell<MyState>>>) -> VirtualNode { }

The route-handler function is executed by the macro from RouteHandler::view(), which is usually called from the application.

Here's the expanded macro:

impl RouteHandler for route_foo_handler {
    fn route(&self) -> &Route {
        &self.route
    }
    fn set_provided(&mut self, provided: ProvidedMap) {
        self.provided = Some(provided);
    }
    fn provided(&self) -> &ProvidedMap {
        &self.provided.as_ref().expect("RouteHandler Provided map")
    }
    fn on_visit(&self, incoming_path: &str) {
        __noop__();
    }
    fn view(&self, incoming_path: &str) -> VirtualNode {
        let state = self.provided().borrow();
        let state = state
            .get(&std::any::TypeId::of::<Provided<Rc<RefCell<MyState>>>>())
            .expect("Get argument type")
            .downcast_ref::<Provided<Rc<RefCell<MyState>>>>()
            .expect("Downcast param");
        route_foo(Provided::clone(state))
    }
}

Would it make sense to pass the "provided" data directly to RouteHandler::view()? I think this could avoid the need for putting the application state into Rc<RefCell<T>> and using Any, because the application can own its state, and just pass a reference to view() temporarily, while rendering the vdom.

Basically passing the state to view(), which then passes it to the route-function?

    fn view<T>(&self, incoming_path: &str, state: T) -> VirtualNode {
        route_foo(state)
    }
njam commented 4 years ago

Maybe I'm missing something obvious. Is there a good reason to own the "provided" on the router?

chinedufn commented 4 years ago

I'd like to move towards a direction where instead of just one massive state object your application can be split into multiple state objects.

I think that this makes writing tests much easier - since you don't have to deal with your entire state object when you're really just trying to test one or two aspects of state.

Given this - passing in provided state as a method wouldn't work since different route handlers will need different state.

Let me know if that makes sense or you have a different idea coming to mind!

njam commented 4 years ago

Ok makes sense, thanks for taking the time to think about it!