Updated: Comment #0


Drupal 7 had the useful function drupal_parse_dependency to parse dependencies declared in the "example (>7.x-2.3)" format.

This was useful for contrib and was used in Libraries API as libraries use the same mechanism to declare dependencies.

In Drupal 8 the parsing is now done in the protected function \Drupal\Core\Extension\ModuleHandler::parseDependency(). The function is therefore inaccessible to contrib and would have to be copied. This is a regression in DX.

Proposed resolution

Make \Drupal\Core\Extension\ModuleHandler::parseDependency() public.

Since it is a utility function and does not access object state at all, tstoeckler proposes that it be made static as well. This is what we do with other utility methods (see \Drupal\(Core/Component)\Utility) and so should not offend OO purists any more than existing code. I am also open to moving this method to a separate utility class in case people find that this would be "tainting" ModuleHandler.

API changes




use Drupal\Core\Extension\ModuleHandler;


Note that there is currently no change notice for the removal of drupal_parse_dependency() :-/

#1 drupal8.parse-dependency-public.2068797-1.patch1.18 KBtstoeckler
PASSED: [[SimpleTest]]: [MySQL] 58,185 pass(es). View


tstoeckler’s picture

Status: Active » Needs review
1.18 KB
PASSED: [[SimpleTest]]: [MySQL] 58,185 pass(es). View

Here we go.

Status: Needs review » Needs work
Issue tags: -DX (Developer Experience), -regression, -Needs change record

The last submitted patch, drupal8.parse-dependency-public.2068797-1.patch, failed testing.

tstoeckler’s picture

Status: Needs work » Needs review
Issue tags: +DX (Developer Experience), +regression, +Needs change record

#1: drupal8.parse-dependency-public.2068797-1.patch queued for re-testing.

I think this is a random error: SearchMatchTest.php, l. 30

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

This would also make it easier to unit test it later!

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

tstoeckler’s picture

Added a change notice.


tstoeckler’s picture

fubhy’s picture

Sorry, but why did we make this public instead of moving it to 'Components\Utility". It has basically 0 to do with ModuleHandler as a service except that it uses it.

fubhy’s picture

Just read this:

I am also open to moving this method to a separate utility class in case people find that this would be "tainting" ModuleHandler.

So, yes, it's "tainting" ModuleHandler. I will just post a quick follow-up patch right here to move it to a utility component. Any suggestions for good name for that class?

fubhy’s picture

xjm’s picture

Issue tags: -Needs change record

Untagging. Please remove the "Needs change notification" tag when the change notice task is complete.

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