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

Persistence of View/Model data #25

Closed
geom3trik opened this issue Jan 30, 2022 · 4 comments
Closed

Persistence of View/Model data #25

geom3trik opened this issue Jan 30, 2022 · 4 comments

Comments

@geom3trik
Copy link
Collaborator

Currently when a view is built using Self{ }.build2(cx, |cx|{ }) the closure is run every time but the view is not added to the context if it already exists. This means that any changes to fields in Self { } are not updated when the view is rebuilt.

This isn't necessarily a problem because any data that did need to be updated could live in a model which is updated when rebuilt. So when you have some data AppData{ }.build(cx), this is updated with new values every time it is called, so calling it inside a binding updates that data every time the binding is rebuilt.

This whole system presents a few problems. The first is that there are two different places data can live and two different behaviours that can happen when rebuilt:

  1. Data inside a view struct, not updated when rebuilt
  2. Data inside a view struct, updated when rebuilt
  3. Data inside a model, not updated when rebuilt
  4. Data inside a model, updated when rebuilt

Data inside of views is local to the view wheras data inside of models can be shared accross multiple views.

The second problem is that even when data is updated there is no easy access to the previous version of the data without having to get a view/model from context and downcasting it to the right type. This becomes a problem if you want to conditionally update a view/model. A further problem is that sometimes it's necessary to persist model data, resulting in an ugly if cx.data::<AppData>().is_none() check.

I think what is needed is an explicit way for the user to determine if the view/model data should be updated or not if there is already a version in context. I'm not sure of the exact systax yet, but perhaps something like:

cx.build(|cx, prev|{
    if let Some(previous_data) = &prev {
        // Choose to not update or could return Some(Self{ }) which would update it
        return None;
    } else {
        // Create a new one if there's no previous one
        return Some(Self{ }); 
})

This isn't ideal because it shoudn't be possible to not create a new instance when there isn't a previous one but it's a start. I'm very much open to suggestions.

@rhelmot
Copy link
Collaborator

rhelmot commented Jan 30, 2022

How about this for cx.build?

fn build<T, FN: FnOnce(&mut Context) -> T, FU: FnOnce(&mut Context, &mut T)>(&mut self, new_builder: FN, update_builder: FU)

That way, we call new_builder if the data is being constructed for the first time and then insert the return value into the state, and we call update_builder with a mutable reference to the value already in the state if one exists.

@geom3trik
Copy link
Collaborator Author

fn build<T, FN: FnOnce(&mut Context) -> T, FU: FnOnce(&mut Context, &mut T)>(&mut self, new_builder: FN, update_builder: FU)

I do like this as it seems more consistent with the current API. But I think most of the time the update closure will be empty.

rhelmot added a commit to rhelmot/VIZIA that referenced this issue Feb 4, 2022
@geom3trik
Copy link
Collaborator Author

@rhelmot correct me if I'm wrong here but I think that this is no longer an issue thanks to #33?

@rhelmot
Copy link
Collaborator

rhelmot commented Feb 26, 2022

Yes, I believe we have answered this pretty conclusively in the direction of "models and views should never persist through rebuilds".

@rhelmot rhelmot closed this as completed Feb 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants