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

Mile23’s picture

Title: Find a better home for theme-related code in ModuleHandler::alter() or drop it entirely » Should themes be able to implement hooks, e.g. hook_system_info_alter()?
Issue tags: +API change

This 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)

fubhy’s picture

I 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.

filijonka’s picture

Version: 8.0.x-dev » 8.1.x-dev
Issue summary: View changes
Status: Active » Postponed
Mile23’s picture

Beta evaluation block in the original issue please?

David_Rothstein’s picture

Hard 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.

Jeff Burnz’s picture

Indeed, 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:

  • hook_page_attachments_alter() - the only alter hook that can be currently used to alter a library in themes, and only for libraries added to page. Added recently to D8.
  • hook_css/js_alter - the only way to alter individual css/js assets, in particular js assets, there is no such thing as an info file override for these in themes.
  • hook_theme_suggestions_alter, hook_theme_suggestions_HOOK_alter() - you cannot add suggestions in preprocess anymore, you must use these hook. Added in D8.

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.

markhalliwell’s picture

Title: Should themes be able to implement hooks, e.g. hook_system_info_alter()? » [regression] Themes unable to implement alter hooks, e.g. hook_element_info_alter()
Version: 8.1.x-dev » 8.0.x-dev
Component: other » theme system
Category: Task » Bug report
Priority: Normal » Major
Status: Postponed » Active
markhalliwell’s picture

markhalliwell’s picture

Issue tags: -API change +Regression
Berdir’s picture

We 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.

markhalliwell’s picture

Title: [regression] Themes unable to implement alter hooks, e.g. hook_element_info_alter() » Should themes be able to implement hooks, e.g. hook_system_info_alter()?
Version: 8.0.x-dev » 8.1.x-dev
Category: Bug report » Task
Priority: Major » Normal
Status: Active » Postponed

Ah, didn't see that issue... I'll un-repurpose this issue then and move my comments over there.

markhalliwell’s picture

Issue tags: -Regression +API change

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

joelpittet’s picture

Issue summary: View changes
Status: Postponed » Active

Unpostponing since we are well on to 8.2.x

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.