-
Notifications
You must be signed in to change notification settings - Fork 156
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
New AppBuilder API #250
Comments
There are more hidden side-effects in
I agree. I've added
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 (#235). Inspiration from other frameworks: Yewimpl Component for Model {
// Some details omitted. Explore the examples to see more.
type Message = Msg;
type Properties = ();
fn create(_: Self::Properties, _: ComponentLink<Self>) -> Self {
Model { }
}
fn update(&mut self, msg: Self::Message) -> ShouldRender {
match msg {
Msg::DoIt => {
// Update your model on events
true
}
}
}
fn view(&self) -> Html<Self> {
html! {
// Render your model here
<button onclick=|_| Msg::DoIt>{ "Click me!" }</button>
}
}
}
fn main() {
yew::start_app::<Model>();
} Percy#[wasm_bindgen]
impl App {
#[wasm_bindgen(constructor)]
pub fn new () -> App {
let start_view = html! { <div> Hello </div> };
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
<div class="big blue">
/* Interpolate values using braces */
<strong>{ greetings }</strong>
<button
class=MY_COMPONENT_CSS
onclick=|_event: MouseEvent| {
web_sys::console::log_1(&"Button Clicked!".into());
}
>
// No need to wrap text in quotation marks (:
Click me and check your console
</button>
</div>
};
dom_updater.update(end_view);
App { dom_updater }
}
} Dracostruct 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<Self::Message> {
// `draco::html::h1()` creates an `<h1>` 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("<main>").into());
} Smithy#[wasm_bindgen(start)]
pub fn start() -> Result<(), wasm_bindgen::JsValue> {
let root_element = get_root_element()?;
let mut count = 0;
let app = smithy::smd!(
<div on_click={|_| count = count + 1}>
I have been clicked {count}{' '}times.
</div>
);
smithy::mount(Box::new(app), root_element);
Ok(())
}
fn get_root_element() -> Result<web_sys::Element, wasm_bindgen::JsValue> {
let document = web_sys::window().unwrap().document().unwrap();
document.get_element_by_id("app")
.ok_or(wasm_bindgen::JsValue::NULL)
} Squark#[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<Action>) {
match action {
Action::ChangeCount(c) => {
state.count = c;
}
};
(state, Task::empty())
}
fn view(&self, state: State) -> View<Action> {
let count = state.count;
view! {
<div>
{ count.to_string() }
<button onclick={ move |_| Some(Action::ChangeCount(count.clone() + 1)) }>
increment
</button>
<button onclick={ move |_| Some(Action::ChangeCount(count - 1)) }>
decrement
</button>
</div>
}
}
}
impl Default for CounterApp {
fn default() -> CounterApp {
CounterApp
}
}
#[wasm_bindgen]
pub fn run() {
WebRuntime::<CounterApp>::new(
window()
.unwrap()
.document()
.expect("Failed to get document")
.query_selector("body")
.unwrap()
.unwrap(),
State::new(),
)
.run();
} Willow#[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<Cmd<Msg>> {
match msg {
Msg::Increment => model.counter += 1,
Msg::Decrement => model.counter -= 1,
}
Box::new(cmd::None)
}
fn view(model: &Model) -> Html<Msg> {
div(
&[],
&[
button(&[on_click(Msg::Increment)], &[text("+")]),
div(&[], &[text(&model.counter.to_string())]),
button(&[on_click(Msg::Decrement)], &[text("-")]),
],
)
}
pub fn main() -> Program<Model, Msg> {
Program::new(view, update, init())
} Dominatorstruct State {
counter: Mutable<i32>,
}
impl State {
fn new() -> Arc<Self> {
Arc::new(Self {
counter: Mutable::new(0),
})
}
fn render(state: Arc<Self>) -> 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#[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<Msg> for App {
fn view(&self) -> Node<Msg> {
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<Self, Msg> {
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());
} Elmapplication :
{ 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 Blazorusing 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<Startup>();
}
} |
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.
It shouldn't be difficult to collapse fn run(self) {
self.finish().run()
} so it shouldn't be a big deal.
I have, in fact, implemented
Say that you had a relatively large (mostly static) webpage that had multiple |
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 tried it and you are right, good.
Nice example. Have you tried it? |
Hmm, got it about the about the panic. Well, if we allow for mounting different copies of the
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. |
Hm, it seems that |
Can we make it simpler? I.e. two classes - |
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 |
Let's write a list of options so we can move forward: 1) App::run(update, view)
2) App::build(update, view).finish().run()
3) App::build(update, view).run()
4) App::build()
.update(update)
.view(view)
.run()
Can you complete the list (add more variants, add - or +, etc.)? |
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:
Note: I don't like the name of the function Also, I would like to clarify: what about the originally suggested change, where |
I love the short API |
|
I agree with the points on 1. With regards to As for the As for I ran into this because I had assumed that the |
@David-OConnor @flosse could you share your opinions, please? |
@MartinKavik I had not time to read all the comments.
I totally agree! And I'd split the API at least into a builder and a final 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 |
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:
|
I like this one. The We could attempt leaving in the older methods, but just deprecate the ones that are no longer useful as well. |
@David-OConnor Is that API design ok with you? |
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 |
|
Seems like we've already settled on @David-OConnor Do you have more input regarding the API? Or is this okay? |
Oh, my bad. Read that wrong. Don't we still have |
Yep - those aren't implemented, but look good. I have no additions. |
I think we can close this. All the changes have been implemented. |
@MartinKavik @David-OConnor I'm starting to get half a mind to move the
Init
frombuild
torun
or something similar to isolate all the side effects inside ofrun
instead of distributing the stateful effects acrossfinish
andrun
.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 insideBuilder
and half the state inside theApp
.So we would have
or
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 infinish
and half inrun
right now).Personally, I also think that
mount
should also be insideInit
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 ofAppCfg
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 thebuild
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 #235 (comment)
The text was updated successfully, but these errors were encountered: