seed-rs / seed

A Rust framework for creating web apps
MIT License
3.8k stars 155 forks source link

Option to update elements by key (ELM's Html.Keyed). #354

Closed akhilman closed 4 years ago

akhilman commented 4 years ago

As I can see from src/virtual_dom/patch.rs, the seed's virtual DOM module patches the elements in order. This means that I cannot insert an element at the top of the list of elements without updating the rest of the elements. The problem will be visible if CSS animation is applied.

ELM and Hybrids provide a special key attribute for each item to find matching new and old items.

https://guide.elm-lang.org/optimization/keyed.html https://hybrids.js.org/template-engine/iteration#keys

David-OConnor commented 4 years ago

React also uses keys. We were trying to avoid using them, since they don't quite fit with the declarative model: They're a user-specified tool to enhance performance, but don't assist in describing what to display. Ie they're a compromise. Perhaps we should add them as an option; I tried that a while back, but abandoned it for a reason I can't recall.

akhilman commented 4 years ago

Here is example. anim

MartinKavik commented 4 years ago

@akhilman Could you provide a link to your example? (So we can run in it on our machines)

akhilman commented 4 years ago

https://github.com/akhilman/seed-transition-test <- source code.

MartinKavik commented 4 years ago

Unfortunately I'll focus on Seeder so I don't have time to implement it now properly.

But I've created the basic API for it (see referenced PR above). If you or somebody else want to continue - we would be glad. Implementation don't have to be super clean because the whole patching should be refactored in the future (https://github.com/seed-rs/seed/issues/293).

akhilman commented 4 years ago

https://github.com/seed-rs/seed/pull/356#issue-376238309

354

 circle![
    el_key(&key),   // key implements ToString
    attrs![
        At::Cx => 15.0 + x,
        At::Cy => 15.0 + y,

Could we have some flag for the parent element as well? I have to consume old nodes iterator to split old nodes to keyed and unkeyed and do lookup in hashmap for keyed nodes. It might be slower then current approach.

MartinKavik commented 4 years ago

Could we have some flag for the parent element as well?

If you mean to add a marker/flag to element in a similar way like we added el_key - please don't do that. The main focus is on the user API and it sounds like implementation leak to public API because of patching algorithm problems. And it would be error-prone.

Implement it, please, in the simplest way - it doesn't have to be super fast. I want to refactor patching in the future. And I've created a benchmark for Seed (https://github.com/krausest/js-framework-benchmark/pull/696) and Seed is faster than Yew (on my machine) so DX is more important than optimizations now. Thanks!

akhilman commented 4 years ago

Ok. Thanks for very beautiful code.

akhilman commented 4 years ago

Done by #357