Make it required to specify variables passed to templates

I saw a comment on another issue that mentioned something about not adding variables to templates in preprocess. That's a very common task, so maybe I misunderstood that. Would there be a way to add additional variables to the registry in preprocess?

I can definitely see encouraging or requiring that the module that defines a particular theme hook should specify the variables it needs for the template either in the the info.yml hook_theme equivalent or in a render element. I guess I'm more talking about themes, particular themes for a client site, that are working with a pre-existing theme hook. Preprocess is very often used to add in some additional variables for a special case that wasn't envisioned in the original theme hook. To me, the point of preprocess functions for themes is to modify the values of existing variables in a theme hook or to add new ones. It's fairly common to do the same thing in custom modules. I don't think either of those is a bad practice that should be discouraged. If it needs to be done in a slightly different way, so that it's easier to understand what variables are defined for a particular template in a particular situation, I think that's fine. I just want to make sure we preserve the basic ability to modify and add to template variables through the theme system.

laurii said:

Theme system is in place just to allow last minute preprocessing on theme. In most cases, you shouldn't preprocess something you are creating yourself.

I agree - but it is important to note that this wouldn't just be a small change - the scope would be massive. I can think of a few core modules off the top of my head that would need to be redone, for example the responsive image module which spends its preprocess cycles (incorrectly IMHO) loading image styles and tossing in huge multi-tiered arrays of responsive image variables. Not to mention the node module etc.

For the responsive image module, to me it would be much better to relegate most of that functionality to the Twig templates themselves, with the exception of loading the Image style which should happen before rendering anyways. I think this could be applied to other core modules as well.

mdrummond said:

To me, the point of preprocess functions for themes is to modify the values of existing variables in a theme hook or to add new ones. It's fairly common to do the same thing in custom modules. I don't think either of those is a bad practice that should be discouraged.

I respectfully disagree - adding variables sounds like bad practice to me because it introduces yet another source of theme context. In my experience, as a developer you want a "single source of truth" as much as possible. In this case, it would be the theme hook. I think in a perfect world scenario, keeping the preprocess hooks as "the method" for modifying output variables with functions not readily available in Twig wouldn't be horrible, even if I dislike it.

I think having contextual module add a contextual_links variable would be a lot better than sticking a variable in title_suffix, particularly since Contextual is a default, core module. For optional modules, or contrib modules, that's not really an option as a way to affect a template, unless you ask people to manually modify their templates to include the variable, which would be hard to rely on.

I get the theoretical point here, that it would be nice to have a handy place to look to see all the variables. I don't think preventing addition of variables is the only way to achieve that goal. If there needs to be a standardized way to add a variable to the registry for a theme hook, or something like that, I could see that working. But adding variables in preprocess for use in templates is incredibly common and very useful. It's not a bad practice, it's a good one. Preprocess logic is great for manipulating data. Template logic is great for working with classes, looping through variables, deciding whether to show a certain block of markup or not. But manipulating data in templates can run into a lot of problems.

Core itself does a ton of adding variables through preprocess, contrib does too, and it's very, very, very common on every client project I have ever worked on. Changing that would be really disruptive to how Drupal theming currently works, and quite frankly I think it would make Drupal a lot less extensible.

I don't want to have to register a new theme hook for node_that_has_one_extra_variable, figure out how to pass all the data from a node render array into a new render array with the new theme hook, just so I can add an extra variable to my node template.

So I think we really need to look at alternative options to solve the good goal of making it easier to see what variables you can use in a template.

I don't think this sounds like an actual problem that needs to be solved. You're suggesting that we take away something many people find useful and that many projects use for very little real benefit as far as I can tell.

The vast majority of theme hooks most themers interact with are created from core or contributed modules. These theme hooks are deeply baked into how Drupal works, and what most themers do is work with the existing templates used to output the data from render arrays implemented in core and contrib to pass data through theme hooks and their associated preprocess functions to templates. There may be a very, very, very small minority of themers who create their own theme hooks, but that's small indeed. Yes, theme hooks are defined by backend developers with a certain set of generic variables that are likely to be useful in templates. Themers come in, and we find no, there's an additional variable I need. Not an existing variable that can be modified, but a brand-spanking new variable. There's often complicated data processing that goes on to get the values for this variable. It's also not uncommon on projects for back-end devs to help out front-end devs by making those new variables in a custom module through preprocess.

I can count on one hand the number of times I've actually created a new theme hook where I had complete control of the variables that I needed for a template. And when I have, it's been in a module, not a theme. Do some people create theme hooks in themes? Maybe? But I'd wager that very, very, very few people do that. That's just not a common thing.

It's hard to overstate how big a disruption this would be to the everyday theming workflow for me and a vast number of other themers.

The goal is good! Make it easier to know what variables you can use in a template. That's cool! But there are other ways to do that than just locking down the addition of variables to templates.

I'm not going to write a long reply about coding philosophy, but just note that in practice adding variables in preprocess is extremely common and useful. Would be sad to see that taken away or made significantly more difficult.

