Problem/Motivation
General community consensus is that global \Drupal calls should never be used in OO code except in edge cases and scenarios where DI is not possible. However, this philosophy, its reasoning, and how it should be applied are not actually reflected clearly in our standards.
Benefits
If we adopted this change, the Drupal Project would benefit by ...
Three supporters required
- https://www.drupal.org/u/{userid} (yyyy-mm-dd they added support)
- https://www.drupal.org/u/{userid} (yyyy-mm-dd they added support)
- https://www.drupal.org/u/{userid} (yyyy-mm-dd they added support)
Proposed changes
Compile a comprehensive list of situations where DI is not possible/appropriate and \Drupal calls are the only viable option, and add them to a handbook page along with information on why it's advisable to use DI when possible. Some potential cases brought up in the slack discussion on the topic:
@jhodgdon
Drupal is also used a lot in Functional tests.
@larowlan
Entities, field type plugins and data type plugins currently have no scope for DI
@mikelutz
\Drupal calls are also used in core to maintain backwards compatibility. We can’t just add a new dependency injection, we have to make it optional, get it by \Drupal and throw a deprecation error if it isn’t passed so that existing code continues to work, We can eventually make the parameter required in the next major version and remove the \Drupal call
@mxr576
adding a new parameter to a service as nullable, retrieving the service from \Drupal::service() if the parameter is null and triggering and error
I'll add that static methods were brought up a couple of times. But, unless the static class exists in one of the above contexts this feels like code smell and should probably be re-implemented in a service.
For entities, it would seem that https://www.drupal.org/project/drupal/issues/2142515 is relevant.
1. {link to the documentation heading that is to change}
Current text
Add current text in blockquotes
Proposed text
Add proposed text in blockquotes
2. Repeat the above for each page or sub-page that needs to be changed.
Remaining tasks
Create this issue in the Coding Standards queue, using the defined template- Add supporters
- Create a Change Record
- Review by the Coding Standards Committee
- Coding Standards Committee takes action as required
- Discussed by the Core Committer Committee, if it impacts Drupal Core
- Final review by Coding Standards Committee
- Documentation updates
- Edit all pages
- Publish change record
- Remove 'Needs documentation edits' tag
- If applicable, create follow-up issues for PHPCS rules/sniffs changes
For a full explanation of these steps see the Coding Standards project page
Comments
Comment #2
mrweiner commentedComment #3
ighosh commentedGlobal \Drupal calls must be used and NOT DI (Dependency Injection) in every place where classes can't be used AND in static functions. Some examples of types of files where classes are not used - .install | .module. In all other situations, DI must be used. Do correct me if I made any mistake. Cheers!
Comment #4
drunken monkeyThere are lots of classes, like entity classes, where DI is currently not supported. See the issue summary.
Comment #5
quietone commentedConverting to the new issue template. This needs work to state the proposed text and where it would be added.