Consider implementing static create method in restriction plugin base

Created on 2 August 2022, over 2 years ago
Updated 16 May 2024, 6 months ago

Problem/Motivation

Constructors are considered internal implementation details without B/C guarantees. Currently, restriction plugin implementors extending LayoutBuilderRestrictionBase need to implement ::create themselves, which requires a call to an internal constructor, even if they have no desire to inject services of their own.

If an upstream change in the constructor happens (such as in Drupal Core), this change could propagate breakage through Layout Builder Restrictions and into custom code.

See Change record: Code refactored to not override constructors for service objects, plugins, and controllers for precedent in contrib for reacting to this design pattern.

Proposed resolution

We can shield custom code against disruption by providing a base implementation of the factory method within LayoutBuilderRestrictionBase. This allows downstream code to stay on the good side of B/C guarantees.

Base:

  /**
   * {@inheritdoc}
   */
  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
    return new static($configuration, $plugin_id, $plugin_definition); // <-- technically, still using an internal API :-(
  }

Downstream

// EntityViewModeRestriction.php
  /**
   * {@inheritdoc}
   */
  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
    $instance = parent::create($container, $configuration, $plugin_id, $plugin_definition); <-- yay, no B/C worries :-)
    $instance->moduleHandler = $container->get('module_handler');
    $instance->database = $container->get('database');
    return $instance;
  }

Similar improvements can happen in all downstream plugin implementations, essentially consolidating the only area of B/C concern to the base implementation. This should serve to harden this module against disruptive upstream changes moving forward.

Remaining tasks

Decide if this is something we want to do.
Determine how far we want to take it -- do we leave the overridden constructors in place in the 2.x branch and remove it in 3.x?
Open MR, commit, and be happy :-)

User interface changes

None

API changes

Possible plugin constructor change...?

Data model changes

None

πŸ“Œ Task
Status

Active

Version

3.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States luke.leber Pennsylvania

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

Production build 0.71.5 2024