From 73e5bfad8b6a00b6bca0c1dc49f27c654d2c4db5 Mon Sep 17 00:00:00 2001 From: catch Date: Thu, 8 Jan 2015 09:54:25 +0000 Subject: [PATCH] Issue #2358981 by tadityar, tstoeckler, larowlan, mpdonadio, Devin Carlson: Provide a mechanism for dynamic library declarations --- .../Drupal/Core/Asset/LibraryDiscovery.php | 7 ++ .../Core/Asset/LibraryDiscoveryInterface.php | 5 ++ .../Core/Asset/LibraryDiscoveryParser.php | 39 ++++++----- .../src/Tests/Common/AttachedAssetsTest.php | 22 +++++++ .../modules/common_test/common_test.module | 23 +++++++ core/modules/system/theme.api.php | 64 +++++++++++++++++++ .../Core/Asset/LibraryDiscoveryParserTest.php | 5 +- 7 files changed, 147 insertions(+), 18 deletions(-) diff --git a/core/lib/Drupal/Core/Asset/LibraryDiscovery.php b/core/lib/Drupal/Core/Asset/LibraryDiscovery.php index cd7a88668395..c79674a10ada 100644 --- a/core/lib/Drupal/Core/Asset/LibraryDiscovery.php +++ b/core/lib/Drupal/Core/Asset/LibraryDiscovery.php @@ -79,4 +79,11 @@ class LibraryDiscovery implements LibraryDiscoveryInterface { return isset($extension[$name]) ? $extension[$name] : FALSE; } + /** + * {@inheritdoc} + */ + public function clearCachedDefinitions() { + $this->collector->clear(); + } + } diff --git a/core/lib/Drupal/Core/Asset/LibraryDiscoveryInterface.php b/core/lib/Drupal/Core/Asset/LibraryDiscoveryInterface.php index ac77f70c9db9..c10ac819e00c 100644 --- a/core/lib/Drupal/Core/Asset/LibraryDiscoveryInterface.php +++ b/core/lib/Drupal/Core/Asset/LibraryDiscoveryInterface.php @@ -50,4 +50,9 @@ interface LibraryDiscoveryInterface { */ public function getLibraryByName($extension, $name); + /** + * Clears static and persistent library definition caches. + */ + public function clearCachedDefinitions(); + } diff --git a/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php b/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php index 5ff88b2d63e2..5ca5f26e1980 100644 --- a/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php +++ b/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php @@ -13,6 +13,7 @@ use Drupal\Core\Asset\Exception\LibraryDefinitionMissingLicenseException; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Component\Serialization\Exception\InvalidDataTypeException; use Drupal\Component\Serialization\Yaml; +use Drupal\Component\Utility\NestedArray; /** * Parses library files to get extension data. @@ -77,15 +78,11 @@ class LibraryDiscoveryParser { $path = $this->drupalGetPath($extension_type, $extension); } - $library_file = $path . '/' . $extension . '.libraries.yml'; - - if ($library_file && file_exists($this->root . '/' . $library_file)) { - $libraries = $this->parseLibraryInfo($extension, $library_file); - } + $libraries = $this->parseLibraryInfo($extension, $path); foreach ($libraries as $id => &$library) { if (!isset($library['js']) && !isset($library['css']) && !isset($library['drupalSettings'])) { - throw new IncompleteLibraryDefinitionException(sprintf("Incomplete library definition for '%s' in %s", $id, $library_file)); + throw new IncompleteLibraryDefinitionException(sprintf("Incomplete library definition for definition '%s' in extension '%s'", $id, $extension)); } $library += array('dependencies' => array(), 'js' => array(), 'css' => array()); @@ -102,7 +99,7 @@ class LibraryDiscoveryParser { // If this is a 3rd party library, the license info is required. if (isset($library['remote']) && !isset($library['license'])) { - throw new LibraryDefinitionMissingLicenseException(sprintf("Missing license information in library definition for '%s' in %s: it has a remote, but no license.", $id, $library_file)); + throw new LibraryDefinitionMissingLicenseException(sprintf("Missing license information in library definition for definition '%s' extension '%s': it has a remote, but no license.", $id, $extension)); } // Assign Drupal's license to libraries that don't have license info. @@ -209,8 +206,8 @@ class LibraryDiscoveryParser { * * @param string $extension * The name of the extension that registered a library. - * @param string $library_file - * The relative filename to the DRUPAL_ROOT of the wanted library file. + * @param string $path + * The relative path to the extension. * * @return array * An array of parsed library data. @@ -218,14 +215,26 @@ class LibraryDiscoveryParser { * @throws \Drupal\Core\Asset\Exception\InvalidLibraryFileException * Thrown when a parser exception got thrown. */ - protected function parseLibraryInfo($extension, $library_file) { - try { - $libraries = Yaml::decode(file_get_contents($this->root . '/' . $library_file)); + protected function parseLibraryInfo($extension, $path) { + $libraries = []; + + $library_file = $path . '/' . $extension . '.libraries.yml'; + if (file_exists($this->root . '/' . $library_file)) { + try { + $libraries = Yaml::decode(file_get_contents($this->root . '/' . $library_file)); + } + catch (InvalidDataTypeException $e) { + // Rethrow a more helpful exception to provide context. + throw new InvalidLibraryFileException(sprintf('Invalid library definition in %s: %s', $library_file, $e->getMessage()), 0, $e); + } } - catch (InvalidDataTypeException $e) { - // Rethrow a more helpful exception to provide context. - throw new InvalidLibraryFileException(sprintf('Invalid library definition in %s: %s', $library_file, $e->getMessage()), 0, $e); + + // Allow modules to add dynamic library definitions. + $hook = 'library_info_build'; + if ($this->moduleHandler->implementsHook($extension, $hook)) { + $libraries = NestedArray::mergeDeep($libraries, $this->moduleHandler->invoke($extension, $hook)); } + // Allow modules to alter the module's registered libraries. $this->moduleHandler->alter('library_info', $libraries, $extension); diff --git a/core/modules/system/src/Tests/Common/AttachedAssetsTest.php b/core/modules/system/src/Tests/Common/AttachedAssetsTest.php index aaf770ee94d1..8401acfbfbf5 100644 --- a/core/modules/system/src/Tests/Common/AttachedAssetsTest.php +++ b/core/modules/system/src/Tests/Common/AttachedAssetsTest.php @@ -374,6 +374,28 @@ class AttachedAssetsTest extends KernelTestBase { $this->assertTrue(strpos($scripts, 'core/assets/vendor/jquery-form/jquery.form.js'), 'Altered library dependencies are added to the page.'); } + /** + * Dynamically defines an asset library and alters it. + */ + function testDynamicLibrary() { + /** @var \Drupal\Core\Asset\LibraryDiscoveryInterface $library_discovery */ + $library_discovery = \Drupal::service('library.discovery'); + // Retrieve a dynamic library definition. + // @see common_test_library_info_build() + \Drupal::state()->set('common_test.library_info_build_test', TRUE); + $library_discovery->clearCachedDefinitions(); + $dynamic_library = $library_discovery->getLibraryByName('common_test', 'dynamic_library'); + $this->assertTrue(is_array($dynamic_library)); + if ($this->assertTrue(isset($dynamic_library['version']))) { + $this->assertIdentical('1.0', $dynamic_library['version']); + } + // Make sure the dynamic library definition could be altered. + // @see common_test_library_info_alter() + if ($this->assertTrue(isset($dynamic_library['dependencies']))) { + $this->assertIdentical(['core/jquery'], $dynamic_library['dependencies']); + } + } + /** * Tests that multiple modules can implement libraries with the same name. * diff --git a/core/modules/system/tests/modules/common_test/common_test.module b/core/modules/system/tests/modules/common_test/common_test.module index 8dbf98a236b3..c463f20474b0 100644 --- a/core/modules/system/tests/modules/common_test/common_test.module +++ b/core/modules/system/tests/modules/common_test/common_test.module @@ -164,6 +164,22 @@ function common_test_preprocess_common_test_render_element(&$variables) { $variables['#attached']['library'][] = 'test/specific_preprocess'; } +/** + * Implements hook_library_info_build(). + */ +function common_test_library_info_build() { + $libraries = []; + if (\Drupal::state()->get('common_test.library_info_build_test')) { + $libraries['dynamic_library'] = [ + 'version' => '1.0', + 'css' => [ + 'common_test.css' => [], + ], + ]; + } + return $libraries; +} + /** * Implements hook_library_info_alter(). */ @@ -174,6 +190,13 @@ function common_test_library_info_alter(&$libraries, $module) { // Make Farbtastic depend on jQuery Form to test library dependencies. $libraries['jquery.farbtastic']['dependencies'][] = 'core/jquery.form'; } + + // Alter the dynamically registered library definition. + if ($module == 'common_test' && isset($libraries['dynamic_library'])) { + $libraries['dynamic_library']['dependencies'] = [ + 'core/jquery', + ]; + } } /** diff --git a/core/modules/system/theme.api.php b/core/modules/system/theme.api.php index eb0930775fef..c083f7ff7e3c 100644 --- a/core/modules/system/theme.api.php +++ b/core/modules/system/theme.api.php @@ -721,6 +721,70 @@ function hook_js_alter(&$javascript) { $javascript['core/assets/vendor/jquery/jquery.min.js']['data'] = drupal_get_path('module', 'jquery_update') . '/jquery.js'; } +/** + * Add dynamic library definitions. + * + * Modules may implement this hook to add dynamic library definitions. Static + * libraries, which do not depend on any runtime information, should be declared + * in a modulename.libraries.yml file instead. + * + * @return array[] + * An array of library definitions to register, keyed by library ID. The + * library ID will be prefixed with the module name automatically. + * + * @see core.libraries.yml + * @see hook_library_info_alter() + */ +function hook_library_info_build() { + $libraries = []; + // Add a library whose information changes depending on certain conditions. + $libraries['mymodule.zombie'] = [ + 'dependencies' => [ + 'core/backbone', + ], + ]; + if (Drupal::moduleHandler()->moduleExists('minifyzombies')) { + $libraries['mymodule.zombie'] += [ + 'js' => [ + 'mymodule.zombie.min.js' => [], + ], + 'css' => [ + 'mymodule.zombie.min.css' => [], + ], + ]; + } + else { + $libraries['mymodule.zombie'] += [ + 'js' => [ + 'mymodule.zombie.js' => [], + ], + 'css' => [ + 'mymodule.zombie.css' => [], + ], + ]; + } + + // Add a library only if a certain condition is met. If code wants to + // integrate with this library it is safe to (try to) load it unconditionally + // without reproducing this check. If the library definition does not exist + // the library (of course) not be loaded but no notices or errors will be + // triggered. + if (Drupal::moduleHandler()->moduleExists('vampirize')) { + $libraries['mymodule.vampire'] = [ + 'js' => [ + 'js/vampire.js' => [], + ], + 'css' => [ + 'css/vampire.css', + ], + 'dependencies' => [ + 'core/jquery', + ], + ]; + } + return $libraries; +} + /** * Perform necessary alterations to the JavaScript settings (drupalSettings). * diff --git a/core/tests/Drupal/Tests/Core/Asset/LibraryDiscoveryParserTest.php b/core/tests/Drupal/Tests/Core/Asset/LibraryDiscoveryParserTest.php index 18e8bbc93bfc..528e5795451f 100644 --- a/core/tests/Drupal/Tests/Core/Asset/LibraryDiscoveryParserTest.php +++ b/core/tests/Drupal/Tests/Core/Asset/LibraryDiscoveryParserTest.php @@ -161,7 +161,7 @@ class LibraryDiscoveryParserTest extends UnitTestCase { * Tests that an exception is thrown when no CSS/JS/setting is specified. * * @expectedException \Drupal\Core\Asset\Exception\IncompleteLibraryDefinitionException - * @expectedExceptionMessage Incomplete library definition for 'example' in core/tests/Drupal/Tests/Core/Asset/library_test_files/example_module_missing_information.libraries.yml + * @expectedExceptionMessage Incomplete library definition for definition 'example' in extension 'example_module_missing_information' * * @covers ::buildByExtension */ @@ -400,7 +400,7 @@ class LibraryDiscoveryParserTest extends UnitTestCase { * Tests that an exception is thrown when license is missing when 3rd party. * * @expectedException \Drupal\Core\Asset\Exception\LibraryDefinitionMissingLicenseException - * @expectedExceptionMessage Missing license information in library definition for 'no-license-info-but-remote' in core/tests/Drupal/Tests/Core/Asset/library_test_files/licenses_missing_information.libraries.yml: it has a remote, but no license. + * @expectedExceptionMessage Missing license information in library definition for definition 'no-license-info-but-remote' extension 'licenses_missing_information': it has a remote, but no license. * * @covers ::buildByExtension */ @@ -434,7 +434,6 @@ class LibraryDiscoveryParserTest extends UnitTestCase { $libraries = $this->libraryDiscoveryParser->buildByExtension('licenses'); - // For libraries without license info, the default license is applied. $library = $libraries['no-license-info']; $this->assertCount(1, $library['css']);