Problem/Motivation

The current theme system is a complicated mix of sub systems that pass data back and forth to one another in ways that confuses new users, and causes bugs. @see #939462: Specific preprocess functions for theme hook suggestions are not invoked

Proposed resolution

Separate each type of thing that happens during the theme system's preprocess phase into separate stages, called in the correct order, and only once. We have identified three things that currently happen in preprocess:

  1. Variables passed in via hook_theme are used to generate variables that will eventually be printed in templates
  2. Variables are added or changed before being printed in templates
  3. The template being used can be changed. (uhm, WHAT?)

The theme system should be refactored to work more like Form API. There should be a stage for definition, and a separate stage for variable alteration, instead of doing all these things in the same step. The ability to change the template being used doesn't belong here and MUST be split out into a separate earlier phase in the theme system preparation tasks, saving resources by not preparing and altering variables that will never be used.

The five basic phases are as follows:

  • Phase 1: Build the theme registry
  • Phase 2: Callback suggestions alter
  • Phase 3: Prepare variables
  • Phase 4: Alter variables
  • Phase 5: Render output

The initial work done towards solving this problem at DrupalCon Portland by Fabianx and jenlampton can be found here.

Phase 1: Build the theme registry #

The process we use to build the theme registry is updated in the following ways:

  • A theme's callbacks, default arguments, and template file name are defined in hook_theme. This remains unchanged from D7.
  • Let modules and themes define suggestions for base hooks, by adding hook_theme_suggestions. This will allow preprocess functions to run for the suggestion without having to define a template or specific preprocess function.
  • Let modules and themes define a render function for specific template suggestions by adding hook_theme_render_TEMPLATE_SUGGESTION to replace theme_THEMEID__suggestion and [module]_THEMEID; the highest priority implementation wins.
  • Priority will be determined by a single theme_stack. A theme in this stack may have an associated theme engine. This simplifies things by replacing the distinction between base themes and themes.
  • Processing in the registry will be executed in the following order: Modules first, Theme Stack second.

Processing in the registry will execute the following steps, for each module and theme:

  1. Invoke hook_theme().
  2. Invoke hook_theme_suggestions() where suggestions may be registered and base_hook may be set, if it exists, otherwise ignore it. The precedence on suggestions is structured such that suggestions added later overrule earlier ones.
  3. Foreach of those $theme_ids returned in hook_theme (not the suggestions) execute the following:
    • Call theme_retrieve_render_functions([hook], $theme_id) on each themes produced from
      • Looks for hook_theme_render_THEME_ID() and hook_theme_render_THEME_ID__*
      • Registers suggestions if needed and sets base_hook to $theme_id and 'function' => found function.
  4. For the whole module or theme call:
    • Call theme_retrieve_templates($engine, [hook], $path, $hooks)
      • Foreach found template matching of of $hooks, the engine sets 'template' => 'path' and 'function' => 'engine_render_function'.

The above process changes are beneficial in the following ways:

  • Functions will now be found during registry build, they just need to be in the "include" or file defining hook_theme(). If hook_theme() can be called, so can the theme or render functions. This is the biggest and most important change to the status quo, which currently uses the engine to search for the theme functions (WHAT?).
  • Theme engines will set render_function directly. No more need to special case the template case.
  • There will be one single theme stack, listed in reverse order. No more need to special case base_themes.
  • Later implementations overwrite earlier implementations completely.
  • The theme registry will now be much simpler to build because a function will be defined for each hook, base hooks may now be nested, and the first hook to implement 'function' wins - regardless whether it is a theme function or a template rendered via engine function.

Phase 2: Callback suggestions alter #

#1751194: Introduce hook_theme_suggestions[_HOOK]() and hook_theme_suggestions[_HOOK]_alter()

hook_theme_suggestions_alter()
hook_theme_suggestions_TEMPLATE_SUGGESTION_alter()

