New AppBuilder API

[MartinKavik]

@MartinKavik @David-OConnor I'm starting to get half a mind to move the Init from build to run or something similar to isolate all the side effects inside of run instead of distributing the stateful effects across finish and run.

I think that this is better since we do this weird dance around initial_orders right now where we don't quite have the correct state to do what we have half the state we want inside Builder and half the state inside the App.

So we would have

App::build(update, view).finish().run_with_init(|_, _| {
    // Do things with url + orders.
    // `Init::default` is a wrapper for `Init::new(Model::default())`.
    Init::default()
})

or

App::build(update, view).finish().run() // Calls `Init::default` by default.

It's a bit of a departure from what we currently have, but I think that it makes the implementation a little less weird and the usage clearer about when exactly the InitFn happens (since it seems to run half in finish and half in run right now).

Personally, I also think that mount should also be inside Init since we might want to mount it to a different place depending on the initial route, but we seem to keep that around forever, so I changed my mind. Point is, there are parts of AppCfg that are not immutable, and I would like to change that.

The downsides, I think, are also fairly obvious in that it's a breaking change, and so on. Also, the model is now provided in the run method instead of the build method, which is also... unique, so to speak.

I haven't seen if the types work out yet, but it seems reasonable to me.

Originally posted by @AlterionX in https://github.com/David-OConnor/seed/pull/235#issuecomment-542944604

[MartinKavik]

isolate all the side effects inside of run instead of distributing the stateful effects across finish and run

There are more hidden side-effects in AppBuilder's methods - see e.g. mount.

App::build(update, view).finish().run()

• App::build(update, view).run() would be even nicer.
• I'm not sure if compiler allows you to write ::default() function, because implementation of Default trait for Model is optional.

It's a bit of a departure from what we currently have, but I think that it makes the implementation a little less weird and the usage clearer about when exactly the InitFn happens (since it seems to run half in finish and half in run right now).

I agree. I've added init as the part of the bigger PR (https://github.com/David-OConnor/seed/pull/189). AppBuilder code wasn't super clean before this change and I've made it worse as a trade-off for fewer breaking changes.

mount it to a different place depending on the initial route

Interesting. Do you have a real-world use-case?

---

So I basically agree that builder API can be improved and I've created this issue so we can design it properly. Implementation details can be discussed in original PR (https://github.com/David-OConnor/seed/pull/235).

---

Inspiration from other frameworks:

Yew

[Link](https://github.com/yewstack/yew#architecture-inspired-by-elm-and-redux) ```rust impl Component for Model { // Some details omitted. Explore the examples to see more. type Message = Msg; type Properties = (); fn create(_: Self::Properties, _: ComponentLink) -> Self { Model { } } fn update(&mut self, msg: Self::Message) -> ShouldRender { match msg { Msg::DoIt => { // Update your model on events true } } } fn view(&self) -> Html { html! { // Render your model here { "Click me!" } } } } fn main() { yew::start_app::(); } ```

Percy

[Link](https://github.com/chinedufn/percy#quickstart---getting-your-feet-wet) ```rust #[wasm_bindgen] impl App { #[wasm_bindgen(constructor)] pub fn new () -> App { let start_view = html! {

Hello

}; let window = web_sys::window().unwrap(); let document = window.document().unwrap(); let body = document.body().unwrap(); let mut dom_updater = DomUpdater::new_append_to_mount(start_view, &body); let greetings = "Hello, World!"; let end_view = html! { // Use regular Rust comments within your html

/* Interpolate values using braces */ { greetings } // No need to wrap text in quotation marks (: Click me and check your console

}; dom_updater.update(end_view); App { dom_updater } } } ```

Draco

[Link](https://github.com/utkarshkukreti/draco#overview) ```rust struct HelloWorld; // A Draco application must implement the `draco::App` trait. impl draco::App for HelloWorld { // `Message` is the type of value our HTML will emit. // Here we aren't emitting anything so we use the unit type. // You can put any type here and this example will still compile. type Message = (); // The `view` function returns what we want to display on the page. fn view(&self) -> draco::Node { // `draco::html::h1()` creates an `

` element. draco::html::h1() // `.push()` adds a child to the element. Here we add a Text Node by pushing a string. .push("Hello, world!") // We use `.into()` to convert an `Element` struct to a `Node` struct which this // function must return. .into() } } #[wasm_bindgen(start)] pub fn start() { // We select the first element on the page matching the CSS selector `main` and start the // application on it. draco::start(HelloWorld, draco::select("main").expect("

").into()); } ```

