Problem/Motivation

Theme development for Drupal is too complicated, with too many layers. This is part of the movement to clean up rendering of content in Drupal.

The process layer was only created because we needed a place to flatten complicated data structures such as objects or arrays into strings. In practice, render arrays themselves allow for late rendering via theme function or template file (and can be manipulated in other preprocess functions), and objects can implement __toString methods for printing. There is no longer a need for this whole extra layer of processing before rendering.

Proposed resolution

Removal of the entire process layer.

Remaining tasks

None!

Completed

#2004286: Defer calls to drupal_get_* functions until printed inside a template by adding a RenderWrapper class
#1843728: Remove template_process_field()
#1941286: Remove the process layer (rdf module)
#1939066: Convert theme_breadcrumb() to Twig
#2003814: Remove Bartik process layer functions, refactor color module alterations
#1843768: Remove template_process_overlay()
#1843788: Remove template_process_username()
#1811828: Use #attached to find the css/js of a view
#1843712: Remove template_process_book_export_html()
#1843678: Move building of messages render array from template_process_page() to template_preprocess_page()
#1843668: Move building of breadcrumb render array from template_process_page() to template_preprocess_page()
#1589968: Move Title from process to preprocess, remove template_process_page()

User interface changes

None

API changes

Removal of the following functions:

  • hook_process
  • hook_process_HOOK
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jenlampton’s picture

Issue summary: View changes

add issue

jenlampton’s picture

Issue summary: View changes

blockers

jenlampton’s picture

Issue summary: View changes

added process_html

jenlampton’s picture

Issue summary: View changes

issues

jenlampton’s picture

Issue summary: View changes

book

jenlampton’s picture

Issue summary: View changes

field

jenlampton’s picture

Issue summary: View changes

overlay module

jenlampton’s picture

Issue summary: View changes

user module

jenlampton’s picture

Issue summary: View changes

views

jenlampton’s picture

Issue summary: View changes

RDF

jenlampton’s picture

Issue summary: View changes

color

star-szr’s picture

Issue summary: View changes

drupal -> Drupal

c4rl’s picture

Issue summary: View changes

Updated issue summary.

ltrain’s picture

Issue summary: View changes

process_entity did not need to be on this list - it has nothing to do with rendering

ltrain’s picture

Issue summary: View changes

Added an issue for removing process functions from Bartik

dasjo’s picture

we have had some discussions with sdboyer, because getting rid of process is kind of tricky in combination with drupal_add_*

joelpittet therefore came up with a proposal: #2004286: Defer calls to drupal_get_* functions until printed inside a template by adding a RenderWrapper class

sam basically would prefer using controllers instead of the logic that happens within pre/process_page and the maintenance version of it.
see this (outdated) gist: https://gist.github.com/c4rl/4780229
and #1843710: Remove template_process_maintenance_page()

dasjo’s picture

Issue summary: View changes

add another blocker for the drupal_add_css and drupal_add_js late rendering.

jenlampton’s picture

Issue summary: View changes

remove remove rdf

jenlampton’s picture

Issue summary: View changes

blocker

jenlampton’s picture

Issue summary: View changes

remove blocker

jenlampton’s picture

Issue summary: View changes

remove breadcrumbs

jenlampton’s picture

Issue summary: View changes

add back breadcrumbs issue

jenlampton’s picture

Issue summary: View changes

other issue

jenlampton’s picture

Issue summary: View changes

reorder

jenlampton’s picture

Issue summary: View changes

more separation

jenlampton’s picture

Issue summary: View changes

dupe

jenlampton’s picture

Issue summary: View changes

remove

jenlampton’s picture

Issue summary: View changes

update

jenlampton’s picture

Issue summary: View changes

also

jenlampton’s picture

Status: Active » Needs review
FileSize
18.98 KB

Okay, this patch is mostly docs updates, but there are some code changes in here as well that will KILL PROCESS. I'm sure it will need a reroll once our blockers are out of the way to pass tests, but I got excited and had to start on this :)

Status: Needs review » Needs work
Issue tags: -Twig, -theme system cleanup

The last submitted patch, core-remove_process_phase-1843650-2.patch, failed testing.

joelpittet’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
Issue tags: +Twig, +theme system cleanup

The last submitted patch, core-remove_process_phase-1843650-2.patch, failed testing.

Anonymous’s picture

Issue summary: View changes

rearrange

scor’s picture

system/theme.api.php will also need to be updated to REMOVE any kind of documentation about hook_process().

scor’s picture

Issue summary: View changes

rearrange

jenlampton’s picture

Issue summary: View changes

.

star-szr’s picture

Issue summary: View changes

Refresh

star-szr’s picture