This is a new phase. The callback_suggestions_alter phase will be the only way to add, change, or remove callback and template suggestions. This alter phase replaces $variables['theme_hook_suggestions'] in preprocess. This phase also allows altering the default "found suggestion", so for a node of type article, "article" might be changed to "page", for example.

Phase 3: Prepare variables #

#2035055: Introduce hook_theme_prepare[_alter]() and remove hook_preprocess_HOOK()

hook_theme_prepare()
hook_theme_prepare_TEMPLATE_SUGGESTION()

This is a new phase. The theme_prepare phase is where theme callback arguments are converted into variables to be rendered in the template. Modules that invoke this hook should only add information to the list of variables in this phase, similar to how hook_menu() just adds information.

Examples of things that MAY be done inside a hook_theme_prepare implementation include:

  • load a node from a nid argument and add it or its members as a variable usable in a template
  • create a URL from a path argument
  • create an image src from a URI

Since sanitization happens when the variables are printed in the template by Twig, there should NOT be any calls to check_plain here.

In Drupal core, this phase might be implemented in the following way:

  $input = $variables;
  $variables = module_invoke_all('theme_prepare', $input); // Modules can add, but not change others' things.
  $variables['#original'] = $input; // Original pristine inputs from earlier phases are stored, so they can be used later.

Phase 4: Alter Variables #

#2035055: Introduce hook_theme_prepare[_alter]() and remove hook_preprocess_HOOK()

hook_theme_prepare_alter()
hook_theme_prepare_TEMPLATE_SUGGESTION_alter()

This is a replacement for the current preprocess layer. The hook_theme_prepare_alter phase allows for altering the variables that were prepared in Phase 3, similar to hook_form_alter and hook_form_FORM_ID_alter. The big difference here (and the reason for renaming this function) is that we don't want modules that defined their own theme callbacks in hook_theme to use this phase to alter those variables into a printable state; instead they should use Phase 3 for that.

Phase 5: Render/Output #

The final phase may have three possible outcomes:

  1. variables are inserted into a template (sanitized) | template -> HTML
  2. variables are concatenated into a string (sanitized) | theme function -> HTML
  3. variables may be returned (non-sanitized). This special case may not end up being necessary, but it would be possible by adding a TRUE/FALSE flag as a third parameter to return these variables as an array. @todo Discuss use cases and implementation.

Remaining tasks

All 4 "API Changes", below.

User interface changes

None, likely.

API changes

  1. Create a new hook: hook_theme_suggestions_alter() @see #1751194: Introduce hook_theme_suggestions[_HOOK]() and hook_theme_suggestions[_HOOK]_alter()
  2. Create two new hooks: hook_theme_prepare() and hook_theme_prepare_alter() @see #2035055: Introduce hook_theme_prepare[_alter]() and remove hook_preprocess_HOOK()
  3. Split things happening in all existing preprocess functions into one of these three distinct stages instead @see #2060773: [META] Migrate hook_preprocess() functions to template plugin prepare() methods
  4. Remove the old preprocess layer @see #2060783: Remove the preprocess layer.

Comments

jenlampton’s picture

Issue summary: View changes

add link to sun's issue

jenlampton’s picture

Issue summary: View changes

add basic 5 phases

jenlampton’s picture

Issue summary: View changes

add whitespace

jenlampton’s picture

Issue summary: View changes

less whitespace

jenlampton’s picture

Issue summary: View changes

more whitespace cleanup

jenlampton’s picture

Issue summary: View changes

added link to closed issue

jenlampton’s picture

Issue summary: View changes

brackets

tim.plunkett’s picture

Preprocess functions MUST be registered manually for hooks and hook suggestions

Emphatic -1. This moves in the opposite direction of the rest of D8, by making TX/DX worse.

Fabianx’s picture

#1: Uhm, lol.

This issue exactly tries to solve this by not requiring the theme system to do that. Once this issue lands, the other one can be finally closed as a dupe.

dawehner’s picture

Thanks for this great writeup.