Smithy

[Link](https://github.com/rbalicki2/smithy#a-simple-smithy-app) ```rust #[wasm_bindgen(start)] pub fn start() -> Result<(), wasm_bindgen::JsValue> { let root_element = get_root_element()?; let mut count = 0; let app = smithy::smd!(

I have been clicked {count}{' '}times.

); smithy::mount(Box::new(app), root_element); Ok(()) } fn get_root_element() -> Result { let document = web_sys::window().unwrap().document().unwrap(); document.get_element_by_id("app") .ok_or(wasm_bindgen::JsValue::NULL) } ```

Squark

[Link](https://github.com/rail44/squark#squark-web) ```rust #[derive(Clone, Debug, PartialEq)] struct State { count: isize, } impl State { pub fn new() -> State { State { count: 0 } } } #[derive(Clone, Debug)] enum Action { ChangeCount(isize), } #[derive(Clone, Debug)] struct CounterApp; impl App for CounterApp { type State = State; type Action = Action; fn reducer(&self, mut state: State, action: Action) -> (State, Task) { match action { Action::ChangeCount(c) => { state.count = c; } }; (state, Task::empty()) } fn view(&self, state: State) -> View { let count = state.count; view! {

{ count.to_string() } increment decrement

} } } impl Default for CounterApp { fn default() -> CounterApp { CounterApp } } #[wasm_bindgen] pub fn run() { WebRuntime::::new( window() .unwrap() .document() .expect("Failed to get document") .query_selector("body") .unwrap() .unwrap(), State::new(), ) .run(); } ```

Willow

