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

Dashboard - Efficiency tweak #23439

Merged
merged 1 commit into from
May 11, 2022
Merged

Conversation

colemanw
Copy link
Member

Overview

Minor performance boost for Angular-based Dashboard page.

Technical Details

Use static binding for non-changing values.

Use static binding for non-changing values
@civibot
Copy link

civibot bot commented May 11, 2022

(Standard links)

@civibot civibot bot added the master label May 11, 2022
@eileenmcnaughton
Copy link
Contributor

r-run still seems to work - how did you identify the need for this?

@totten
Copy link
Member

totten commented May 11, 2022

Looks sensible to me. I suspect it's identified based on general/systemic preference rather than a specific usage/measurement.

The {{::ts()}} always seems look a good idea. The {{:: $ctrl.foo}} seems like it might be a little more context-dependent? But looks safe in this case. (The dashboard doesn't include any mechanism for editing or refreshing the dashlet metadata.)

@totten totten added the merge ready PR will be merged after a few days if there are no objections label May 11, 2022
@eileenmcnaughton
Copy link
Contributor

@totten so we should always put those :: before ts in angular?

@totten
Copy link
Member

totten commented May 11, 2022

@totten so we should always put those :: before ts in angular?

(Reconsiders) Well... 95% of the time...

The proper question is whether the value will change during the page-view:

  • If the value foo will not change, then output with {{:: foo}}
  • If the value foo will change, then output with {{ foo }}. (This requires more plumbing to watch for changes. It's not necessarily expensive in isolation, but it can add-up if you have hundreds on a page.)

Most ts() expressions will not change -- they're just simple messages like ts('Continue'). But sometimes you have params like ts("Hello %1", {1: name}) which might change. (A quick-and-dirty grep for ts( in ang/**.html suggests the split is roughly 95% vs 5%.) For the 5%, it's a judgment-call.

@eileenmcnaughton eileenmcnaughton merged commit ce0fa37 into civicrm:master May 11, 2022
@eileenmcnaughton eileenmcnaughton deleted the dashboardUpdate branch May 11, 2022 23:15
@colemanw
Copy link
Member Author

Right, the :: at the beginning of the expression tells Angular to evaluate it only once (or, techincally, evaluate it every digest cycle until it resolves to something other than undefined and then stop).

If you omit the :: then Angular will evaluate it every digest cycle, using up the client's cpu needlessly if the value never changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants