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

Re-enable Diffing #1

Closed
StarArawn opened this issue Dec 7, 2021 · 7 comments
Closed

Re-enable Diffing #1

StarArawn opened this issue Dec 7, 2021 · 7 comments
Labels
bug Something isn't working

Comments

@StarArawn
Copy link
Owner

StarArawn commented Dec 7, 2021

Issue

Currently diffing is turned off. This is because of the following problem:

A user creates Widgets A > B > C where C is a child of B and B is a child of A.

Widget A state changes and passes new props to C but not to B. Widget A gets marked as dirty and is re-rendered.

When re-rendering the diff for A says to re-render my children. So it re-renders B. B does a diff check and nothing has changed so it will not re-render C even though C has changed.

Possible solutions:

Always attempt to re-render all children if parent has been marked dirty.
Pros:

  • Easy to implement

Cons:

  • We should attempt to re-render B in this scenario. Performance will suffer for larger situations.

Diff props to children separately in parent and build a re-render list.
Pros:

  • Faster re-rendering

Cons:

  • Harder to implement.
  • Requires a list of props that are passed to child widgets list. Not easy to get.

Others?

@StarArawn StarArawn added the bug Something isn't working label Dec 7, 2021
@Type1J
Copy link

Type1J commented Dec 20, 2021

The second solution is better. I'm not sure why the props are "Not easy to get.". Please explain.

@StarArawn
Copy link
Owner Author

StarArawn commented Dec 20, 2021

The second solution is better. I'm not sure why the props are "Not easy to get.". Please explain.

I think I've come up with a third better solution that will work internally to the proc macros.

  1. Whenever someone writes a "prop" such as: <Text content={some_value} /> that prop is wrapped in a Binding. Internally a widget struct will look something like:
pub struct MyWidget {
  some_prop: Binding<i32>,
}

We then bind that prop to the widget that is using it. We can then completely ignore diffing and we can also detect what widget changed because x prop changed. Children of X parent will never re-render unless the props change.

@MrGVSV
Copy link
Collaborator

MrGVSV commented Dec 23, 2021

Children of X parent will never re-render unless the props change.

What about layout? Will they re-render if the layout of the parent or a sibling changes?

@StarArawn
Copy link
Owner Author

StarArawn commented Dec 23, 2021

What about layout? Will they re-render if the layout of the parent or a sibling changes?

Layout is calculated for all widgets. Morphorm doesn't have a way of handling partial updates yet. I think its complicated because updating a child cascades upwards and then back down. So typically its better to just update everything.

@StyMaar
Copy link

StyMaar commented Feb 9, 2022

I see this issue and I can't help thinking about this video I've seen recently by the author of Svelte. Svelte eschew the diffing problem by not having a virtual DOM altogether and relies on a compile-time code generation instead.

I don't know if you're aware of Svelte approach, and if you purposely decidedly not to go in this direction, or if it's something you weren't aware before and it's something that could inspire you.

@StarArawn
Copy link
Owner Author

I see this issue and I can't help thinking about this video I've seen recently by the author of Svelte. Svelte eschew the diffing problem by not having a virtual DOM altogether and relies on a compile-time code generation instead.

I don't know if you're aware of Svelte approach, and if you purposely decidedly not to go in this direction, or if it's something you weren't aware before and it's something that could inspire you.

We don't really have a virtual DOM per say because there is no HTML rendering. Instead we build out rendering primitives and those are extracted into draw calls. When we talk about diffing here we are talking about how we update those rendering primitives when widgets change(being reactive). We've recently made big changes in how we handle props and context and I think its going to let us be more flexible here. Looking at Svelte we are already doing some of what svelte does. :)

@StarArawn
Copy link
Owner Author

Fixed with the rewrite! 👍 Widget are properly diffed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants