Problem/Motivation
There's some momentum on component based theming based on Drupal. One of the most popular ways to deal with this is to include external files from a Twig template. For example:
{{ include("card.html.twig", {
title: label,
text: body,
},
with_context: false,
) }}
Obviously, this doesn't allow any kind of preprocessing in the preprocess functions or theme suggestions. If there would be an easy way to load those templates using theme system, we wouldn't necessarily have to lose all that functionality.
Proposed resolution
Create a Twig function that will include a Twig template while also running Drupal's normal preprocess functions and theme suggestions.
The new function should be as close to the syntax of Twig include, and shouldn't be much more difficult to use so that people would replace their includes with this new function.
Since Drupal's Twig templates will automatically take {{ render_array }} and render it to a string, the easiest and most flexible way for this new Twig function to work is for the function to return a render array that uses [ '#theme' => 'theme_hook_name', '#var1' => 'etc' ]. Drupal's render API will then call the theme registry to find the appropriate template and process all of its variables, etc.
An idea for how this could look in a Twig template.
{{
include( "card", {
title: label,
text: body,
}
) }}
Which would then be in PHP:
print render([
'#theme' => 'card',
'#title' => $label,
'#text' => $body,
]);
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | 2818121-20-twig-theme-function.patch | 3.45 KB | johnalbin |
Comments
Comment #2
fabianx commentedIt would be fantastic if we would be able to create this in a way that IDEs could allow auto-completion.
I even thought about automatically creating one function for each theme hook and allowing named parameters. (e.g. include_theme_card('suggestions' => 'card--red', 'label' => '...')).
Unfortunately I don't use an IDE and am not aware how good named parameters function support is, so someone else will need to check if it is worth it to explore this.
Comment #3
cosmicdreams commentedIf by IDE's you mean PHPStorm we could create an issue for that in their issue queue.
Comment #4
cosmicdreams commented@laruii in the OP you say:
I disagree. You can execute preprocess hooks on the template that includes these twig directives. Which could be a node, a field, a paragraph, whatever drupal-level thing you're putting the twig statement in.
Doing this way is more disorganized for sure. But you assert that it's obvious that it's not possible. There's nothing about this implementation that removes preprocessing from Drupal 8.
Perhaps I'm not understanding what you're saying. If so, please help me understand.
Comment #5
xanoTo make sure this is easy to debug, I want to propose we only support this for elements for which a builder API exists (#2316941: Use the builder pattern to make it easier to create render arrays). This allows us to add validation to the builders, so if themers make mistakes in their templates, they'll see exceptions with hopefully useful messages instead of having to dig through PHP code to find out what part of the render array was wrong.
Comment #6
joelpittetAdding related experiment in Contrib
Comment #7
lauriiiComment #8
lauriiiFirst version of render array generator is in components module! Please test :)
Comment #12
markhalliwellComment #16
johnalbinFYI, Twig's include tag docs have been updated to say:
So a
{% include_theme %}tag would be semantically incorrect. If we want to have a render array generator, it will need to be a Twig function.Comment #17
markhalliwellSo, core modifies the rendering of any variable to automatically be passed through the renderer. This is what allows variables that are render arrays to remain as such until the very last second so they don't have to be passed as rendered strings to the templates (which allows for easier manipulation as needed).
I just tested this locally and it works:
I'd say that we've had this ability for quite some time, but it's not really documented anywhere.
Tempted to just close this as won't fix, but perhaps we should add some documentation somewhere.
Comment #18
rgpublic@markcarver: Well, this is well-known AFAIK, but I thought this issue is about creating an easy-to-use/readable function/tag to call this stuff without introducing all the hash-prefixed keys etc. After all the Twig language/concept is also about easy readability/maintainability of the code - especially for non-geeks if you have a split PHP and UI team... So, I still think it'd be worthwhile to introduce sth. like the proposed function to avoid having to spread those convoluted associative arrays all over the place.
Comment #19
markhalliwellThe OP just states: "Create a render array generator that can be used in Twig."
The reason being: "One of the most popular ways to deal with this is to include external files from a Twig template. Obviously, this doesn't allow any kind of preprocessing in the preprocess functions or theme suggestions."
This is implying that it needs to run through core's renderer.
Which core already has, both in the
|renderTwig filter, which is just an alias for therender_var()Twig function... which standalone variables are also passed through (as mentioned above).Nowhere does it mention anything about eliminating the property key # prefix.
Why is this continually used as a "reason" for Twig stuff. No, Twig is just a template language. If someone (geek or non-geek) is attempting to use render array (which is a PHP backend API), then they've clearly passed the "non-geek" threshold as this would be or rather should be considered "advanced usage" anyway.
Actually, this is one of the reasons
theme()was removed. Creating "symantic sugar" for something that already exists is, ultimately, confusing and causes more headaches than it's worth in the long run.Besides, whatever has already been proposed here would still have to spread some sort of an associative array, regardless if it's prefixed with a # property key or not.
Twig already supports outputting render arrays, let's just use it and properly document it.
Comment #20
johnalbinI think the issue summary doesn't clearly explain what people are doing now that is problematic. So I've updated it:
The alternative to that would be to add a theme function that would look like this:
With Twig's new named arguments, you could also call that function like this:
Or like this:
And, yes, as Mark states, this is identical to the already-possible example of:
BTW, I've been playing with the implementation of the theme function in contrib, Components 8.x-2.x-dev. https://git.drupalcode.org/project/components/blob/8.x-2.x/src/Template/...
I've attached that code here in the form of a core patch.
Also, I'm wondering if we should call this function
template()instead? Then the hook parameter could optionally allow hyphenated template names (e.g. "item-list") or even full template names (e.g. "item-list.html.twig") and we could convert those to proper theme hook names in the background.Comment #21
markhalliwellFrom #2173655: Refactor theme() to _theme(); make it a private API to discourage module developers from circumventing the renderable build system:
Granted, this is several years old (and these functions don't exist or are not used anymore), but the problem this caused could easily be translated to Twig if this were implemented:
Do I use
{{ render_var() }},{{ some_variable|render,{{ theme() }}, or in the case of the alternative function name:{{ template() }}?Doing this would only foster more confusion.
Nevermind the fact that whatever name is chosen here would essentially lock core in supporting this named function for years to come.
Given the possibility of a lot of future internal Theme System/API changes in the future (#2869859: [PP-1] Refactor theme hooks/registry into plugin managers), I don't see what added benefit this has in core at the moment.
I'd rather not back us into a corner should the need for using a named variable/function in Twig come to pass.
For example: in Drupal Bootstrap, all theme hooks are given a contextual
themevariable that contains valuable metadata regarding the theme:https://git.drupalcode.org/project/bootstrap/blob/c9e102b8c089e8c33ff632...
This is something I plan on introducing to core somewhere down the line, but its exact implementation will likely change overtime.
I don't think function and variable names overlap, but you can see how this would still add to the confusion.
IMO, any named function that adds semantic sugar around a render array should be left to contrib as an opt-in approach.
There is technically no need for it in core; one can still do what is proposed printing standalone render arrays in Twig.
Comment #22
larowlanI think there is some contention here, so tagging for review before people expend too much energy.
Comment #23
johnalbinTLDR; I don't think we need a generic array generator in core. We do need a better alternative than Twig's include function.
I think this is your best argument. I was hesitant about reusing the function name, "theme", because it plays right back into this old confusion. Thank you for reminding me. This is a solid reason for not including this function.
Recent Drupal core development has shown that is easy to mark functions as deprecated. :-)
Hmm… I think our definition of what "it" we are discussing is different. Which makes sense since the title of this issue is "Create render array generator that can be used in Twig".
Ugh. That is a horrible title considering the current "Problem/Motivation" says:
Rephrasing the title of the issue: Do we need a generic render array generator? In addition to theme hooks, we also have render elements. Do we need a
element()function that generates a render element like "button" or "details"? I don't see evidence that we do need that. So I agree with Mark on this point; we don't need a generic render array generator in Twig.I'm going to remove the "render array generator" terminology from this title/issue summary. And I'm also going to update the example code since, according to Twig, Twig's include tag is not recommended. Here's an updated "proposed resolution":
Comment #24
fabianx commentedHere is an even nicer syntax proposal:
At this stage we are already at the rendering stage so calling render() is perfectly okay btw.
Or what about:
And mapping text to the default slot.
Just some ideas.
Comment #25
johnalbin<card title={{ label }} >I really like this syntax. It is the same as used by web components and is a natural extension of the HTML API. However…
Drupal's list of theme hooks was never designed to be a list of components. If we implement the syntax exactly as shown above, we would introduce an immediate infinite loop in our code. :-D
Specifically,
html.html.twigincludes the code<html{{ html_attributes }}>, which would nesthtml.html.twiginside itself. lol!We have several other theme hook names that conflict with HTML elements: table, mark, menu, input, select, fieldset, details, form, textarea.
React solves this problem by using capitalized camel-case.
<ItemList items={{ label }} />I fear that going to this syntax is going to mean a LOT more code than the current patch though. The current patch (which needs some improvements) only adds a simple Twig function as an alternative to Twig include. It's very small. Maybe this new syntax proposal should be a related issue?
Comment #26
ghost of drupal pastComponents ... twig... that reminds me of something very very old. (That's me. I was not cast as the ghost of drupalcon past for nothing. And even that was five years ago.) Anyways #1499460-37: [meta] New theme system says
Also #1499460-127: [meta] New theme system
With a proposal at https://jacine.net/post/19652705220/theme-system and http://jacine.github.io/drupal/
We lost this somewhere along the way. I'm sure this issue is not it but ... perhaps start now for Drupal ... 10?
Just sayin'. I don't plan to get involved much this time.
Comment #27
markhalliwellI still don't believe that adding some sort of sugar-coated semantic wrapper function gives anyone any real benefit here. If anything, it will only lend to more confusion (at the moment). For now, people can simply use a render array. If and when that changes (or becomes more difficult to do), I think we can revisit this.
There are a lot more pressing theme system issues that need attention over this particular issue.
Comment #28
johnalbinWas marking this issue as "Postponed (maintainer needs more info)" intentional? I don't see any maintainers asking questions. And I'm happy to answer any questions.
Comment #29
markhalliwellNice dig...
Comment #30
andypost@JohnAlbin I'm sure everything is postponed on metadata definition for "components" (whatever it means) and the blocker is #3064854: Allow Twig templates to use front matter for metadata support
Meantime, looking at https://git.drupalcode.org/project/components/blob/8.x-2.x/src/Template/...
It could also should benefit from it (at least in validation of passed in arguments)
Comment #31
johnalbinI think the discussion we are having is productive and interesting. And the patch is nowhere near to being ready.
Mark, I promise I'll ping you before this patch is RTBC. And you can go work on those other more pressing issues in the meantime.
Andy, can you expand on why you think metadata would help with this issue? Theme hooks already describe the variables they expect (though poorly since a theme hook only lists variable names and not variable types. Did your "validation of passed in arguments" mean that we could replace theme hook definitions with meta data in the Twig file? I like that idea! And I'm confused what it has to do with the current scope of this issue.
Comment #32
markhalliwellFront matter has nothing to do with this issue.
I'm utterly confused with
Yet the patch in #20 is basically that: generates a render array.
Why is this needed?
Comment #33
johnalbinThe original issue description conflated the motivation (we need a better Twig include that is Drupal aware) with the implementation (a render array will trigger all the Drupal stuff while including a template). And it only mentioned the implementation in the issue title.
We do not need a generic render array generator.
We do need need a better Twig include function. As for why, see the current issue summary.
Comment #40
andypostLooks like SDC also adds namespacing to components #3340712: Add Single Directory Components as a new experimental module
For example in #3347672: Create new SDC component for Umami (Common Card)