Adding callback_form_element_process (#process) to Form Element removes system callbacks

There is a Drupal 7 era thread about this: https://drupal.stackexchange.com/questions/49216/how-do-i-add-a-process-...

The difficulty here is changing it will be a BC break for everyone who's just coded around it by using array merge and calling the original element info

So perhaps it's a docs issue and we should advocate that folks do that

Also, with this proposed change, it is no longer possible to fully replace or remove the callbacks from '#type'.

A solution could be to introduce a placeholder callback, so that we can:

$element = [
  '#type' => 'radios',
  '#process' => [
    'my_custom_process_callback',
    Element::PROCESS_CALLBACK_PLACEHOLDER,
    'my_custom_late_process_callback',
  ],
];

and we get this:

$element = [
  '#type' => 'radios',
  '#process' => [
    'my_custom_process_callback',
    [Radios::class, 'processRadios'],
    'my_custom_late_process_callback',
  ],
];

The placeholder should be a valid callback that, if not replaced, just returns the first parameter.
At the same time, it should be a constant that is easy to compare against or look up in an array.

This leaves some design choices to make:
- What exactly is the placeholder value? If it is a static method, how do we call the class and method?
- Where do we do the placeholder replacement? I was thinking of a new service ElementTypePopulator, which handles all of the '#type' replacement, so that we don't have to clutter up Renderer and FormBuilder.

(I did post some proof of concept code in a slack convo, but I am still too undecided about the class naming.)

It seems for all the callback types #process, #after_build, #pre_render and #post_render, the placeholder can be a simple identify function.
fn ($x) => $x
(but as regular named function or static method, not as closure)

Can't Element::PROCESS_CALLBACK_PLACEHOLDER be a method that just does this:

1. look at #type in the render array it's working on
2. get the Element plugin class for that #type
3. call ELEMENTCLASS::getInfo()
4. get the callback names from the returned info array
5. call them

To resolve this, you need to ensure that your custom process callback is added while preserving the existing callbacks. Modify your my_module_field_widget_form_alter function to append the custom callback to the existing #process array, rather than replacing it.

function my_module_field_widget_form_alter(&$element, \Drupal\Core\Form\FormStateInterface $form_state, $context) {
  if ($context['items']->getFieldDefinition()->getName() !== 'field_tags') {
    return;
  }
  
  if (isset($element['#process'])) {
    $element['#process'][] = '_my_module_field_widget_form_process';
  }
  else {
    $element['#process'] = ['_my_module_field_widget_form_process'];
  }
}

@joachim

Can't Element::PROCESS_CALLBACK_PLACEHOLDER be a method that just does this:

1. look at #type in the render array it's working on
2. get the Element plugin class for that #type
3. call ELEMENTCLASS::getInfo()
4. get the callback names from the returned info array
5. call them

Yes, this would also do the trick.
However, it does require a call to \Drupal::service(), if it is to be a constant.
Or alternatively, we could make this a class with __invoke() and DependencySerializationTrait. Because these things tend to get serialized. Actually not so bad!

call ELEMENTCLASS::getInfo()

Just to clarify, we would not call it like this, because it is not a static method.
Instead, we would call `$info = $this->elementInfoManager->getInfo($element['#type'])` as we already do in FormBuilder and Renderer.
I am working on something, just need to add tests..

I created a new MR with these placeholder objects following the idea by @joachim.

I want to add tests, but not sure yet where to put them.

Here we go, some unit tests.
Let's discuss if this is a good direction, and how we can refine it.

We can't use readonly in combination with DependencySerializationTrait.

I am setting to "Needs review" for one round of feedback, but I am sure we will have to change some parts.

Perhaps we want to move the ElementProcessCallback* classes to `Drupal\Core\Form\` namespace?
For now I had it in `Drupal\Core\Form\Render\Callback`, because form element type plugins are typically in the same namespace as render element type plugins.
I ma not changing it now because I want to see more feedback first.

Can't Element::PROCESS_CALLBACK_PLACEHOLDER be a method that just does this:

Yes, this would also do the trick.
However, it does require a call to \Drupal::service(), if it is to be a constant.
Or alternatively, we could make this a class with __invoke() and DependencySerializationTrait. Because these things tend to get serialized. Actually not so bad!

Actually one more problem with this approach.
If multiple operations (e.g. form alter hooks) want to add '#process' callbacks, while preserving the element type default callbacks, they need to make sure that the element defaults are not executed more than once.

If the callback is a constant, we can easily check for in_array().
But if it is an object?
Actually, in_array(*, *, FALSE) applies weak object comparison, and this actually seems to work in this case.
But I am a bit afraid of weak object comparisons, I don't trust them.

If multiple operations (e.g. form alter hooks) want to add '#process' callbacks, while preserving the element type default callbacks, they need to make sure that the element defaults are not executed more than once.

The following pattern works in most cases:

          $element['#process'] ??= [new ElementTypeProcessCallback($this->elementInfoManager)];
          $element['#process'][] = 'my_process_callback';

@donquixote which MR? The newer one in draft?

@mdsohaib4242 that is exactly what the code in the issue summary is doing, that's why this issue exists.

I notice that '#pre_render' does not allow object with __invoke() method, if it is not a closure.

See #3498999: Support object with __invoke() for #pre_render.

(We can eventually handle #pre_render in a separate issue, but for now I find it interesting to include it here as proof-of-concept, to see the bigger picture of where this is going.)

@donquixote which MR? The newer one in draft?

Well, I am proposing to use the newer MR which is in draft.
Imo the old MR is a dead end, because it breaks existing behavior and makes it impossible to fully remove/replace the callbacks from #type.

But I don't want to remove the old one yet, unless everybody agrees on the new direction.
Having both means we can discuss which direction is preferable.

@mdsohaib4242
Actually this does not do anything, because at the time you add your own '#process' callback, the '#process' array is still empty.
The would-be-replaced callbacks are added later by the form builder which is after hook_form_alter() based on $element['#type'].

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

The issue is still relevant. I encountered it in version 11.1.5.
When creating a custom form, if you add a #pre_render callback in element, the subsequent code does not add #pre_render callbacks.