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

do not call render function when a computed value did not actually change #10344

Closed
indirectlylit opened this issue Aug 4, 2019 · 4 comments
Closed

Comments

@indirectlylit
Copy link

What problem does this feature solve?

Currently, views are re-rendered if any value updates in the dependency tree, even if the actual value passed to the view did not change.

This was originally reported in #7767 and #8540. A fix was attempted in 653aac2 (2.5.17-beta.0) but it was reverted due to regressions.

The current behavior causes performance issues in some situations, such as listening to scroll events and waiting for a certain threshold to be crossed. As described in #8540, work-arounds must be used to avoid excessive re-renders.

What does the proposed API look like?

There would be no change in API.

@posva
Copy link
Member

posva commented Aug 4, 2019

Please always provide a repro when reporting a bug.
If there were regressions you will probably find explanations of the reasoning in those issues

@posva posva closed this as completed Aug 4, 2019
@indirectlylit
Copy link
Author

indirectlylit commented Aug 5, 2019

Understood, I provided repro links in #8540 and there were additional repro links in #7767. I didn't have permission to re-open those issues so I posted this one here.

If there were regressions you will probably find explanations of the reasoning in those issues

Yes: the fix turned out to be much more complicated than originally expected so it was pulled out of the release, and that was fine.

However perhaps you could re-open either #7767 or #8540 since the issue is still extant? They were closed with the expectation that the fix would be released.

@posva
Copy link
Member

posva commented Aug 5, 2019

I see, pasting the actual jsfiddle link and copy pasting instructions is better in that scenario to avoid confusion.

As you already saw in #8446 (comment), this was intentionally left as because it introduced other problems. Everything is said in the thread with the problems this would introduce and workarounds

@indirectlylit
Copy link
Author

this was intentionally left as because it introduced other problems

Okay. My understanding is it was intentionally left out of 2.5.17 because the specific fix caused a regression. If you'd like to say this is a permanent "wont fix", that's your call to make, but I don't believe that was the conclusion from the previous work.

I do believe the behavior is a subtle performance footgun, since computed prop caching might be assumed to be more aggressive than it actually is. The docs say:

computed properties are cached based on their reactive dependencies

The part that's left ambiguous is whether they are cached based only on their direct dependencies, versus indirect dependencies anywhere in the tree.

Thanks for your consideration.

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