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

Download example #115

Closed aubaugh closed 5 years ago

aubaugh commented 5 years ago

This pull request is to add the feature to download JSON from the Github API for the isomorphic example. The JSON data holds information about the contributors for the master branch of Percy.

Right now only the client performs the fetch request, but I'd imagine there's a way to have the server perform the same operation before serving the route. I'm just not completely sure how to go about doing that.

The contributors are stored in an Option<Vec<Contributor>> field for State. While Contributor holds the login name and html url for each contributor. Right now the definition for Contributor lives in app/src/state.rs.

aubaugh commented 5 years ago

I was having some difficulties with lifetime errors when trying to implement Msg::DownloadContributors. I'll give it another shot later today.

chinedufn commented 5 years ago

I was having some difficulties with lifetime errors when trying to implement Msg::DownloadContributors. I'll give it another shot later today.

Ahhh gotcha sounds good - just lmk if I can help!

aubaugh commented 5 years ago

So far I have download_contributors_json calling store.borrow_mut().msg(&Msg::DownloadContributors). I moved the download_json declaration to store.rs, and this is what I have in store.rs for Msg::DownloadContributors:

Msg::DownloadContributors => {
    let callback = Closure::wrap(Box::new(move |json: JsValue| {
        self.state.msg(&Msg::SetContributorsJson(json));
    }) as Box<FnMut(JsValue)>);
    download_json(
        "https://api.github.com/repos/chinedufn/percy/contributors",
        callback.as_ref().unchecked_ref(),
    );

    callback.forget();
}

I then get the following error when running:

error[E0495]: cannot infer an appropriate lifetime due to conflicting requirements
  --> examples/isomorphic/app/src/store.rs:46:55
   |
46 |                   let callback = Closure::wrap(Box::new(move |json: JsValue| {
   |  _______________________________________________________^
47 | |                     self.state.msg(&Msg::SetContributorsJson(json));
48 | |                 }) as Box<FnMut(JsValue)>);
   | |_________________^
   |
note: first, the lifetime cannot outlive the anonymous lifetime #1 defined on the method body at 30:5...
  --> examples/isomorphic/app/src/store.rs:30:5
   |
30 | /     pub fn msg(&mut self, msg: &Msg) {
31 | |         match msg {
32 | |             Msg::SetPath(path) => {
33 | |                 if let Some(router) = &self.router {
...  |
62 | |         }
63 | |     }
   | |_____^
   = note: ...so that the types are compatible:
           expected &mut store::Store
              found &mut store::Store
   = note: but, the lifetime must be valid for the static lifetime...
   = note: ...so that the types are compatible:
           expected wasm_bindgen::closure::WasmClosure
              found wasm_bindgen::closure::WasmClosure

I'm just not sure how to interpret this error. The concept of lifetimes is new to me. An explanation from another perspective might be helpful.

chinedufn commented 5 years ago

The issue is this line

self.state.msg(&Msg::SetContributorsJson(json));

Your closure is capturing a reference to self with no guarantee that that self will live long enough.

For all the compiler knows self will get deleted before your fetch request finishes - so it can't prove that this code will work.


Rather than worry about getting around that ... for now let's just move it back to how you had it originally and not worry about this.

Sorry!!

aubaugh commented 5 years ago

No worries! Thanks for the info.

At the moment the example doesn't seem to fetch the JSON when I try to navigate to localhost:7878/contributors in the browser (without clicking on the link in the HomeView).

Would this be because the server isn't fetching the JSON? Should I use an HTTP client to do that, such as actix_web::client?

chinedufn commented 5 years ago

It's because this line is what ends up fetching the JSON

https://github.com/chinedufn/percy/blob/3174f22510f7d74271fb97507eb4512ece0d726d/examples/isomorphic/app/src/store.rs#L28

So over here https://github.com/chinedufn/percy/blob/3174f22510f7d74271fb97507eb4512ece0d726d/examples/isomorphic/app/src/lib.rs#L48

you can add something along the lines of

store.borrow_mut().msg(&Msg::SetPath(store.borrow().path.clone()) and things should work


Should I use an HTTP client to do that, such as actix_web::client

You could but I think it's fine/better in my opinion to let the client handle this so that the initial page load time is faster.

Pre-downloading on the server is usually useful if the client has a slower internet connection but we can probably think about that in the future.

aubaugh commented 5 years ago

That did the trick!

I'm not sure how to check has_initiated_contributors_download with a surrounding if statement without getting an already mutably borrowed error message, since store is mutably borrowed in the callback closure.

chinedufn commented 5 years ago

I'm not sure how to check has_initiated_contributors_download with a surrounding if statement without getting an already mutably borrowed error message, since store is mutably borrowed in the callback closure.

Ahhh right

Maybe have the download_contributors_json have the if statement and download attempt wrapped in a request_animation_frame (instead of right now it just immediately attempts to download

https://github.com/rustwasm/wasm-bindgen/blob/master/examples/request-animation-frame/src/lib.rs#L10-L14

Then you can let store = Rc::clone(&store), move that into your RAF closure and you'll be able to store.borrow() just fine.

aubaugh commented 5 years ago

Thank you!

Why does it work with request_animation_frame being run? I don't understand the difference of the borrows on store between the download being performed immediately and RAF being run followed by the download being performed.

chinedufn commented 5 years ago

Why does it work with request_animation_frame being run? I don't understand the difference of the borrows on store between the download being performed immediately and RAF being run followed by the download being performed.

on_visit() gets called in here, where Store is mutably borrowed https://github.com/chinedufn/percy/blob/master/examples/isomorphic/app/src/store.rs#L23

So trying to borrow again gave you that error.

RAF scheduled the closure to run on a different tick (during the next animation frame), completely outside of that mutable borrow.

So when you try to borrow Store in your closure it isn't already mutably borrowed so everything is okay.

chinedufn commented 5 years ago

Merged - awesome work !!!