Here are some questions.

  • Phase 2) Alter Suggestions: I guess this information will happen on runtime not on theme registry process time?
    I guess something else can't solve the dynamic case.
  • Phase 3/4 As people used to put a lot of stuff into preprocess there has been a practice of replacing preprocess hooks instead of just do what you want and override anything. Is it okay to remove this usecase?
  • In order to get this and #1886448: Rewrite the theme registry into a proper service done in time I suggest to start with this issue and then port the hopefully easier code to a theme registry.
tim.plunkett’s picture

@Fabianx there is no explicit issue for that, but it almost happened in the theme registry issue. I'm happy to hear that it is not a goal of this issue, but that was not clear.

Fabianx’s picture

#3:

> Phase 2) Alter Suggestions: I guess this information will happen on runtime not on theme registry process time?
> I guess something else can't solve the dynamic case.

Yes, it is by runtime and that should be okay.

Suggestions are by runtime always, because you can even suggest things by adding an array() of suggested hooks to the theme() function or #theme render key.

> Phase 3/4 As people used to put a lot of stuff into preprocess there has been a practice of replacing preprocess hooks instead of just do what you want and
> override anything. Is it okay to remove this usecase?

Yes, and I explain why:

* Drupal had a mix of preparation and altering.
* Preparation is the task of making sure the input is "prepared" for the output, e.g. in the case of a link to change from the input $path, $text to the output variables $attributes, $text (check_plained), $url that can be used in the template.
* Altering is the task of adding classes, etc.

Because Drupal had both cases mixed, preprocess functions had been replaced rather than appended.

Preparation should never include adding classes or anything else that is not needed for the task of preparing $variables for the output.

We cannot replace a drupal_get_form, why should we allow that on the theme layer?

If someone really needs to do something this advanced (again we found no use case in all of core for that during Twig conversions and also not in our experience), you need to use hook_implements_alter or something similarly advanced.

The advantage is this simplifies the theme registry a _lot_.

> In order to get this and #1886448: Rewrite the theme registry into a proper service done in time I suggest to start with this issue and then port the
> hopefully easier code to a theme registry.

Agree. My biggest worry was how drupal_alter works with themes, but it is fully supported, so only alter hooks from active themes (and base themes) are called, which should be perfectly fine.

So for now I think we can start by just implementing the new layer and calling the old preprocess functions as a compatibility layer, pass tests, then convert all (mostly automatic).

thedavidmeister’s picture

thedavidmeister’s picture

thedavidmeister’s picture

Things that could be issues here:

- "remove post_process from drupal_render() as it breaks drillability"
- "replace pre_process with proper alter hooks in drupal_render()"
- "drupal_render() should preserve the original variables in #original"
- "drupal_render() should have 'default' inline rendering behaviour and a way for #types to declare that they don't need the theme system"

thedavidmeister’s picture

Issue summary: View changes

Little x, forth

c4rl’s picture

@thedavidmeister #8 Can you explain these better? You are mentioning drupal_render(), so I curious how you see these issues relevant here rather than #1843798: [meta] Refactor Render API to be OO.

c4rl’s picture

hook_theme_prepare/hook_theme_prepare_alter feels to me working identical to the D7 preprocess/process layer. Given that we wanted to git rid of the D7 process layer, how is this proposal substantively different? Is there a clear (existing) 90% use case for having both of these functions? Arguably is there overlap with the EntityRenderController?

This special case may not end up being necessary, but it would be possible by adding a TRUE/FALSE flag as a third parameter to return these variables as an array.

