Problem/Motivation
Currently ModuleHandler::alter() invokes alter hook implementations in the theme layer once it has been initialized.
// Allow the theme to alter variables after the theme system has been
// initialized.
global $theme, $base_theme_info;
if (isset($theme)) {
$theme_keys = array();
foreach ($base_theme_info as $base) {
$theme_keys[] = $base->name;
}
$theme_keys[] = $theme;
foreach ($theme_keys as $theme_key) {
$function = $theme_key . '_' . $hook;
if (function_exists($function)) {
$this->alterFunctions[$cid][] = $function;
}
if (isset($extra_types)) {
foreach ($extra_types as $extra_type) {
$function = $theme_key . '_' . $extra_type . '_alter';
if (function_exists($function)) {
$this->alterFunctions[$cid][] = $function;
}
}
}
}
}
This is something that always bothered. If themes are themes and not modules (and also handled differently and not just a special type of module) why do we allow them to implement alter hooks? And if we want them to implement alter hooks why do we make the whole thing so fragile that it is never known whether a given hook is actually safe to implement in the theme or not. There are various cases in which this concept totally fails. Here is just one:
A theme implements hook_system_info_alter() (yes, as silly as that is):
Now, due to the fact that the system info is only built once and then goes into the cache until it gets rebuilt for the next time this can lead to a few interesting scenarios. Imagine you clear the cache of your site while on the backend. The front-end theme implements hook_system_info_alter(). However, since we were on the backend theme during the system info rebuild that hook implementation from our front-end theme is never invoked. Right, so now you got some sort of Russian roulette for your site which causes your site to behave differently based on whether your cache was rebuilt with the front-end theme loaded or not.
The same basically applies to every alter hook that is related to some sort of caching.
Another scenario:
Some random API function, that might be called from anywhere in your code and at any time (e.g. module A might use it in some random preprocess hook and module B uses it in hook_boot()), internally invokes an alter hook. Now, when invoked in hook_boot() it will not invoke the theme-level alter hook implementation as themes are simply not loaded yet during hook_boot(). However, when invoked in that preprocess hook the theme most likely will be initialized and the hook gets invoked successfully leading into two different results. Shit just got real.
Or, probably even worse:
Theme A is active, and we invoke a few alter hooks for it. Suddently, Module B comes in and changes the active theme to Theme C. Now, all further alter hooks are invoked for Theme C. What?
Proposed resolution
So, how do we identify whether an alter hook is appropriate to be implemented by a theme? Simple answer: We can't. At least not adequately. We really need either black or white here. And since we can't give themes the same privileges and not load them as early in the bootstrap as modules (at least in terms of hooks) as they need to be swappable (for example to allow for backend themes) for me the answer would be to completely disallow alter hooks in themes and just remove those lines from ModuleHandler::alter().
Thoughts?
Comments
Comment #1
Mile23This isn't really a question of how ModuleHandler should work, but how the _alter() API works.
See also: #2024083: [META] Improve the extension system (modules, profiles, themes)
Comment #2
fubhy CreditAttribution: fubhy commentedI am pretty sure we want to get rid of hooks (alter or not) in themes entirely. It's race condition hell. Will bring this up during DCon.
Comment #3
filijonka CreditAttribution: filijonka commentedPostponing based on https://www.drupal.org/core/beta-changes
Comment #4
Mile23Beta evaluation block in the original issue please?
Comment #5
David_Rothstein CreditAttribution: David_Rothstein commentedHard to see how this could go into a stable release - isn't it Drupal 9 material at this point?
One thing we could do, of course (even in 8.0.x and 7.x) is document better how alter hooks in themes are supposed to be used... I'm not too familiar with it but my understanding is that the intention was to allow themes to use it to make small visual tweaks to the display of items on the site (which often requires an alter hook to accomplish). If you're doing something crazier than that you're probably misusing the functionality.
Comment #6
Jeff Burnz CreditAttribution: Jeff Burnz commentedIndeed, this is basically impossible in D8 to implement such ideas, at least not without rather major overall of certain API's, since alters are even more ingrained into the theming system than perhaps ever before.
For example, consider this:
With no support for this issue in 1.5 years and a team of people working in the opposite direction, I would call this issue dead in the water. D9 at the earliest, and frankly I won't be supporting it then either, not without a rewrite of the theme system that hands themes comparable abilities via another set of hooks or methods.
Sorry fubhy, but I simply can not get behind such a thing in D8, not at this stage, probably not ever. alters are simply far to useful to themes, which is why we fought the long hard battle to get them in D7 in the first place.
Comment #7
markhalliwellMoved comment over to #2390973: Allow themes to contain companion modules and depend on them.
Comment #8
markhalliwellMoved comment over to #2390973: Allow themes to contain companion modules and depend on them.
Comment #9
markhalliwellComment #10
BerdirWe have multiple issues about this, for example #2390973: Allow themes to contain companion modules and depend on them, see the discussion there. There are more about specific alter hooks I think. We need to merge those issues together.
Comment #11
markhalliwellAh, didn't see that issue... I'll un-repurpose this issue then and move my comments over there.
Comment #12
markhalliwellComment #14
joelpittetUnpostponing since we are well on to 8.2.x