[Link](https://github.com/sindreij/willow/blob/master/examples/counter/src/app.rs) ```rust #[derive(Debug, Clone, PartialEq, Eq)] pub enum Msg { Increment, Decrement, } #[derive(Debug, Clone)] pub struct Model { counter: i32, } fn init() -> Model { Model { counter: 4 } } fn update(msg: &Msg, model: &mut Model) -> Box> { match msg { Msg::Increment => model.counter += 1, Msg::Decrement => model.counter -= 1, } Box::new(cmd::None) } fn view(model: &Model) -> Html { div( &[], &[ button(&[on_click(Msg::Increment)], &[text("+")]), div(&[], &[text(&model.counter.to_string())]), button(&[on_click(Msg::Decrement)], &[text("-")]), ], ) } pub fn main() -> Program { Program::new(view, update, init()) } ```

Dominator

[Link](https://github.com/Pauan/rust-dominator/blob/master/examples/counter/src/lib.rs) ```rust struct State { counter: Mutable, } impl State { fn new() -> Arc { Arc::new(Self { counter: Mutable::new(0), }) } fn render(state: Arc) -> Dom { // Define CSS styles lazy_static! { static ref ROOT_CLASS: String = class! { .style("display", "inline-block") .style("background-color", "black") .style("padding", "10px") }; static ref TEXT_CLASS: String = class! { .style("color", "white") .style("font-weight", "bold") }; static ref BUTTON_CLASS: String = class! { .style("display", "block") .style("width", "100px") .style("margin", "5px") }; } // Create the DOM nodes html!("div", { .class(&*ROOT_CLASS) .children(&mut [ html!("div", { .class(&*TEXT_CLASS) .text_signal(state.counter.signal().map(|x| format!("Counter: {}", x))) }), html!("button", { .class(&*BUTTON_CLASS) .text("Increase") .event(clone!(state => move |_: events::Click| { // Increment the counter state.counter.replace_with(|x| *x + 1); })) }), html!("button", { .class(&*BUTTON_CLASS) .text("Decrease") .event(clone!(state => move |_: events::Click| { // Decrement the counter state.counter.replace_with(|x| *x - 1); })) }), html!("button", { .class(&*BUTTON_CLASS) .text("Reset") .event(clone!(state => move |_: events::Click| { // Reset the counter to 0 state.counter.set_neq(0); })) }), ]) }) } } #[wasm_bindgen(start)] pub fn main_js() -> Result<(), JsValue> { #[cfg(debug_assertions)] console_error_panic_hook::set_once(); let state = State::new(); dominator::append_dom(&dominator::body(), State::render(state)); Ok(()) } ```

Sauron

[Link](https://github.com/ivanceras/sauron#example) ```rust #[derive(Debug, PartialEq, Clone)] pub enum Msg { Click, } pub struct App { click_count: u32, } impl App { pub fn new() -> Self { App { click_count: 0 } } } impl Component for App { fn view(&self) -> Node { div( vec![class("some-class"), id("some-id"), attr("data-id", 1)], vec![ input( vec![ class("client"), r#type("button"), value("Click me!"), onclick(|_| { sauron::log("Button is clicked"); Msg::Click }), ], vec![], ), text(format!("Clicked: {}", self.click_count)), ], ) } fn update(&mut self, msg: Msg) -> Cmd { sauron::log!("App is updating from msg: {:?}", msg); match msg { Msg::Click => { self.click_count += 1; Cmd::none() } } } } #[wasm_bindgen(start)] pub fn main() { Program::mount_to_body(App::new()); } ```

Elm

[Link](https://package.elm-lang.org/packages/elm/browser/latest/Browser#application) ```elm application : { init : flags -> Url -> Key -> ( model, Cmd msg ) , view : model -> Document msg , update : msg -> model -> ( model, Cmd msg ) , subscriptions : model -> Sub msg , onUrlRequest : UrlRequest -> msg , onUrlChange : Url -> msg } -> Program flags model msg ```

Blazor

[Link](https://github.com/exceptionnotfound/BlazorToDoList/blob/master/BlazorToDoList.App/Program.cs) ```csharp using Microsoft.AspNetCore.Blazor.Hosting; namespace BlazorToDoList.App { public class Program { public static void Main(string[] args) { CreateHostBuilder(args).Build().Run(); } public static IWebAssemblyHostBuilder CreateHostBuilder(string[] args) => BlazorWebAssemblyHost.CreateDefaultBuilder() .UseBlazorStartup(); } } ```

[AlterionX]

There are more hidden side-effects in AppBuilder's methods - see e.g. mount.

I don't think it modifies the DOM, so I didn't consider it a side effect. By side effect, I meant that it doesn't modify any global state (i.e. the DOM). It may panic, but I don't consider that a side effect either.

App::build(update, view).run() would be even nicer.

It shouldn't be difficult to collapse finish and run. At the same time, I'm a bit leery to combine the build and run capabilities. If we did implement this, it would look like this:

fn run(self) {
    self.finish().run()
}

so it shouldn't be a big deal.

I'm not sure if compiler allows you to write ::default() function, because implementation of Default trait for Model is optional.

I have, in fact, implemented Init::default here. I don't think it's a breaking change.

Interesting. Do you have a real-world use-case [for multiple mount points]?

Say that you had a relatively large (mostly static) webpage that had multiple feeds in scattered locations throughout your page. I was thinking that it might be easier to provide a Model with a url for what the feed was listening to and mount two instances of the App with different urls in two different locations. We could simply get around the thing by instantiating "pages" or subsections, but I think that running multiple instances is the way to go as there's not much point in them being connected since the two parts are entirely disjoint.

[MartinKavik]

It may panic, but I don't consider that a side effect either.

It is a problem, because it's surprising. You think that you are only configuring your application but it is calling DOM and can panic under your hands.

I have, in fact, implemented Init::default here. I don't think it's a breaking change.

I tried it and you are right, good.

Say that you had a relatively large (mostly static) webpage that had multiple feeds in scattered locations

Nice example. Have you tried it?

[AlterionX]

Hmm, got it about the about the panic. Well, if we allow for mounting different copies of the App in different places, this would need to go into the Init struct, so I guess it depends on what happens there, I suppose.

Have you tried it?

For the example: no, I have not. I was just exploring the issue mentally. I haven't had a lot of time to code for the past week-ish.

[AlterionX]

Hm, it seems that run is a one-shot method, so I would also suggesting creating a third class between AppBuilder and App that only has the method run if we continue with the build -> finish -> run pattern.

[MartinKavik]

Can we make it simpler? I.e. two classes - App and AppBuilder and start app with App::build(update, view).run()?

[AlterionX]

Hence the "if".

Again, however, this version is simpler but the other is more flexible, so we can always hide the above call with a run implemented on the AppBuilder. In fact, we would simply hide all of the calls with App::run(update, view) if we take it to an extreme.

[MartinKavik]

Let's write a list of options so we can move forward:

1) App::run(update, view)

• + short
• - you can't call methods like routes

2) App::build(update, view).finish().run()

• + you can do some extra configuration in finish
• - need to create another class

3) App::build(update, view).run()

• something between 1) and 2)

