Convert template_preprocess and related functions in views_ui

Created on 10 July 2025, about 1 month ago

Problem/Motivation

Steps to reproduce

Proposed resolution

* Move all template_preprocess functions in update module to UpdateThemeHooks, including kook_theme itself
* Deprecate the old functions and move them to update.module, so that the file definitions can be removed
* Update all references, including direct calls in tests and @see/@covers in tests and templates.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

πŸ“Œ Task
Status

Active

Version

11.0 πŸ”₯

Component

views_ui.module

Created by

πŸ‡¨πŸ‡­Switzerland berdir Switzerland

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @berdir
  • Merge request !12688temmplate preprocess conversions β†’ (Closed) created by berdir
  • Pipeline finished with Failed
    about 1 month ago
    Total: 232s
    #544497
  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    Doing the deprecations here feels pretty silly, also for those 3 helper functions that unfortunately aren't prefixed. The chances of those being called by anyone seem pretty much zero.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 584s
    #544505
  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland
  • Pipeline finished with Failed
    about 1 month ago
    Total: 5523s
    #545475
  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    Properly merged with the existing ViewsUIThemeHooks class.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 244s
    #546214
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    seems to need a phpstan update

  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    Fixed that.

  • Pipeline finished with Success
    about 1 month ago
    Total: 765s
    #546413
  • Pipeline finished with Success
    about 1 month ago
    Total: 595s
    #547078
  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    Updated the issue summary it was referencing update.

    I am reviewing this now.

  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    This needs a change record and the cr links need updating for non template preprocess functions and file.

    Other than that I checked each deprecation version is correct and consistent.
    All initial preprocess are set to the correct hook.
    All are __function__
    all deprecated functions seem reasonable

    I have not run the zebra, I will shortly.

    Setting to needs work for CR.

  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    I decided to just link to this issue instead of a CR because there are no known calls to these functions, not even in D7: https://git.drupalcode.org/search?group_id=2&scope=blobs&search=%22views..., there really is no reason to call them.

    I can do a CR, my goal is to avoid unnecessary noise. There are social media channels, feeds and so on, lots of people will at least scan them to see if it affects them. For me, a CR about these functions is just noise, they should have been _ prefixed, then we wouldn't need the BC layer.

  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    I think that's reasonable since it's not used anywhere.

    I've resolved my comments and based on 9 this is ready to go!

  • πŸ‡¬πŸ‡§United Kingdom catch

    Similar question as on πŸ“Œ Convert template preprocess in system.module Active .

    The config factory dependency is only used on one method, so should it be a service closure?

  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    I don't really see a scenario where this could be invoked and ConfigFactory isn't already initialized, at which point using a service closure is just overhead?

    Currently, ConfigFactory is initialized as part of the dependency chain of \Drupal\Core\EventSubscriber\OptionsRequestSubscriber, a prio 1000 request subscriber (through route provider and path processors). This seems like a good candidate for a service closure as it only needs the route provider on OPTIONS requests.

    If I disable that, it's the TimeZoneResolver, a prio 299 request subscriber, that also does the first actual get() call (possibly information that could be put in the container, like language), but there are more actual get() calls.

    \Drupal\Core\Theme\DefaultNegotiator::determineActiveTheme (system.theme)
    \Drupal\Core\Extension\ThemeHandler::listInfo (core.extension, due to theme status check)
    \Drupal\Core\Session\UserRolesAccessPolicy::calculatePermissions (user roles, we removed killed a persistent cache here as that's slower than config get)

    And that's on mostly warm caches, such as the request matching, inbound path processing involves more config for example.Β¨

    system has more services, that's a bit more complicate, I'll check that separately.

    Putting back to RTBC to evaluate that again.

    • catch β†’ committed c3c23def on 11.x
      Issue #3535324 by berdir, nicxvan: Convert template_preprocess and...
  • πŸ‡¬πŸ‡§United Kingdom catch

    No that's a good answer although I'm worried that once we added in more of core and contrib we're going to end up loading a lot of code that's not used.

    Committed/pushed to 11.x, thanks!

Production build 0.71.5 2024