You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
According to the Drupal 8 Backward Compatibility Policy, constructors for service objects are considered internal, which means they are subject to change on a minor release:
The constructor for a service object (one in the container), plugin, or controller is considered internal unless otherwise marked, and may change in a minor release. These are objects where developers are not expected to call the constructor directly in the first place. Constructors for value objects where a developer will be calling the constructor directly are excluded from this statement.
The implication is that if we override their constructors, and they happen to change their signature on a minor release, it would require us to update our current code for all instances where we're overriding their constructors, to conform to the new signature. This shouldn't have to happen on a minor release.
Some of the most used Drupal contrib modules are already using this design pattern, like the aforementioned Webform, Migrate Plus, Search API, etc.
Some extra benefits from this approach also include less code bloat:
We no longer need a __construct() method, since the __construct() and create() methods are effectively merged into one.
We no longer need a use statement per service injected. Their only purpose was so we could typehint on the constructor.
That reduces the final code lines by quite a bit. Also due to the above, adding a new service dependency injection after already running the generate command is no longer a major PITA.
I think this solution would include:
A change in drupal-console-core's TwigRenderer method to add a new twig function, similar to serviceClassInjection, that would use this new design pattern.
A change to the generator twig template files, to use this new function, and also remove the now unnecessary use statements as well as the constructor.
I have PR's ready for the new function on TwigRenderer and a change to the Controller generator to use it.
This solution could be applied to the following generators:
generate:controller
generate:entity:content
generate:form
generate:form:config
generate:plugin:block
generate:plugin:condition
generate:plugin:imageformatter
generate:plugin:mail
generate:plugin:rest:resource
generate:plugin:skeleton
The text was updated successfully, but these errors were encountered:
* Issue #4169 - generate:controller
* Issue #4169 - generate:form:config
* Issue #4169 - generate:form
* Smallfix.
Adds spacing between Twig `use_class_services` block declarations.
* Issue #4169 - generate:plugin:block
* Issue #4169 - generate:plugin:mail
* Issue #4169 - generate:plugin:skeleton
This one has a lot of coding standard errors,
as well as a malformed namespace.
I have not attempted to fix those, as they are
out of scope for this issue, although I fixed
them for the create() function, since that's
directly related to what I'm changing.
* Issue #4169 - generate:plugin:rest:resource
This one has some coding standard errors.
I have not attempted to fix those,
as they are out of scope for this issue, although I fixed
them for the create() function and the
currentUser property definition, since that's
directly related to what I'm changing.
* Issue #4169 - generate:plugin:condition
* Issue #4169 - generate:plugin:imageformatter
This one has some coding standard errors.
I have not attempted to fix those,
as they are out of scope for this issue, although I fixed
them for the create() function and the
property definitions, since that's
directly related to what I'm changing.
* Issue #4169 - generate:entity:content
Issue title
Problem/Motivation
According to the Drupal 8 Backward Compatibility Policy, constructors for service objects are considered internal, which means they are subject to change on a minor release:
The implication is that if we override their constructors, and they happen to change their signature on a minor release, it would require us to update our current code for all instances where we're overriding their constructors, to conform to the new signature. This shouldn't have to happen on a minor release.
There is a good blog post from jrockwitz (maintainer of Webform for D8), that goes into more detail: https://www.jrockowitz.com/blog/webform-road-to-drupal-9
Solution
We can implement a new design pattern that would fix the problem.
As an example, from the
generate:controller
command, injecting 3 services as an example.Before:
After:
Some of the most used Drupal contrib modules are already using this design pattern, like the aforementioned Webform, Migrate Plus, Search API, etc.
Some extra benefits from this approach also include less code bloat:
use
statement per service injected. Their only purpose was so we could typehint on the constructor.That reduces the final code lines by quite a bit. Also due to the above, adding a new service dependency injection after already running the generate command is no longer a major PITA.
I think this solution would include:
serviceClassInjection
, that would use this new design pattern.I have PR's ready for the new function on TwigRenderer and a change to the Controller generator to use it.
This solution could be applied to the following generators:
The text was updated successfully, but these errors were encountered: