Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.There are some variables that should just not be declared in hook_theme() or they run the risk of conflicting with attributes used by drupal_render() and/or the FAPI.
Used internally by drupal_render() - definitely don't use these for theme variable names:
access
printed
cache
type
defaults_loaded
pre_render
children
theme
render_children
markup
states
attached
theme_wrappers
post_render
prefix
suffix
Used internally by theme() - definitely don't use these in hook_theme() unless you really know what you're doing:
theme_hook_original
theme_hook_suggestion
theme_hook_suggestions
directory
attributes
title_attributes
content_attributes
Not covered above, but has special meaning within the core FAPI - generally discouraged to avoid human misunderstandings, strongly discouraged for theme hooks commonly used in/around forms unless you're implementing exactly what is documented at https://api.drupal.org/api/drupal/developer%21topics%21forms_api_referen...
after_build
ajax
array_parents
autocomplete_path
collapsed
collapsible
cols
default_tab
default_value
delta
description
disabled
element_validate
empty
empty_option
empty_value
field_prefix
field_suffix
group
header
js_select
languages
maxlength
multiple
options
parents
placeholder
process
required
resizable
return_value
rows
size
title
title_display
tree
value_callback
weight
This should all be clearly documented in the docblock for hook_theme as many themers would be unaware of all the items in the full list.
Comments
Comment #1
jhodgdonMaintaining this list in hook_theme() seems like it might create a problem -- if some property gets added to the Render API or Form API, I doubt it would get added to the hook_theme() documentation.
I think it would be better to say something about why you want to avoid using form/render API properties for your theme variable names, and link to where these properties are listed and documented. The properties are listed/documented in hook_element_info() -- which is also being worked on in:
#2015177: Update documentation around hook_element_info() to not say/imply element types are limited to the Form API
but since you're the one working on it there, you probably know that. :) And a link to the Form API reference page might also be useful?
Comment #2
thedavidmeister CreditAttribution: thedavidmeister commented@jhodgdon - this is all true.
Comment #2.0
thedavidmeister CreditAttribution: thedavidmeister commentedUpdated issue summary.
Comment #2.1
thedavidmeister CreditAttribution: thedavidmeister commentedUpdated issue summary.
Comment #2.2
thedavidmeister CreditAttribution: thedavidmeister commentedUpdated issue summary.
Comment #2.3
thedavidmeister CreditAttribution: thedavidmeister commentedUpdated issue summary.
Comment #2.4
thedavidmeister CreditAttribution: thedavidmeister commentedUpdated issue summary.
Comment #2.5
thedavidmeister CreditAttribution: thedavidmeister commentedUpdated issue summary.
Comment #3
thedavidmeister CreditAttribution: thedavidmeister commentedMaybe the right approach to take here is one of test coverage rather than documentation? or maybe we need both?
The proposed docs in the linked issue in #1 does have some attributes, but nowhere near as many as are currently in the issue summary,
and the issue summary doesn't cover theme() internals yet so it's going to get bigger.Edit: I'm updating the issue summary now, it should be more comprehensive shortly.
Comment #3.0
thedavidmeister CreditAttribution: thedavidmeister commentedUpdated issue summary.
Comment #4
jhodgdonTest coverage will not help people writing hook_theme() in contributed modules.
So... maybe hook_theme() should say something like:
When choosing variable names for your theme hooks, avoid standard properties that are used by drupal_render(), theme(), or the (link)Form API(/link).
[the idea being that people would look at the documentation in those 3 places for the list of standard properties]
But... wait a minute. Does it really matter if you have a variable called "foo" and there is also a standard form/theme/render property called "#foo"?
Comment #5
Eric_A CreditAttribution: Eric_A commentedI've taken a quick look at the third list. Some of these properties are owned by FAPI. Most are just data properties which have meaning for certain element types. The first category is not to be used by any theme hook that is meant to be used by a FAPI element. The second category is to be matched by theme hooks. It is not like you can swap out theme hooks blindly.
Comment #6
thedavidmeister CreditAttribution: thedavidmeister commented#4
It does if you want to convert theme() calls to drupal_render($build) calls because they end up being the same thing and your variable ends up having two meanings (one is probably unintended).
#5
Yeah, it might be worth splitting this list into two lists - one for things that are actually part of the FAPI and those that are just used by certain element types because the latter is "safer".
The first category applies to any render array, not just those in the FAPI.
This issue is only talking about hook_theme() so we're not really considering #markup style render arrays that bypass theme() atm.
Currently the documentation does not really reflect this concept, that's the point of this issue. How on earth is someone who is using Drupal for the first time supposed to know that there's a bunch of variable names you could set in hook_theme() that have conflicts with the Render API itself? Why can't I swap them out blindly? How do I find out which variable names to avoid?
Comment #7
jhodgdonOK...
So:
a) I don't think we want to add these lists to the documentation of hook_theme() at all.
b) We don't need to make another list in this issue of the properties referenced on the Form API Reference page. They're already listed there.
c) The properties used by drupal_render() should be mentioned in the drupal_render() documentation. The properties used by theme() should be mentioned there.
d) Therefore, I think none of the properties listed in this issue should be explicitly mentioned in hook_theme() docs. We should just tell about the general principle of "avoid reserved property names" and tell where to find out what those reserved property names would be.
Right?
Comment #8
thedavidmeister CreditAttribution: thedavidmeister commented@jhodgdon, I'm happy with that. However we organise it it just needs to be easy to understand and find when you're looking for it. Needing to read through the code of theme() line by line to discover the 'directory' key in $variables is pretty bad.
Comment #9
jhodgdonGood. My main concern leading to #7 is maintainability of a big list of "reserved words" far away from the code that actually reserves them. The Form API Reference already has this problem... I've been trying for a couple of years to change that:
#1617948: [policy for now] New standard for documenting form/render elements and properties
#100680: [meta] Make API module generate Form API documentation
Maybe this will be the year, but I don't have much hope.
Comment #10
thedavidmeister CreditAttribution: thedavidmeister commentedAs per a comment I just left at #2030243: Remove theme_file_upload_help() from the theme system, it seems to me like it might even make sense to say that the variable names of all theme hooks intended for use as #theme_wrappers (those that respect/render #children) should be unique within core and should be avoided in custom/contrib code.
The reasoning being:
- #theme does not care about #theme_wrappers
- #theme_wrappers do not care about #theme, they only care about #children
- #theme_wrappers do not even care about other #theme_wrappers
- IIRC it is not documented anywhere that certain #theme hooks are incompatible with theme hooks intended to be used as #theme_wrappers
- If we require the use of nested child elements to avoid conflicts between #theme and #theme_wrappers, then the whole concept of #theme_wrappers is redundant beyond a very modest performance boost
- The behaviour of render arrays where a single attribute is used by both #theme and #theme_wrappers or multiple #theme_wrappers is undefined
Comment #11
thedavidmeister CreditAttribution: thedavidmeister commentedRelated to #10 #2042285: #theme_wrappers should be able to have a unique set of variables per wrapper hook
Comment #12
thedavidmeister CreditAttribution: thedavidmeister commented#2042285: #theme_wrappers should be able to have a unique set of variables per wrapper hook just landed, so everything in the issue summary that just references variables used by #theme or #theme_wrappers hooks in the FAPI can be ripped out.
It is now possible to "blindly" swap out #theme_wrappers hooks on an element with no negative side effects :)
Once that is done I think we can turn the final docs update into a novice issue and it should get cleaned up pretty fast after that :)
Comment #13
jhodgdonSee #7 for what I think is the plan to address this issue. I don't think we need to update the lists of properties in the issue summary here, because I don't think we're going to list them all but rather reference other lists. Right?
Comment #14
thedavidmeister CreditAttribution: thedavidmeister commentedI think this issue should also ensure the "other lists" exist and are up to date.
Comment #15
jhodgdonThat seems like a separate issue. For instance, you already updated the drupal_render() docs on a seprate issue; if the others are not documenting what properties they use, let's file separate issues for them (or you can file separate issues now saying "verify that...").
Comment #16
thedavidmeister CreditAttribution: thedavidmeister commentedis it a different issue? the issue title just says we need to document reserved attributes for renderables in general, it's not referencing a specific docblock or function.
Comment #17
jhodgdonThat would be OK, except that if you limit the scope of this issue, it's possible that each sub-issue could be handled by a novice with a small amount of time on their hands instead of this being a huge project that will take many reviews to see if it's completely accurate.
Comment #18
thedavidmeister CreditAttribution: thedavidmeister commentedI agree with you, but I'm just proposing a slightly different approach to limiting the scope of the issue.
I think we could start by removing each item from the issue summary that we can now ignore thanks to #2042285: #theme_wrappers should be able to have a unique set of variables per wrapper hook - like "attributes" for example, and then we can make novice issues out of what's left. I have a feeling much of what is listed under FAPI can now be ignored.
Comment #18.0
thedavidmeister CreditAttribution: thedavidmeister commentedUpdated issue summary.
Comment #19
sunComment #20
jibranComment #29
andypostLooks like duplicate of #2015307: \Drupal\Core\Render\RendererInterface::render() docs do not mention most of the base # attributes, adding default attributes to elements or how rendering works
Comment #30
star-szrThanks @andypost, I think this is separate but related (note it was filed by the same person in the same month). Looks to me like the related issue is almost done other than a formatting cleanup.