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

Wasted Rendering #1181

Closed
zeng450026937 opened this issue May 14, 2020 · 9 comments
Closed

Wasted Rendering #1181

zeng450026937 opened this issue May 14, 2020 · 9 comments

Comments

@zeng450026937
Copy link

Version

3.0.0-beta.12

Reproduction link

https://jsfiddle.net/450026937/whdtnqgk/32/

Steps to reproduce

  1. Have child component with props declaration, but without using it in render function
  2. Have parent component using it with props binding, like :key="value"
  3. Update the "value" in parent component, and will trigger parent component update

What is expected?

Parent component update without calling child component's render function, as child component has it's own render effect.

What is actually happening?

Child component's render function is also invoked, but nothing changed with the node tree.


After looking into the source code, maybe we should split out the processing of parent update & self update in setupRenderEffect().
Making attrs shallow reactive like props will be a good idea, which allow just update child component's attrs & props(also fallthroughAttrs) during parent component update, and leaving child component trigger it's update if needed.
If this is not possible, an additional lifecycle hook, "shouldUpdateCompoent" should be needed to avoid wasted rendering.

@yyx990803
Copy link
Member

Why would a component declare a prop and then never use it? Why is optimization even needed for this? Sounds useless to me.

@zeng450026937
Copy link
Author

Declare a prop here is for demonstrate only.
Fallthrough attrs changing cause child component re-render too.

https://jsfiddle.net/450026937/whdtnqgk/39/

@zeng450026937
Copy link
Author

Please kindly let me known why this optimization is unnecessary.
In my experience, vue component is design to having it's own render scope, which is triggered by an render watcher.
But changing the fallthrough attrs of the component, also makes re-rendering triggered, which should be an wasted rending, and that is confusing me.
It is designed on purpose?

@yyx990803
Copy link
Member

@zeng450026937 fallthrough should cause the child to update, because it may get applied to the child root regardless of what the child's own render function uses.

@zeng450026937
Copy link
Author

@yyx990803 Because fallthrough attrs is considered has no effect to child component, why not just use the previous vnode tree and patch it with new fallthrough attrs, but calling child render function to get a completely same vnode tree?

@zeng450026937
Copy link
Author

If child component render a large list, which should be a noticeable impact.

@yyx990803
Copy link
Member

  1. Fallthrough attributes are static most of the time.

  2. Fallthrough attributes need to be properly merged with the component's own bindings, e.g. there is a class binding falling through AND the component itself also has a class binding. When a parent is updating a child, it's possible that the child's own state has changed in the same tick, which changed the class binding on its own root. Simply updating from parent and skipping the child won't account for the merge. To ensure correctness, updating the child when props have changed is the reasonable expectation to make.

When you work on a framework you need to understand the trade-offs: premature optimizations often end up in gains that are simply too small to justify the extra complexity and edge cases they create.

@450026937
Copy link

In fact, class/style falling through is the common scenario when writing a component.

For example, if we are performing a tweening animation on child component, that would be unacceptable.
Holding ref of target element can make things happy, but another scheduler is needed for separating read/write tasks, while that's what v-dom doing.

In addition, define a component without explicit prop declarations, all attrs are considered as props, which result like a component declare a prop but never use it.

When user who is simply passing through all props/attrs to child component, that also lead another component re-render again.

If someone who is build a component library, unnecessary re-render is quietly a problem, and there’s no way to optimize this behavior currently.

@yyx990803
Copy link
Member

For example, if we are performing a tweening animation on child component, that would be unacceptable.

The basic assumption is that if a parent passes a dynamic prop to a child, then the child is expected to update when that prop changes. That's how parent-child updates work. Even a React PureComponent updates if any props change, no matter if it's used. If you want to do some expensive tweening with style bindings, you should wrap the child with a div and do the tweening on that div instead.

In addition, define a component without explicit prop declarations, all attrs are considered as props, which result like a component declare a prop but never use it.

  1. This is no longer the case for stateful components. Only functional components without prop declarations have this behavior.

  2. A component with optional props should expect to update on any props change. The parent doesn't know if the prop is just fallthrough to the root or may be used deep in the tree.


This is simply a correctness over maximum optimization trade-off - as I said, it's not worth it. If you want to fix it, be my guest, but I'm not going to waste time on that.

@vuejs vuejs locked as resolved and limited conversation to collaborators May 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants