Problem/Motivation
We currently have this in our LibraryDiscoveryParser class:
if ($extension === 'core') {
$path = 'core';
$extension_type = 'core';
}
else {
if ($this->moduleHandler->moduleExists($extension)) {
$extension_type = 'module';
}
else {
$extension_type = 'theme';
}
$path = $this->drupalGetPath($extension_type, $extension);
}
This was only meant as a temporary 'hack' with the plan to remove this in favour of getting this from a parameter instead.
Proposed resolution
Either:
1. Pass the $type into LibraryDiscoveryInterface::getLibrariesByExtension/getLibraryByName
2. Pass an Extension object around instead, as this hold the extension type ($extension->getType())
3. Determine the extension type another way?
Remaining tasks
Discuss, decide, patch, tests.
User interface changes
None
API changes
None
Comments
Comment #1
sunComment #8
rodrigoaguileraI think that code is now at
LibraryDiscoveryParser
I found a bug trying to get a library from a module that was not enabled with
LibraryDiscovery::getLibraryByName
I feel the solution is closely related to this issue.
I think it can be solved by following proposed resolution number 3. That way we can avoid changing the API since we can detect the type of the extension using handler services available.
If we can guess the type there is no need to pass it earlier.
Let me illustrate first with the attached test test (I hope it produces the warning mentioned above)
Comment #10
rodrigoaguileraAfter reading more issues I realized this will be half-solved by
#2347783: Deprecate drupal_get_path() and drupal_get_filename() and replace with ExtensionList::getPath() and ExtensionList::getPathname()
Because at least it will raise an exception.
Let's wait for that issue to be solved before addressing this.
Comment #13
gábor hojtsyThis is causing issues for deprecated library testing at #3067965: Detect use of deprecated libraries in extending libraries, twig templates and PHP.
Comment #17
daffie commented#2347783: Deprecate drupal_get_path() and drupal_get_filename() and replace with ExtensionList::getPath() and ExtensionList::getPathname() has landed.
Comment #18
larowlanTriaged this as part of Bug smash initiative
As this is an API cleanup, its less of a bug and more of a task
Reclassifying as such
Comment #19
larowlanThis is a duplicate of #2808063: LibraryDiscoveryParser::buildByExtension() doesn't validate that extensions exist - applying credit over there