(dev/core#5243) mixin/smarty - Reduce expensive calls to getTemplateDir()/setTemplateDir()
#30281
Conversation
…mplateDir()` on S3/S4/S5
|
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
eileenmcnaughton
changed the title
getTemplateDir()/`setTe…getTemplateDir()/setTemplateDir()
|
@totten I'm happy with this based on my testing - technically the PR is showing as 'mine' but it's your commit - I'll give it merge-ready now but hopefully only briefly as I'm wanting to deploy the updated rc tarball with this & some other Smarty patches (I will check their merge status) |
eileenmcnaughton
added
the
merge ready
|
I'm gonna plug this into a civix-backport and make sure that works. |
|
Did some more testing on civix backport. Tldr: Seems good. (Full details are listed on that PR.) Since all of this started with civicrm/org.civicrm.contactlayout#143, I decided to go back retest that. Seems good. (Specifically, I started on 5.73.1 and confirmed the original "Summary"<=>"1" problem -- then copied over the proposed version of |
|
it's like magic |
|
Merging as that is what I think the intent of the above comment was |
| // Add the dir if not already present | ||
| if (!in_array($dir, $templateDirs, TRUE)) { | ||
| array_unshift($templateDirs, $dir); |
There was a problem hiding this comment.
This uniqueness check seems to be missing from the new version...
Overview
Follow-up to #30148 to address https://lab.civicrm.org/dev/core/-/issues/5243 identified by @eileenmcnaughton. (This is one possible resolution - suggested by @totten.)
Before
The
smartymixin registershook_civicrm_config-- with one listener on behalf of each extension. On Smarty 3/4/5, this listener callsgetTemplateDir(), prepends its own template-dir, and callssetTemplateDir().Everytime an extension calls
getTemplateDir(), it forces a full (re)normalization of the directory-list (to sync-up with the prior addition)... which examines all the directories that we're previously registered.After
In effect, this is a refactoring that moves the per-extension listeners from
hook_civicrm_configto an internal event (civi.smarty-v2.addPaths.XXX). Specifically:The
smartymixin registers forcivi.smarty-v2.addPaths.XXX(array &$dirs)-- one listener on behalf of each extension. Each listener prepends/enqueues its own extension (array_unshift($dirs, $dir)).The
smartymixin registers a single listener forhook_civicrm_config. It serves as an adapter. It fires the internal event (civi.smarty-v2.addPaths.XXX(array &$dirs)), gets a list of all template dirs, and applies one update to$smarty(getTemplateDir()+array_merge()+setTemplateDir()).You only need to (re)normalize the directory-list one time.