As I have explained elsewhere (#1985974-16: Make l() optionally return structured output), it is bad design to have a function return inconsistent data types as an API.

thedavidmeister’s picture

#9 - you're right. I'll post them over there and turn them into actual issues too. I was just chatting in IRC and in a hurry to write them down somewhere.

c4rl’s picture

Here's a philosophical question that will invoke some hard-to-swallow realities:

Some principles here are closely related to #1843798: [meta] Refactor Render API to be OO. Do we feel that by spending time on this present issue and the theme registry issue we are doing enough to improve the theme system for D8? I understand we have to be realistic with freeze dates, but I want to make sure we aren't "polishing the brass on the Titanic" too much.

I don't want to invoke too much political turmoil, but it seems we have a few different approaches we can take.

1. Work to improve what we have. Given the impending freeze date, we'll try to get in this issue and the theme registry issue as realistic improvements for D8. We'll worry about refactoring Render API after these realizing that it may have to wait for D9.

2. Work from a clean slate. Given the impending freeze date, let's get as much done to completely re-architect the Render API and theme system improvements will be sub-issues of #1843798: [meta] Refactor Render API to be OO. Though this is definitely an *enormous* task, if we start early and get enough momentum, is there potential to have our efforts be immune from threshold dates? In reading http://buytaert.net/code-freeze-and-thresholds under "What isn't bound by thresholds" it says:

We obviously want Drupal 8 to ship as a coherent product, so a major focus will be around better integration of existing features. For example...Completing conversions of major APIs such as Twig

So, now that Twig is in core, and the brokenness of the Render API threatens its success and being "coherent," is there any potential to see #1843798: [meta] Refactor Render API to be OO as a critical, threshold-immune, larger-scope meta that accommodates this present issue?

I know it is late in the process, I completely acknowledge that, so I don't want to stir the pot, and I hate to delay further, I'm just thinking about possible directions and where to spend the most effort to get the best release.

thedavidmeister’s picture

#12 - In the render API issue you said:

We believe in order to achieve what we want to accomplish, render arrays must move toward an Object Oriented design.

"must" is a very strong word.

The only thing that we've outlined in #1843798: [meta] Refactor Render API to be OO as potentially worthy of holding back the D8 release is that our renderable thingies need to be drillable.

For that we don't *need* full OO, and I don't think it's fair on the other initiatives to hold back D8 until the entire theme system is converted to objects. As I understand the minimum requirements for "drillability" (which is simply that Twig needs access to the processed-but-not-rendered version of the renderable in a structured format) we could make our existing arrays a bit more standardised and to be smarter/more consistent about the way we process their variables and we'd have drillable variables in Twig in a fraction of the time.

What makes us think that starting from a "clean slate" won't introduce as many new problems as it solves and blow out D8 for another 6-12 months? Are there other D8-release-blocking issues that I'm not aware of that *require* objects to solve?

The OO thing is great but something not being OO doesn't automatically make it broken, I think we can wait on D9 for that particular conversion if Twig drillability is our only concern in this philosophical discussion.

effulgentsia’s picture

So, now that Twig is in core, and the brokenness of the Render API threatens its success and being "coherent," is there any potential to see #1843798: [meta] Refactor Render API as a critical, threshold-immune, larger-scope meta that accommodates this present issue?

Both that issue and this one are already categorized as tasks, not features, so are already threshold immune. However, they are not currently code freeze immune. Per http://drupal.org/core/release-cycle, code freeze is what separates the "clean up phase" we're in now and the "polish phase". While we're in the clean up phase, things that improve consistency and coherence are welcome and encouraged. Once code freeze hits and we enter the polish phase, coherence and DX are no longer enough of a justification to break APIs.

I think the best next step here is to update what the proposed resolution here would be if #1843798: [meta] Refactor Render API to be OO were to make it. For example, should we leave phases 2-4 unchanged, and execute them on the child elements when drilling down from a parent element's template? Or, do we need a new phase before 2 for preparing variables that are involved in drilling through a child element separately from ones that are only needed when rendering a child element? Once there's a clear sense of what we would want here in a post-#1843798: [meta] Refactor Render API to be OO world, then we can see what work would be the same regardless of that issue and do it in parallel to that one. I do not think we should wait until the other issue is done before proceeding with this one, because most of the changes suggested in this issue are good regardless of that one, and won't be magically fixed by that one.

c4rl’s picture

#14 @effulgentsia, thanks for chiming-in. Basically what would change if #1843798: [meta] Refactor Render API to be OO made it in is that the mechanism for reaching out to the variable alter functions would take on some sort of OO manifestation, but the purposes would be retained. I don't think parent elements need to drill through child elements, they just need methods to access each other.

I agree that we should proceed with this issue, there are some good ideas here.

The real culprit I think is the timeline of the release cycle -- it wouldn't be such a scramble if there were a sense we don't have to wait 3 years for better APIs. It could be that my interest in making Render API better will result in contributing to redefining our release cycle. :)