4) App::build()
        .update(update)
        .view(view)
        .run()

• + all builder methods are consistent
• - it would probably need to explicitly define types when you call only App::build()
• - maybe too verbose

Can you complete the list (add more variants, add - or +, etc.)?

[AlterionX]

I want to note that this isn't necessarily a list of exclusive options since each option is a strict superset of what's allowed by the previous option apart from 3. We could implement all of these functions and link them to each other in the documentation.

Namely,

App::run(update, view)

is equivalent to

App::build(update, view).run()

with the following impl

impl App {
    fn run(update: _, view: _) {
        App::build(update, view).run()
    }
}

App::build(update, view).run()

is equivalent to

App::build(update, view).finish().run()

with the following impl

impl App {
    fn build(update, view) -> AppBuilder {
        AppBuilder::new(update, view)
    }
}
// -- snip
impl AppBuilder {
    fn finish(self) -> App {
        App::new(...)
    }
    fn run(self) {
        self.finish().run()
    }
}

I don't like this, but

App::build(update, view).finish().run()

is equivalent to

App::builder()
        .update(update)
        .view(view)
        .run()

with the following impl

impl App {
    fn build(update, view) -> AppBuilder {
        App::builder()
            .update(update)
            .view(view)
    }
}
// -- snip
impl AppBuilder {
    fn build(self) -> App {
        App::new(...)
    }
    fn run(self) {
        self.build().run()
    }
}

I'm going to name each of those:

1. App::run(update, view) single function run
   • + This is fairly useful for any MPA with no sinks as they have no need for routes.
2. App::build(update, view).finish().run() build then run
   • +- Separates the notion of actually creating the application and running the application. This is useful if you need to manipulate the thing between build and run, but not useful otherwise.
   • - Function names are a bit cryptic (as is). See note at bottom regarding these names in general.
     • build doesn't actually build, but starts the building process
     • finish finishes the building process, but also partially starts running the App
     • run behaves as expected
   • +- Has an extra stage between building the App and running the App This potentially aids in disallowing illegal configurations.
   • I'm personally opposed to having finish take anything as an argument.
3. App::build(update, view).run() build and run
   • +- Conflates the notion of actually creating the application and running the application. This is useful if you don't need to manipulate the thing between build and run, but not useful otherwise.
   • - Prevents me from passing around just an App -- I need to pass around an AppBuilder if I want to start running the App at a later time (i.e. initial http(REST) api calls to get state)
   • +Has no break between legal and illegal state, same as the build then run method and single function run method.
4. App::build().update(_).view(_).run() spread builder
   • Same pros/cons as the build and run method.
   • - Allows for missing update and view -- what does this mean? Does it simply panic? It might be difficult to capture this at compile time.
   • - Fundamentally equivalent to the build and run approach, but allows for bad state.

Note: I don't like the name of the function build since we aren't building the app, but rather creating a builder. Actually, the typical names I've seen for the Builder pattern is builder then build instead of build then finish. Examples: typed-builder and derive_builder

Also, I would like to clarify: what about the originally suggested change, where run takes a lambda to create the Init struct instead of it being passed into the build function? Is that accepted, or still under discussion?

[David-OConnor]

I love the short API App::run(update, view), with the ability to expand for flexibility, as-required.

[MartinKavik]

1. App::builder(update, view).build().run()
   • I suggest to make it the main ("official") way to to start app - function naming is better, it's easy to call more builder methods and it's basically the same like the current API.
2. App::run(update, view)
   • What are advantages over (1.)? (Except the expression length.)
   • Is it possible to have a function and method with the same name? (App::run(update, view) and App::run(self))

3. run takes a lambda to create the Init struct instead of it being passed into the build function

   • Could you provide some real-world examples where it's better for Seed users or where we can't use builder's init?

[AlterionX]

I agree with the points on 1.

With regards to App::run(update, view), if you don't need sink, or routes (aka it's not a single page app), and you don't do anything fancy with the mount or the window, this is sufficient.

As for the run duplication issue, we can just come up with a different name. If we go with a three struct sequence (something like AppBuilder -> AppRunner -> App), this shouldn't be a concern as run is implemented on AppRunner and App separately.

