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

Performance: Removed watchers and/or increased watchers specificity #1143

Merged
merged 2 commits into from
Dec 1, 2021

Conversation

stackoverfloweth
Copy link
Contributor

@stackoverfloweth stackoverfloweth commented Nov 29, 2021

Watchers can be expensive, especially when the object is a large array, or the watcher is "deep". Even inexpensive watchers can become create performance issues when you're dealing with a large number of the components within a view. This PR outright removes many watchers. Where watchers couldn't be removed, I was still able to increase the watchers specificity. For example, watching $route only to be checking a very specific param within the handler. It's better to just watch the specific param by watching $route.params.tenant for example. Some watchers were replaced with computed variables to follow vue's suggestion that watchers be used for "side-effects of changing data". There are also some cases where I was able to use the watcher API to only temporarily watch a property instead of continuing to watch for the lifetime of the component. Together these changes should help improve performance of the system and also improve the clarity of the intent of watchers within these components.

@zhen0
Copy link
Member

zhen0 commented Dec 1, 2021

Thanks @stackoverfloweth - one small request for the PR itself (no code change needed) - can you add a sentence or two about the benefits of this change?

zhen0
zhen0 previously requested changes Dec 1, 2021
Copy link
Member

@zhen0 zhen0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @stackoverfloweth - one small request for the PR itself (no code change needed) - can you add a sentence or two about the benefits of this change?

@stackoverfloweth stackoverfloweth dismissed zhen0’s stale review December 1, 2021 15:21

let me know if that comment satisfies the requirement

Copy link
Member

@zhen0 zhen0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhen0 zhen0 merged commit 8664459 into master Dec 1, 2021
@zhen0 zhen0 deleted the performance/audit-watchers branch December 1, 2021 16:47
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

Successfully merging this pull request may close these issues.

2 participants