I have read the whole thread, and am still fuzzy on the *practical benefits* should this change indeed be made -- that's not to say they don't exist, just to say that they aren't spelled out well enough, or don't amount to enough that I am able to re-articulate them.

Oh and if you want themers / frontenders to not to have to deal with render arrays, that's IS a good goal! Lemme tell you, backend developers don't want to deal with them either :P Wish we could just cut them out altogether !!

If you want themers so keep using Drupal, don't remove this. It seems like we're making many things way more difficult for corectness. Make Drupal for developers, not for winning a design prize.

The demo repository uses a lot of code, so I presume there would have to be extra APIs added to make this less burdensome.

I guess it depends how much you want to push the abstraction layers and reusability of smaller and smaller pieces, and adding more logic to one place (code to simplify this architecture) instead of having it somewhere else (preprocess functions inserting variables)..

There's nothing stopping someone from implementing what you're mentioning today using the current APIs, maybe you should propose it as a theming workflow for others to use?

FYI: Display Suite's PPF fields heavily rely on preprocessors to fill in their values. I suppose DS could still declare them by altering the theme registry though.

Just a thought, but maybe it'd be possible to run a step after preprocessors and before going into the template which "simply" checks which variables have been passed in and adds them to some lookup table if they weren't already declared? Could perhaps be used to differentiate between 'static' and 'dynamic' theme variables, if that's useful.

Disclaimer: I don't yet know how Drupal actually passes its variable to Twig, so maybe this is impractical for some reason.

I still need to look at the example lauriii set up, but wanted to mention a brief thought I had.

Part of the challenge with the process of getting variables from theme hooks through preprocess to templates, is that we conflate inputs and outputs.

The variables specified in a theme hook at least somewhat act as "Hey, here are the things you need to pass into a render array with this theme hook in order for it to work." Sometimes a preprocess function can add another variable that is actually required in order for that theme hook to work, but it's very hard to track that down.

Sometimes these input variables are just used as is and output in a template. But sometimes there needs to be some massaging of that input data before it's useful for it to appear in a template. That input data can be turned into new variables that are easier to work with in the template. For example, an object like a node or a field might be passed into a render array, and preprocess takes care of turning that into something easier to output for a template.

What could be useful is making this separation of concerns more clear. Defining the inputs for a render array—these are the required or the optional items to pass into a render array—and a separate list of the output variables that should be used in the template itself. Not that it should be impossible to access that raw, input data. But having both of those clearly defined might make variables in templates a lot easier to work with. Right now they're all mixed together, and it's confusing.

Some sort of render array registry to be able to look up the inputs and outputs for a render array could be handy. Keep in mind that could vary based on conditional items. It's pretty common in preprocess to only add certain variables under certain conditions, and that doesn't necessarily line up with a template suggestion for example.

It's a complicated system, but that allows us to do some pretty complex things. I'm all for ways to make this easier to work with, I just want to make sure we do so without sacrificing the flexibility we have that enables us to do some pretty ambitious things with Drupal theming.

Took a look at the example repo. Thanks for sharing that, lauriii. That makes things much more clear.

Here's my understanding of what's being proposed.

Essentially, use render elements for everything rather than simple arrays. If there are variations on the variables used by a render array, manually create a theme hook suggestion that defines those additional variables. This could in theory require a lot more templates, one for each additional optional variable. The decision of whether to use a theme suggestion rather than the default theme hook would took place in a pre-render callback.

The advantage of this approach is that all potential variables are defined within a hook_theme definition. That may or may not be easier to understand than reading through preprocess functions to discover additional required render array keys not listed in hook_theme (or a render element's info statement).

The example given looks fairly understandable for when there is one set of optional variables. I wonder, however, how much more complex this would be when there are, say, five sets of optional variables, which certainly can happen. Would additional theme suggestions be necessary for all the possible combinations of optional variable sets? That could escalate very quickly.

I'm unclear on where the exact advantage lies in defining variables only in hook_theme statements. Would there be some additional method of exposing those defined variables, along with all the possible variations for a particular render array? This code is fairly complex, more complex than what most themers have been exposed to through simple preprocess functions. So unless there is some additional method of taking advantage of this change, I'm still hard-pressed to see where the advantage of this is.

I can see now that there's still flexibility in allowing for expanding upon render elements/arrays defined in core or contrib. Letting themes do additional things to render arrays/elements is pretty key.

I'm just not totally clear yet on what this extra complexity buys us by making this a requirement.

Also, if there really was a change to make this the only ways to add variables to a render array, my understanding is that this would need to be a 9.x issue. Removing the ability to add variables in preprocess would be a huge API change.

Sounds like all the discussion bases have been pretty much covered.

There is one thing that occurred to me that hasn't been discussed yet.

Global Twig Context Variables. There are more than a few modules and pieces of functionality that depend on being able to add variables accessible globally in every twig context.

Currently, you either need to add a preprocess, or do a lot of custom code to make that happen in Drupal 8.

If we were to move forwards with the implementation we're sketching around here, we'd definitely want to add an easy ability for 'global' twig variables to be added to all contexts during rendering, and think about some of the ways that functionality could be both useful and abused.