effulgentsia’s picture

Basically what would change if #1843798: [meta] Refactor Render API made it in is that the mechanism for reaching out to the variable alter functions would take on some sort of OO manifestation, but the purposes would be retained.

So, taking the example from the issue summary of that issue, say a theme's node.html.twig prints out:

{{ content.field_image.0.attributes.src }}

I'm assuming that the expectation would be that phases 3 (hook_theme_prepare() and hook_theme_prepare_THEME_ID()) and 4 (hook_theme_prepare_alter() and hook_theme_prepare_THEME_ID_alter()) would need to be run on the content element, and then on the content.field_image element, and then on the content.field_image.0 element. That doesn't impact this issue's proposal: integrating these phases into the drilling process is a responsibility for the other issue.

What I'm not clear on though is whether hook_theme_prepare_THEME_ID_SUGGESTION_ID_alter() would need to run on each of the elements. In other words, are suggestion-specific variable alters intended only for when the actually suggested template is invoked, or are they also needed for preparing the variable correctly even when printed from a parent element's template? Or put more simply, does it matter what the template suggestions for content.field_image or content.field_image.0 are when their variables are being printed by node.html.twig? The answer to that may affect how we want to structure these phases.

However, if more discussion is needed to answer that question, I think it's okay to proceed with this issue in the meantime, and be okay with doing some rework in the other issue if it turns out to be needed.

markhalliwell’s picture

I noticed module_and_theme_invoke_all, I think this is a must have but couldn't find a related issue about this. One small suggestion, maybe call it: extension_invoke_all instead.

effulgentsia’s picture

I don't think we need a module_and_theme_invoke_all() or extension_invoke_all(). Themes can already implement alter hooks. So while they wouldn't be able to participate in phase 3 (hook_theme_*_prepare()), they would be able to participate in phase 4 (hook_theme_*_prepare_alter()), and I think that's good enough for now. If I'm wrong about this and there's a need for themes to participate in phase 3, please explain a use case where that's needed, but I suggest opening a separate issue for that in order to not slow this one down.

However, I'm not sure if we can effectively move on this issue until after #1843650: Remove the process layer (hook_process and hook_process_HOOK) is done. If someone can outline a plan for doing these in parallel, please do so, but in the meantime, I highly suggest that anyone interested in this issue help get the other one done faster.

c4rl’s picture

However, I'm not sure if we can effectively move on this issue until after #1843650: [meta] Remove the process layer is done.

Yes, I'm actually confused how the proposed hook_theme_*_prepare() and hook_theme_*_prepare_alter() *physically* differ from the current preprocess/process layer (See #10). @jenlampton or @Fabianx, can you clarify?

Fabianx’s picture

There are several changes here:

* preprocess / process functions were replaced, not added to - this was unique to the theme layer and added more confusion than helped.
* template_x was completely unique and very strange to have as it had nothing to do with the module.

* It is a good idea to have modules _add_ to the preparation but not change it. (array merge made by module_invoke_all, like the old hook_menu).

The new API is exactly like form_api with: form definition and form_alter and form_FORM_ID_alter. While it is still similar, we have a distinct _preparation_ stage, which is just for going from:

$input to $variables.

Nothing more, nothing less.

@c4rl: I know you do not like the direction this is going and would prefer a pure OO approach. However this is doable now and will be almost as powerful.

There is no reason we need to remove the render arrays with objects.

We are actually OO within twig, its just not possible with pure PHP.

We could have something like:

$x['#drillable_callback'] = 'FactoryMethod/Function';

