(REF) CRM_Core_Resources - Move hook declaration from addCoreResources() to Container.php #14008
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s() to Container.php
tldr: It's easier to declare `hook_civicrm_buildAsset` listeners at a high-level.
Asset building can use two modes -- production mode writes a static file to
disk when it's being reference. Debug mode just generates a URL for a
web-service (which in turn dynamically renders the content in a separate
page-view).
If the only mode were production mode, then the code would be on pretty
solid ground. We could even simplify things a lot by changing the
AssetBuilder contract to replace the hooks with callbacks, as in:
```php
Civi::service('asset_builder')->getUrl('crm-menu.css', function() {
return '...the css code...';
});
```
Why have a hook? Because hooks are generally context-free and
always-available. If we use debug-mode (or if we add a feature to warm-up
the caches during deployment), then we'll want to fire that hook from a
different context (e.g. web-service or CLI), and the hook-listener needs to
be available in those other contexts.
It would be nice if we could declare hooks generally without needing to edit
the `Container.php` mega-file (e.g. maybe some kind of annotation). But,
for the moment, I think this is the best spot that we have in `civicrm-core`
for ensuring that hook listeners are fully/consistently registered.
|
(Standard links)
|
|
THAT's where it goes! Thanks @totten |
|
unrelated fail |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
It's easier to declare
hook_civicrm_buildAssetlisteners at a high-level.Before
Hook listener declared in-situ (near where it's typically used)
After
Hook listener declared in central way (so it's always defined).
Technical Details
Asset building can use two modes -- production mode writes a static file to disk upfront (when the file is being referenced). Debug mode just generates a URL for a web-service (which in turn dynamically renders the content in a separate page-view).
If the only mode were production mode, then the previous code would be on pretty solid ground. We could even simplify things a lot by changing the AssetBuilder contract to replace the hooks with callbacks, as in:
Why have a hook? Because hooks are generally context-free and always-available. If we use debug-mode (or if we add a feature to warm-up the caches during deployment), then we'll want to fire that hook from a different context (e.g. web-service or CLI), and the hook-listener needs to be available in those other contexts.
It would be nice if we could declare hooks generally without needing to edit the
Container.phpmega-file (e.g. maybe some kind of annotation). But, for the moment, I think this is the best spot that we have incivicrm-corefor ensuring that hook listeners are fully/consistently registered.