Problem/Motivation

After moving RenderElement and FormElement plugin discovery from annotations to attributes in #3421439: Convert form and render element plugin discovery to attributes, it has often become necessary in many element plugin implementation to import "RenderElement" or "FormElement" classes from two different namespaces for each plugin type:

  1. \Drupal\Core\Render\Element\RenderElement and \Drupal\Core\Render\Attribute\RenderElement
  2. \Drupal\Core\Render\Element\FormElement and \Drupal\Core\Render\Attribute\FormElement

This can have a negative impact on developer experience, because IDEs such as PHPStorm often do not provide typehint/autocomplete for the class name if a use statement was already added,or if the referenced class exists in the same namespace as the current one.

Additionally, \Drupal\Core\Render\Element\RenderElement and\Drupal\Core\Render\Element\FormElement are abstract classes, and there is convention in adding a "Base" suffix to these types of classes.

Suggestion here is to call these classes RenderElementBase and FormElementBase.

Steps to reproduce

Proposed resolution

  1. Create abstract class \Drupal\Core\Render\Element\RenderElementBase and move all the contents of \Drupal\Core\Render\Element\RenderElement there
  2. to Create abstract class \Drupal\Core\Render\Element\FormElementBase and move all the contents of \Drupal\Core\Render\Element\FormElement there
  3. Set \Drupal\Core\Render\Element\RenderElement to extend \Drupal\Core\Render\Element\RenderElementBase
  4. Set \Drupal\Core\Render\Element\FormElement to extend \Drupal\Core\Render\Element\FormElementBase
  5. Add deprecation messages for \Drupal\Core\Render\Element\RenderElement and \Drupal\Core\Render\Element\FormElement
  6. Add CR

Remaining tasks

User interface changes

API changes

  1. \Drupal\Core\Render\Element\RenderElement renamed to \Drupal\Core\Render\Element\RenderElementBase
  2. \Drupal\Core\Render\Element\FormElement renamed to \Drupal\Core\Render\Element\FormElementBase

No other changes

Data model changes

N/a

Release notes snippet

As this is a simple rename with the API left in place to Drupal 12 I don't think this warrants a release note.

Issue fork drupal-3431452

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

godotislate created an issue. See original summary.

godotislate’s picture

godotislate
he/him
CreditAttribution: godotislate at Digital Polygon commented
Issue summary: View changes
godotislate’s picture

godotislate
he/him
CreditAttribution: godotislate at Digital Polygon commented
Issue summary: View changes

godotislate opened merge request !7195

godotislate’s picture

godotislate
he/him
CreditAttribution: godotislate at Digital Polygon commented
Status: Active » Needs review

Created CR: https://www.drupal.org/node/3436275
MR with updated test for deprecation: https://git.drupalcode.org/project/drupal/-/merge_requests/7195

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Needs work

This actually seems like a large change, probably safer to deprecated for removal in D12

godotislate’s picture

godotislate
he/him
CreditAttribution: godotislate at Digital Polygon commented
Status: Needs work » Needs review

Updated CR and MR to 12 for removal version.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Needs work

Thanks appears to need manual rebasing though.

godotislate’s picture

godotislate
he/him
CreditAttribution: godotislate at Digital Polygon commented
Status: Needs work » Needs review

Rebased and resolved conflict.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Reviewed & tested by the community

THanks conflict appears to be resolved.

godotislate’s picture

godotislate
he/him
CreditAttribution: godotislate at Digital Polygon commented

Thanks for reviewing.

Separately, I wish that there were a similar way to deal with validation Constraint plugins, since my guess is that there are a lot more contrib and custom validation plugins being written than render element plugins, but since Constraint is a Symfony class, not sure there's a solution.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Needs work

As things are called statically on the classes we need to issue deprecations from there too because this will avoid the constructor.

So for example we need to add things like \Drupal\Core\Render\Element\FormElement::processPattern() with the code:

  /**
   * {@inheritdoc}
   */
  public static function processPattern(&$element, FormStateInterface $form_state, &$complete_form) {
    @trigger_error('\Drupal\Core\Render\Element\FormElement::processPattern() is deprecated in drupal:10.3.0 and is removed from drupal:12.0.0. Use \Drupal\Core\Render\Element\FormElementBase::processPattern() instead. See https://www.drupal.org/node/3436275', E_USER_DEPRECATED);
    return parent::processPattern($element, $form_state, $complete_form);
  }

Not pretty but the best we can do.

godotislate’s picture

godotislate
he/him
CreditAttribution: godotislate at Digital Polygon commented
Status: Needs work » Needs review

Added deprecation warnings to all the static methods with corresponding tests.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Reviewed & tested by the community

Will remember that about static methods going forward

Deprecations look good though.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Version: 11.x-dev » 10.3.x-dev
Issue summary: View changes
Status: Reviewed & tested by the community » Fixed

@godotislate thanks for getting this done. It's great to have this complete before people start to covert to attributes as it was pretty icky.

Committed and pushed 8d1ff20647 to 11.x and b89ad7f9c8 to 10.3.x. Thanks!

  • alexpott committed b89ad7f9 on 10.3.x 
    Issue #3431452 by godotislate, smustgrave, alexpott: Rename...

  • alexpott committed 8d1ff206 on 11.x 
    Issue #3431452 by godotislate, smustgrave, alexpott: Rename...

godotislate closed merge request !7195

Status: Fixed » Closed (fixed)

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

scott_euser’s picture

scott_euser CreditAttribution: scott_euser as a volunteer and at Soapbox commented

I guess it's not possible to backport empty RenderElementBase and FormElementBase classes to 10.2 that extend the old ones to allow contrib to fix depreciations now vs having to wait until end of 10.2 security coverage on December 9th? Happy to do the legwork there if it is an option but I guess it's not as no further non-security releases to 10.2 right?