Issue summary: View changes

Reorganize

star-szr’s picture

Issue summary: View changes

Colour change

jenlampton’s picture

Issue summary: View changes

turn green!

jenlampton’s picture

Status: Needs work » Needs review
FileSize
2.4 KB
21.23 KB

Also removed all the process hooks from theme.api.php :)

Status: Needs review » Needs work

The last submitted patch, core-remove_process_phase-1843650-7.patch, failed testing.

Anonymous’s picture

Issue summary: View changes

Update problem/motivation section - we didn't end up converting render arrays to objects but render arrays themselves work well for our aims.

catch’s picture

Priority: Normal » Major

While this is in progress, could we get a quick patch to mark process as deprecated in the API documentation?

star-szr’s picture

Status: Needs work » Needs review
FileSize
1.09 KB

Something like this?

(If anyone prefers I can move this patch to a new issue.)

catch’s picture

Status: Needs review » Reviewed & tested by the community

Looks right to me. I'm OK with commit ->active but if someone strongly wants a new issue that's fine too.

jenlampton’s picture

commit ->active sounds good to me :)

jenlampton’s picture

Issue summary: View changes

up

YesCT’s picture

Issue tags: +RTBC July 1

This issue was RTBC and passing tests on July 1, the beginning of API freeze.

catch’s picture

Status: Reviewed & tested by the community » Active

Committed/pushed to 8.x, thanks!

Could do with a change notice to point out these have been marked deprecated, which can be updated when it's removed.

Wim Leers’s picture

Title: [meta] Remove the process layer » Change notice: Remove the process layer
Priority: Major » Critical

This is not really a meta issue anymore, and now needs a change notice.

catch’s picture

Issue tags: +Needs change record
star-szr’s picture

Status: Active » Needs review

Change notice draft:

The template process layer is deprecated and will be removed

The template process layer and process functions (e.g. template_process_HOOK(), MODULE_process_HOOK(), THEME_process_HOOK()) have been deprecated and will be removed in Drupal 8.

In the Drupal 7 theme system, process functions were used to flatten variables into strings for printing (see template_process() from Drupal 7 for an example). With the shift towards using (more) render arrays and renderable objects (objects with __toString() methods), in Drupal 8 we can remove this additional layer of processing and complexity from the theme system in favour of "lazy rendering" as late as possible and only when necessary.

Render arrays and renderable objects can be manipulated throughout the preprocess phase without needing to worry about the variable already being turned into markup or a flattened string. The render array or renderable object can then be printed in a template file without first flattening it into a string. With the Twig template engine (the default template engine in Drupal 8), you simply print the variable in your template and the Twig engine determines whether it is a render array, renderable object, string etc. and renders it appropriately.

The following example prints an Attribute object with a __toString() method in a Twig template:

{{ attribute }}

Impacts:

  • Module developers
  • Themers
star-szr’s picture

Any feedback on the above before I post?

star-szr’s picture

joelpittet’s picture

It makes me almost as happy as if we had renderable objects:)

star-szr’s picture

Title: Change notice: Remove the process layer » [meta] Remove the process layer
Priority: Critical » Major
Status: Needs review » Active
Issue tags: -Needs change record +Twig, +theme system cleanup

Posted change notice:
https://drupal.org/node/2038981

Going back to active + major + meta issue for now.

joelpittet’s picture

Issue tags: +RTBC July 1

Sorry for the cross post

joelpittet’s picture

Issue summary: View changes

api changes

jenlampton’s picture

Status: Active » Needs review
FileSize
22.33 KB

removal :)

Status: Needs review » Needs work

The last submitted patch, drupal-remove_process_layer-1843650-23.patch, failed testing.

markhalliwell’s picture

Status: Needs work » Needs review
FileSize
2.61 KB
25.79 KB

Rerolled. Also did a S&R for template_process and removed/changed additional documentation. This patch will probably fail until #1589968: Move Title from process to preprocess, remove template_process_page() is committed. Then we can retest/reroll if needed.

Status: Needs review » Needs work
Issue tags: -Twig, -theme system cleanup, -RTBC July 1

The last submitted patch, drupal-meta-remove-the-process-1843650-25.patch, failed testing.

markhalliwell’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
Issue tags: +Twig, +theme system cleanup, +RTBC July 1

The last submitted patch, drupal-meta-remove-the-process-1843650-25.patch, failed testing.

markhalliwell’s picture

Status: Needs work » Needs review
FileSize
25.06 KB
25.93 KB

Reroll

markhalliwell’s picture

Issue summary: View changes

update

Fabianx’s picture

Once testbot comes back green, lets get this in!

This is according to my understanding:

RTBC