As for init, again, the problem is how we need to implement build (or finish as it is called). The init function is not run when the app is started, but rather on calling build which is very unexpected. Not to mention that the App's state is inconsistent with when update or view is called. This is the primary concern I have for sticking the init function into the AppBuilder. It's not used anywhere other than in the run function, so I don't think it's necessary to initialize it in the builder. Not to mention that we can then remove two fields in AppCfg that are never used again after run. It's just that I want to isolate the state to where it's being used instead of passing it through.

I ran into this because I had assumed that the init function would get called prior to anything else the App did, but I ended up having to chain a future outside of Seed and initialize init due to various interactions between my update, run, and init functions.

[MartinKavik]

App::run(update, view)

• I just don't see user pain which we want to resolve with this API. So I think that adding this as a trade-off for more code (= more potential bugs, bigger WASM size), larger public API to maintain and a little bit more inconsistent app start among examples / projects on Seed is not good idea.

As for init, again, the problem is how we need to implement build

• I don't care about implementation now if it looks at least doable. I'm trying to find best design (i.e. find real-world examples) from the user point of view, we can discuss implementation in PR.

@David-OConnor @flosse could you share your opinions, please?

[flosse]

@MartinKavik I had not time to read all the comments.

• I don't care about implementation now if it looks at least doable. I'm trying to find best design (i.e. find real-world examples) from the user point of view, we can discuss implementation in PR.

I totally agree!

And I'd split the API at least into a builder and a final App. Moreover I don't like the name init because I have no idea what it means. I'd prefer a more precise naming like before_mounted or after_mounted etc.

Without really thinking about all the point, I'd draft the API like this:

App::new(update, view)        // create a builder
  .routes(my_routes)          // configure whatever you like
  .before_mounted(before_mnt) // declare what should happen but don't do it now
  .after_mounted(after_mnt)   // again, declare but execute it later
  .build()                    // build the instance
  .run();                     // run it

[MartinKavik]

More drafts to discuss.

Minimal example:

App::builder(update, view).build_and_run();

Example with all builder methods:

App::builder(update, view)
    .routes(routes)
    .window_events(window_events)
    .sink(sink)
    .before_mount(before_mount)
    .after_mount(after_mount)
    .build_and_run();

fn before_mount(_url: Url) -> BeforeMount {
    BeforeMount::new()
        .mount_type(MountType::Overtake)
        .mount_point("application")
}

fn after_mount(_url: Url, _orders: &mut impl Orders<Msg>) -> AfterMount<Model> {
    AfterMount::new()
        .model(Model { count: 5 })
        .url_handling(UrlHandling::None)
}

Changes:

• ::builder and .build_and_run to use more standard naming as @AlterionX suggested.
• build_and_run to prevent using App in non-initialized state (e.g. it panics on app.update(..)).
• before_mount and after_mount to better express purpose as @flosse suggested.
• before_mount is a combination of .mount and .mount_type so we can set it when the app is started as @AlterionX suggested.
• after_mount is renamed init.

[AlterionX]

I like this one. The before_mount and after_mount is super nice, imo.

We could attempt leaving in the older methods, but just deprecate the ones that are no longer useful as well.

[MartinKavik]

@David-OConnor Is that API design ok with you?

[MartinKavik]

@David-OConnor: I'm looking at the builder pattern as an implementation detail that allows some details (routes, before_mount etc) to be optional, and start (or something else) is simple, vice a formal pattern. Ie someone not familiar with the builder pattern may be confused (What does build vs builder vs run do?), when the purpose of this code is mainly to specify which init, update, and view fns to use. (While letting you include optional ones) (https://github.com/David-OConnor/seed/pull/275#issuecomment-549037009)

I don't think that the builder pattern is an implementation detail in this case because it's the part of public API. When I open AppBuilder docs and see method start I would be probably confused - does it mean start building or start App?

[MartinKavik]

start, build_and_run, run.. Another alternative could be start_app. Other ideas/opinions please (see previous comment)? @flosse , @AlterionX

[AlterionX]

Seems like we've already settled on build_and_run? A bit posthumous, but personally a fan since it doesn't stray from my knowledge of builders.

@David-OConnor Do you have more input regarding the API? Or is this okay?

[MartinKavik]

@AlterionX final version is https://github.com/David-OConnor/seed/blob/5ce6379c9affc3c65d204c91221f20f08f7b6069/src/vdom/builder.rs#L220

[AlterionX]

Oh, my bad. Read that wrong.

Don't we still have before_mount and after_mount though?

[David-OConnor]

Yep - those aren't implemented, but look good. I have no additions.

[AlterionX]

I think we can close this. All the changes have been implemented.