And while traversing Twig can do:

if (isset($x['#drillable_callback']) { // convert first ...

This can _then_ create an Object based on a factory method (and we plan to do that with #type => attributes to create Attribute() objects on the fly).

#18: I think you are right to make it work without themes for now for the prepare.

We are very much going into the direction finally of a "theme component library", where you have something like for example:

#type => 'item_list', '#theme' => 'user_list__anonymous'

And the preparation would really a basic preparation of input -> $variables, which probably is fine as a preparation.

And yes adding new "types" is probably not done by themes, so yes, that is totally fine.

@jenlampton and me wanted to make it possible to do much more in themes, but I see now that the distinction between theme and module is still helpful.

#18: Oh, this can still go on - even if process layer is not yet gone:

process is similar to _alter and for now I think we can at least try to

Besides that #2004286: Defer calls to drupal_get_* functions until printed inside a template by adding a RenderWrapper class is almost RTBC, so process is gone soon anyway.

#16: It does not matter where things are printed.

Drill-down == template embedded inside another template - like inline templating - without having to do anything.

effulgentsia’s picture

As I understand it, hook_theme_prepare_*() is primarily for the module that defines the theme hook, so that it runs first. For example, node.module's template_preprocess_node() would become node_theme_prepare_node(). Whereas, hook_theme_prepare_*_alter() would be for modules/themes to alter what was prepared. For example, bartik_preprocess_node() alters what was prepared in template_preprocess_node(), so would become bartik_theme_prepare_node_alter().

That is different from *_process(). Process is not currently used for altering, but for converting variables to a printable state (like converting arrays to strings), which is unnecessary in twig, since we can use objects with __toString() or render() methods for delaying those kinds of conversions until render time.

Come to think of it though, we can do this issue and #1843650: Remove the process layer (hook_process and hook_process_HOOK) in parallel. In this issue, we can remove the 'preprocess functions' from the theme registry building, but retain the 'process functions' (and invoke them between phases 4 and 5) until after that issue is done.

Fabianx’s picture

#21:

Yes, however node should for example add additional classes within an alter hook - if possible (performance wise) and not within the preparation stage.

To have a clean seperation of:

* Suggestions (alter)
* Preparation of Base Type / Theme ID for output by a template
* Altering of Variables
* Altering of Variables based on a suggestion

effulgentsia’s picture

Re #22: I agree with the goal of clean separation, but am not clear on why adding classes in an alter() is considered cleaner than doing it in prepare(). I would think of prepare() as being suitable for all additions: whether top-level variables, additional classes, or additional attributes (e.g., the ones in rdf.module). And that way, alter() is reserved for changing/removing top-level variables, classes, attributes, etc.

Fabianx’s picture

#23:

It is not generally "reserved", but lets start to think of hook_theme_prepare more like a hook_type_prepare (in the future).

So for an "item_list" or a generic "entity" you would for example not add a "node" class.

So I am assuming that most special casing goes away, but I am totally happy with a straight conversion for now as long as preparation and alter stages are separate.

So the idea is to be able to re-use things as much as possible.

So if I want to inherit the "node" theme thing without needing most of it, I could ...

But yes for now I can also just unset the classes I don't need.

There are really no strict rules around this.

-----

@c4rl brought up in IRC that optionally returning $vars or the rendered output is bad design and I'll move that part to follow-up for drillability. This is really the preparation to make this clean design work first.

markhalliwell’s picture

@effulgentsia: You're right, sorry. This is just the first time I saw that kind of function[ality] in a post (and I've been desiring it). Did a quick search and module/theme invokes are fix and now deprecated anyway in favor of OOP: #1331486: Move module_invoke_*() and friends to an Extensions class.

c4rl’s picture

#21:

That is different from *_process().

It is different in *intent* but not *physically* different in that AFAIK _prepare_alter() is intended to be called at the same time as _process() and receives the same arguments. We have two layers of template variable altering in D7, and this proposes we will have two layers of template variable altering in D8. Right? Since it was deemed that the _process layer wasn't *physically* necessary, I am wondering what 90% use-case we are trying to solve? Can someone provide an example?

#20:

There is no reason we need to remove the render arrays with objects. We are actually OO within twig, its just not possible with pure PHP.

I'm not sure I understand. What is "not possible" and why? If you mean "not practically possible" given the code freeze date, I agree that this present issue is lower-hanging fruit than #1843798: [meta] Refactor Render API to be OO. If you mean "not technically possible" I respectfully disagree and encourage you to follow-up in #1843798: [meta] Refactor Render API to be OO. :)

#20:

We could have something like: $x['#drillable_callback'] = 'FactoryMethod/Function';

Having a 1-to-1 mapping of #type to a factory class registry is of the principles of http://github.com/c4rl/renderapi. #type maps to SomeClass; this doesn't require #drillable_callback. If you have OO-related comments, it would be great to hear them on #1843798: [meta] Refactor Render API to be OO.

#24:

So the idea is to be able to re-use things as much as possible...There are really no strict rules around this.

One of the principles established at the BADCamp Theme Sprint https://groups.drupal.org/node/265858 is we need to work from use cases not "what-if" scenarios. Again, echoing my response to #21 above, can someone provide a coherent 90% use case where two layers of variable alteration are necessary (and could not be accommodated by hook_module_implements_alter)?

Fabianx’s picture

#21:

* Prepare alter == preprocess, prepare == new phase and themes can't use it.
* It is not possible to get a call for traversing pure PHP arrays, which are fast and easy to manipulate. It is possible for array objects.
* #drillable: I am only talking about my plans for now and not about far-in-the-future D9 things that make things much cleaner.
* There are no rules in how modules use _prepare_alter. If node wants to add the class in hook_prepare, sure it should go ahead, but the best practice would be to add it in the altering. But there are no rules for doing that or enforcing that.

c4rl’s picture

I think #27 means to be replying to #26 instead of #21.

Prepare alter == preprocess, prepare == new phase and themes can't use it.

It isn't clear from the issue description that themes can't use the _prepare() phase. I can speak for myself that I would likely have less questions about this issue if you provided better description in the issue summary and provided some code examples and use-cases. You seem to have a good grasp on these concepts as you have shown in the comments but the summary doesn't currently explain them as well as it could. Can you and/or @jenlampton revise?

It is not possible to get a call for traversing pure PHP arrays, which are fast and easy to manipulate. It is possible for array objects.

You misspoke in #20, I believe. You said "its just not possible with pure PHP." You didn't say "its just not possible with pure PHP *arrays*." Hence my misunderstanding.

#drillable: I am only talking about my plans for now and not about far-in-the-future D9 things that make things much cleaner.

If you believe #1843798: [meta] Refactor Render API to be OO is "far-in-the-future," please leave a comment at that issue explaining your position. Thanks. :)

If node wants to add the class in hook_prepare, sure it should go ahead, but the best practice would be to add it in the altering

Do you mean to say "If node module wants to add a class in hook_prepare not related to a theme callback that node module defines the best practice would be to add it in the altering?" If so, that makes sense to me. Again, I recommend adding this to the summary along with some code examples.

Fabianx’s picture

I think it will all clear up once there is some code to discuss :).

jenlampton’s picture

re #19 & #28

There is a link in the issue summary to a gist that has a full explanation and use-cases.
https://gist.github.com/jenlampton/5654283

jenlampton’s picture

Issue summary: View changes

Added implementation example for phase 3

c4rl’s picture

Issue summary: View changes

Clarify summary

star-szr’s picture

Issue summary: View changes

Add first sub-issue

star-szr’s picture

star-szr’s picture

Today on the Twig call we briefly discussed the idea of introducing the new prepare hooks (with some implementations but not all) and deprecating the usage of template_preprocess() functions. The hope is that this would allow us to split up and remove existing preprocess implementations even after code freeze since they are not part of the public API.

Does this sound feasible?

thedavidmeister’s picture

#32 - can we put prepare hooks in drupal_render() instead of theme and deprecate/remove #pre_render and maybe replace with #render for #type implementations that currently create #markup in the #pre_render? and could they be drupal_alter() instead of hook? and could drupal_render() enforce that the "original" variables are preserved as an attribute in the structured data, like the way we have the original entity available in entity update hooks, etc...

thedavidmeister’s picture

can we put prepare hooks in drupal_render() instead of theme and deprecate/remove #pre_render and maybe replace with #render for #type implementations that currently create #markup in the #pre_render?

Not yet, because of theme suggestions.

could drupal_render() enforce that the "original" variables are preserved as an attribute in the structured data

#2025259: drupal_render() should preserve the original render element in #original so pre and post render functions can access "raw" input

thedavidmeister’s picture

Issue summary: View changes

.

jenlampton’s picture

Issue summary: View changes

added another hook under API changes

jenlampton’s picture

Issue summary: View changes

add link to new prepare phase, add note about prepare_alter not happening.

thedavidmeister’s picture

for #33 and #34 - #2044105: Introduce #alters for \Drupal\Core\Render\Renderer::render()

Also, I thought I'd bring some attention to #2029819: Implement a ThemeHandler to manage themes (@see ModuleHandler) as the support for invoking alters and allowing the active theme to respond (functionality that is very important to the future adoption of this proposal) at the moment is not great.

thedavidmeister’s picture

Issue summary: View changes

Add information about theme_registry() changes

jenlampton’s picture

Issue summary: View changes

update

jenlampton’s picture

Issue summary: View changes

updte

jwilson3’s picture

Issue summary: View changes

Improved general readability and added some HTML structure

jwilson3’s picture

Issue summary: View changes

whitespace

jwilson3’s picture

Issue summary: View changes

Updated issue summary.

jwilson3’s picture

Issue summary: View changes

missing closing ul

jwilson3’s picture

Issue summary: View changes

Updated issue summary.

jwilson3’s picture

Issue summary: View changes

Updated issue summary.

jwilson3’s picture

Issue summary: View changes

I don't think this interpretation was correct, so changing it back.

jwilson3’s picture

Issue summary: View changes

Updated issue summary.

jwilson3’s picture

Issue summary: View changes

Clean up documentation about hook_theme

jwilson3’s picture

Issue summary: View changes

Updated issue summary.

jwilson3’s picture

Issue summary: View changes

oops

jwilson3’s picture

Issue summary: View changes

Clean up Phase 1 explanation

jwilson3’s picture

Issue summary: View changes

Updated issue summary.

jwilson3’s picture

Issue summary: View changes

More phase 1 cleanup.

star-szr’s picture

Issue summary: View changes

Adding anchor links.

rafamd’s picture

Issue summary: View changes
mgifford’s picture

star-szr’s picture

Version: 8.0.x-dev » 9.x-dev
Status: Active » Postponed

Bumping to 9.x since all the children are there, I think we've done what we can here for 8.x.x.

catch’s picture

Version: 9.x-dev » 8.1.x-dev
Category: Task » Plan

At least some of the children look like they'd still be viable for a minor release. As long as we keep a BC layer there's still a lot we can do. Moving back for now.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

joelpittet’s picture

Status: Postponed » Active

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

markhalliwell’s picture

Status: Active » Closed (outdated)

This issue is more or less "outdated".

As @Cottser said in #39, we've really done as much as we can do for this particular plan in 8.x. The only thing we can really do now (for the current theme system APIs) is something like #2954402: Refactor ThemeManager::render(), split into smaller immutable objects., which is really just splitting things up into different classes, not actual API fixes/additions.

The other reason I'm closing this is in favor of #2821399: [meta] Theme System Modernization Initiative and #2869859: [PP-1] Refactor theme hooks/registry into plugin managers which completely replaces the proposed "plan" here.

This issue really should have been closed long ago as it simply became an open-ended "catch-all" meta issue.

So I'm closing this.