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

scalable application support #189

Conversation

@MartinKavik
Copy link
Contributor

commented Jul 24, 2019

Well, RealWorld example is almost done (code is complete, I need to update readme, create demo and write some tests). So I can finally create PR with improvements which I created while I was working on it.
There are relatively many changes so I'll try to explain them on examples.


Let's look at examples/animation_frame/src/lib.rs:

1. Change:
From:

fn update(msg: Msg, model: &mut Model, orders: &mut Orders<Msg>) {

To:

fn update(msg: Msg, model: &mut Model, orders: &mut impl Orders<Msg>) {

Order isn't struct but trait now, so we have to use keyword impl. It's a breaking change but it has advantages which I'll describe later.

2. Change
From:

let cb = Closure::wrap(Box::new(|time| {
    seed::update(Msg::OnAnimationFrame(time));
}) as Box<FnMut(RequestAnimationFrameTime)>);

To:

let (app, msg_mapper) = (orders.clone_app(), orders.msg_mapper());
let cb = Closure::new(move |time| {
    app.update(msg_mapper(Msg::OnAnimationFrame(time)));
});

orders has a new methods -clone_app and msg_mapper. They allow us to send app instance into closures without magic like seed::update or passing it somehow through Model (it was relatively often requested feature). It's possible thanks to impl Orders because it "hides" App type parameters. So we don't need to derive Serialize & Deserialize and register trigger_update_handler anymore. Note: I also changed Closure::wrap to Closure::new where possible in the whole codebase.

3. Change
From:

enum Msg {
    Init,
...
match msg {
        Msg::Init => {
            orders
                .send_msg(Msg::SetViewportWidth)
                .send_msg(Msg::NextAnimationStep)
                .skip();
        }
...
app.update(Msg::Init);

To:

fn init(_: Url, orders: &mut impl Orders<Msg>) -> Model {
    orders
        .send_msg(Msg::SetViewportWidth)
        .send_msg(Msg::NextAnimationStep);
    Model::default()
}
pub fn render() {
    seed::App::build(init, update, view)

This change removes the need to call app.update and create special handling with Msg::Init. It also allows us to choose the right Model according to Url so we don't have to create a special default Model for the case "I don't know which page to display yet". It's the second breaking change.


Let's look at examples/server_integration/client/src/lib.rs:

4. Change:
From:

Msg::ExampleA(msg) => {
    *orders = call_update(example_a::update, msg, &mut model.example_a)
    .map_message(Msg::ExampleA);
}

To:

Msg::ExampleA(msg) => {
    example_a::update(msg, &mut model.example_a, &mut orders.proxy(Msg::ExampleA));
}

There is another new method in orders: proxy. It removes boilerplate for plumbing sub-modules and allows us to pass orders instead of creating new ones. So we can pass app instance and effect queue through modules. Note: I've removed helper function call_update - it's the third breaking change.


Let's look at src/routing.rs:

5. Change
From:

// The first character being / indicates a rel link, which is what
// we're intercepting.
// todo: Handle other cases that imply a relative link.
// todo: I think not having anything, eg no http/www implies
// todo rel link as well.

To:

// The first character being / or empty href indicates a rel link, which is what
// we're intercepting.
// @TODO: Resolve it properly, see Elm implementation:
// @TODO: https://github.com/elm/browser/blob/9f52d88b424dd12cab391195d5b090dd4639c3b0/src/Elm/Kernel/Browser.js#L157
...
// @TODO should be empty href ignored?
if !href.is_empty() {
    // Route internally based on href's value
    let url = push_route(Url::from(href));
   update(routes(url));
}

The main goal is to don't refresh browser tab on click the element with empty link (i.e. click on <a href=""></a> does nothing.).


Let's look at src/fetch.rs:

6. Change
From:

pub fn new(url: String) -> Self { 
...
pub enum FailReason { 
    RequestError(RequestError),
    Status(Status),
    DataError(DataError),
}
...
SerdeError(Rc<serde_json::Error>), 
...
 T: DeserializeOwned + Debug + 'static

To:

pub fn new(url: impl Into<Cow<'static, str>>) -> Self {     // <--- A
...
// @TODO use https://github.com/rust-lang-nursery/failure?
pub enum FailReason<T> {
    RequestError(RequestError, FetchObject<T>),   // <--- B
    Status(Status, FetchObject<T>),
    DataError(DataError, FetchObject<T>),
}
...
type Json = String;
SerdeError(Rc<serde_json::Error>, Json),    // <--- C
...
T: DeserializeOwned + 'static,          // <--- D

A - more ergonomic API - you can pass url as a String or &'static str
B - if request fails, you want to know why and get more information or e.g. try another deserializer
C - if deserialization fails, you can try it again with another deserializer
D - more ergonomic API - Debug was needed because of ugly code, fixed

Motivation for B+C:
Example - Server sends either status 200 + expected data (like list of blog articles) or 4xx and list of errors. => So we try to parse response data into articles if the status code is 200 or try to parse errors if code is 400 or at least log problems. It's more comfortable with those changes.

4th breaking change.


Let's look at the RealWorld example, because I haven't written example yet. Realworld's lib.rs:

7. Change
From:

// Nothing - it's a new feature.

To:

#[wasm_bindgen(start)]
pub fn start() {
    seed::App::build(init, update, view)
        .routes(|url| Msg::RouteChanged(url.try_into().ok()))
        .sink(sink)      //  <------- new method
        .finish()
        .run();
}

pub enum GMsg {
    RoutePushed(Route<'static>),
    SessionChanged(Session),
}

fn sink<'a>(g_msg: GMsg, model: &mut Model<'a>, orders: &mut impl Orders<Msg<'static>, GMsg>) {
    if let GMsg::RoutePushed(ref route) = g_msg {
        orders.send_msg(Msg::RouteChanged(Some(route.clone())));
    }
   // or classic `match g_msg` like in `Update` function
   ... 

// File: ` seed-rs-realworld/src/route.rs `:
pub fn go_to<Ms: 'static>(route: Route<'static>, orders: &mut impl Orders<Ms, GMsg>) {
    seed::push_route(route.clone());
    orders.send_g_msg(GMsg::RoutePushed(route));
}

It's a new concept for communication among modules. sink it's almost the same like update but you received GMsg ("global message") instead of module's Msg. I'm using it in RealWorld example to notify root module that we want to change page/url (de facto child -> parent communication) or for sending session data into pages (event producer -> event sinks).
How to use it:

  • create enum GMsg
  • associate it with Orders in update, init and sink functions (just add 2nd parameter):
    • &mut impl Orders<Msg, GMsg> (Note: Default global message is ())
  • send messages and perform commands with new Orders methods:
    • orders.send_g_msg(GMsg::Something) or orders.perform_g_cmd(...)
  • or you can use app instance: app.sink(GMsg::Foo).

Smaller changes:

  • Fixed macro md.
  • Changed Closure::wrap to Closure::new except 2 cases in lib.rs: set_interval and set_timeout (Added todo that we should remove them because we are using gloo).
  • Added dyn everywhere (probably same as PR #188).
  • Fixed paths in all examples (see for example examples/animation_frame/index.html. (They was failing with non-root path in url).
  • Fixed default handling for non-existing API in example/server_integration.
  • Added md! and raw! to examples/server_interaction/src/lib.rs so they are tested.
  • Fixed typo in Makefile.toml.
  • Fixed IDE warning by reordering functions in examples/websocket/src/server.rs.
  • Updated example in Readme.md.
@MartinKavik MartinKavik force-pushed the MartinKavik:feat/scalable-application-support branch from 171f3a1 to f86e52b Jul 24, 2019
@MartinKavik

This comment has been minimized.

Copy link
Contributor Author

commented Jul 24, 2019

// Rebased on the current master & force-pushed - conflicts resolved.

@David-OConnor

This comment has been minimized.

Copy link
Owner

commented Jul 25, 2019

Looks like a load of improvements, espcecially towards flexiblity with complex projects. Review in prog.

@MartinKavik

This comment has been minimized.

Copy link
Contributor Author

commented Jul 25, 2019

There shouldn't be any hacks. The most "tricky" part for review will be probably orders.rs - just ask if something isn't clear enough so we know where to add more comments / what should be simpler or more readable.
P.S Sorry for bigger PR, I was continuously changing this code during realworld ex. development so it would be impractical to create many smaller PR in progress.

@David-OConnor David-OConnor merged commit d162ffe into David-OConnor:master Jul 26, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@theduke

This comment has been minimized.

Copy link

commented Jul 29, 2019

Awesome changes, @MartinKavik.

Your real world example really is the mother of all examples.
I'm using very similar patterns in my somewhat large app!

It would be great to reflect the changes here in the documentation page.

  • A sub-page for how to use the model nesting functionalities (proxy, map_message)
  • Documenting init
  • ...
@levalleux-ludo

This comment has been minimized.

Copy link

commented Aug 2, 2019

Hi there,
I've detected a behavioural change when upgrading from seed 0.3.5 to 0.4.0 and I think it comes from this PR.
It's about the routing.

In previous version, if you directly open URL "https://seed-rs.org/changelog", it shows the changelog (as expected, I guess). But since the upgrade, it goes on the home page whatever the URL entered (you can try on Seed webpage). In order to go to changelog, you need to explicitely navigate from the home page, by clicking the ChangeLog link on the top banner.

I've seen the same change in my app and I found a workaround (inspired by seed-rs-realworld app) :
I have had to defined an init() function, that does :
fn init(url: Url, orders: &mut impl Orders) -> Model {
orders.send_msg(routes(url));
Model::default()
}

Now, the behaviour is as expected (ie as before the upgrade).
Hope this feedback helps.
If needed, I can create an issue.

levalleux-ludo added a commit to levalleux-ludo/adex-explorer that referenced this pull request Aug 2, 2019
(necessary since seed 0.4.0 - PR #189 'scalable application support'')
David-OConnor/seed#189 (comment)
levalleux-ludo added a commit to levalleux-ludo/adex-explorer that referenced this pull request Aug 2, 2019
(necessary since seed 0.4.0 - PR #189 'scalable application support'')
David-OConnor/seed#189 (comment)
@daogangtang

This comment has been minimized.

Copy link

commented Sep 23, 2019

Appreciated at this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.