Status: Needs review » Needs work
Issue tags: -Twig, -theme system cleanup, -RTBC July 1

The last submitted patch, drupal-meta-remove-the-process-1843650-29.patch, failed testing.

markhalliwell’s picture

Status: Needs work » Needs review
Issue tags: +Twig, +theme system cleanup, +RTBC July 1

Weird... only 1 error?? Tested this failing test locally and it passed with patch... sigh testbot fail, retest!

#29: drupal-meta-remove-the-process-1843650-29.patch queued for re-testing.

star-szr’s picture

Status: Needs review » Needs work

One question and one nitpicky bit :)

+++ b/core/includes/theme.incundefined
@@ -450,11 +449,8 @@ function drupal_theme_rebuild() {
-  // Processor functions work in two distinct phases with the process
-  // functions always being executed after the preprocess functions.
   $variable_process_phases = array(
     'preprocess functions' => 'preprocess',
-    'process functions'    => 'process',
   );

@@ -654,7 +650,7 @@ function _theme_build_registry($theme, $base_theme, $theme_engine) {
-    foreach (array('preprocess functions', 'process functions') as $phase) {
+    foreach (array('preprocess functions') as $phase) {

@@ -1036,30 +1006,30 @@ function theme($hook, $variables = array()) {
-  if (isset($info['preprocess functions']) || isset($info['process functions'])) {
+  if (isset($info['preprocess functions'])) {
     $variables['theme_hook_suggestions'] = array();
-    foreach (array('preprocess functions', 'process functions') as $phase) {
+    foreach (array('preprocess functions') as $phase) {
       if (!empty($info[$phase])) {
         foreach ($info[$phase] as $processor_function) {
           if (function_exists($processor_function)) {

Can (some of?) these parts be simplified by not treating the process phases as an array when we don't need to?

+++ b/core/modules/search/templates/search-result.html.twigundefined
@@ -53,7 +53,7 @@
+ * @see template_preprocess()

+++ b/core/modules/system/templates/page.html.twigundefined
@@ -58,7 +58,7 @@
+ * @see template_preprocess()

+++ b/core/themes/bartik/templates/page.html.twigundefined
@@ -72,10 +72,9 @@
+ * @see template_preprocess()

+++ b/core/themes/seven/templates/page.html.twigundefined
@@ -59,8 +59,8 @@
+ * @see template_preprocess()

Let's not add these back, see #2013094: [policy adopted, patch needed] Stop saying '@see template_preprocess()' in every twig file.

markhalliwell’s picture

Status: Needs work » Needs review
FileSize
1.72 KB
25.64 KB

Re #1: I would rather just remove just the implementations here so we can quickly, loudly and completely remove the process layer from core. This will all, more than likely, get refactored anyway with #2035055: Introduce hook_theme_prepare[_alter]() and remove hook_preprocess_HOOK().

Re #2: I was not aware of that issue, took them out.

steveoliver’s picture

Mark, I think we should go with Cottser's suggestion to stop treating the one phase here as an array.

markhalliwell’s picture

I'd rather leave that logic in place here and just remove the process phase. The $phases array will still be utilized by #2035055: Introduce hook_theme_prepare[_alter]() and remove hook_preprocess_HOOK().

markhalliwell’s picture

Ok, I went ahead and refactored it to not use arrays. Turns out with the proof-of-concept patch in #2035055: Introduce hook_theme_prepare[_alter]() and remove hook_preprocess_HOOK(), it's not going to be needed after all.

jenlampton’s picture

Status: Needs review » Reviewed & tested by the community

Just gave it another quick test and (now that it's green) setting it back to RTBC. Good work team!

Fabianx’s picture

+1 for RTBC

star-szr’s picture

Title: [meta] Remove the process layer » Remove the process layer (hook_process and hook_process_HOOK)

Gave this another review, I agree it looks good. Nicely done @Mark Carver!

catch’s picture

Title: Remove the process layer (hook_process and hook_process_HOOK) » Change notice: Remove the process layer (hook_process and hook_process_HOOK)
Status: Reviewed & tested by the community » Active
Issue tags: +Needs change record

OK. Committed/pushed to 8.x. Will need a change notice.

star-szr’s picture

Title: Change notice: Remove the process layer (hook_process and hook_process_HOOK) » Remove the process layer (hook_process and hook_process_HOOK)
Status: Active » Fixed
Issue tags: -Needs change record

Thanks! I just updated the existing change notice:

https://drupal.org/node/2038981

See also #2004286-101: Defer calls to drupal_get_* functions until printed inside a template by adding a RenderWrapper class for the newly proposed RenderWrapper change notice.

jenlampton’s picture

<3

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture