New AppBuilderAPI #290

AlterionX · 2019-11-20T11:52:06Z

Here's an implementation for the new API (#250) that I've been toying around with.

I may have gone a little crazy on the types.... I could have also combined 4 of the fields in the new AppBuilder struct into 2, but it might have been (even more) confusing, so I left it as is.

It should still support everything for the older version, and I haven't marked the old methods as deprecated yet.

I also haven't gotten around to converting the examples yet, but I've been up pretty much all night at this point, so I'll fix it later.

AlterionX · 2019-11-20T11:57:16Z

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

This is what counter looks like now. Also, I think I need to combine build_and_run and build_and_start.

David-OConnor · 2019-11-20T12:39:23Z

Love that - Adding the init system was necessary to make things more flexible, but the existing API had too much boilerplate for default settings.

examples/counter/src/lib.rs

src/vdom.rs

examples/websocket/Cargo.toml

src/vdom.rs

src/vdom/builder.rs

MartinKavik · 2019-11-20T16:18:55Z

Looks good on the first glance. Let me know when you have questions or when it's ready for the second review.

AlterionX · 2019-11-21T01:28:26Z

@MartinKavik I think it's ready for another review!

MartinKavik

Could you update also changelog, please?

examples/user_media/src/lib.rs

src/vdom.rs

src/vdom/builder.rs

src/vdom/builder/after_mount.rs

src/vdom/builder/before_mount.rs

src/vdom.rs

MartinKavik · 2019-11-22T12:27:09Z

@David-OConnor I will be several days without PC - so once the last comments are resolved and you don't want to add more, feel free to merge it.

David-OConnor · 2019-11-24T00:04:10Z

@AlterionX : Do you plan to modify this further? Ready to merge. Overall a solid improvement, both with the after_mount functionality, and the cleaner/reduced-boilerplate API.

AlterionX · 2019-11-24T00:51:18Z

I have a small question about the order in which messages are processed. It's somewhere in the review, but I'll post it again here:

I think I'm going to change [the order in which messages created during initialization are processed]. Right now, I think the order in which effects are processed is fairly confusing. What we have here is as follows:

IntoAfterMount generates a series of Effects, message_q_0.

model is set.

routing function is called and creates a message, message_q_1.

message_q_1 is processed with update. (This is created second!)

Previously generated effects (message_q_0) are called. (This is created first!)

I propose that we simply toss the routing message onto the queue and process them in the order that they are created. This might break things. But I think that this is the order they should be processed in. If a message needs to get passed after routing, then I think that the user should implement it so that the routing message triggers the additional effects.

David-OConnor · 2019-11-25T21:54:23Z

What do you think it'll break? Your proposal sounds good.

AlterionX · 2019-11-26T00:52:52Z

Just a bit afraid to change behavior, really. I just realized that I forgot to push the changes that actually do what I said, but I'd already implemented it. Whoops! Sorry about that.

I'm a bit afraid if people depend on messages being sent from IntoAfterMount having the correct state due to changes triggered by the routing message. I guess I should put this into the changelog as well.

David-OConnor · 2019-11-26T03:11:16Z

Let's not worry about that - if we break things, that's fine. Hopefully will catch anything between the merge and the next release, and if not, no big deal.

src/orders.rs

src/vdom.rs

AlterionX · 2019-11-29T03:01:28Z

@MartinKavik Made the change! Hopefully it's good to go.

MartinKavik · 2019-11-29T11:01:25Z

@AlterionX

This branch cannot be rebased due to conflicts

Please rebase your branch onto updated master, squash commits as it makes sense to you and I'll shoot it to master. Thanks :)

- Implement API (slight use of generics). - Convert examples to `BeforeMount` and `AfterMount` from `Init`. - Isolate one of the bug fixes in a method that's always called: `build`.

- Add deprecation notices to parts of old API. - Convert examples to new API. Add two additional conversions for AfterMount and BeforeMount. - Ensure window listeners are set after model is set. - Fix user_media example.

…rContainer.

…ntainer::append_fron` to `merge`.

MartinKavik · 2019-11-30T22:19:31Z

@AlterionX Good job! Let's find out how it works in the real world :)

David-OConnor · 2019-11-30T22:27:03Z

Are we ready to merge this?

MartinKavik · 2019-11-30T22:29:53Z

@David-OConnor Sorry I was a little bit too fast, but code seems to be ok, no conflicts and tests pass.