- Issue created by @wim leers
- First commit to issue fork.
- ๐ฎ๐ฉIndonesia el7cosmos ๐ฎ๐ฉ GMT+7
I tried adding duplicate components in test, but canโt reproduce the crash.
Q: do we have any plugin manager/discovery that throws exception when there are duplicate plugins? - ๐จ๐ทCosta Rica alemadlei
For mentored contribution at DrupalCon Atlanta, the SDC group is taking a look at this issue.
So far one of the team members has verified that this is not crashing on the latest Drupal 11 stable version. We did find a couple of things that are odd or unexpected, but once all team members validate, we will create separate issue tickets.
Since this was reported in August of 2024 we are going to test with Drupal 10.3 and verify it crashed there and was addressed by a later release.
- ๐บ๐ธUnited States mcolebank
We created duplicate components with the same name ('image') in a custom module and in a custom theme on Drupal 11.
In a Test Module we created and tested the components Image/image.component.yml and Example/image/image.component.yml which had the same yml files. When the component was included in the theme, Image/image.component.yml rendered and the Example/image component was ignored.
In a Test Theme we created and tested the components Image/image.component.yml and Example/image/image.component.yml which had the same yml files. When the component was included in the theme, Example/image/image.component.yml rendered and the Image/image component was ignored.
The duplicate components with the same name did not crash Drupal. We also tested in Drupal 10.3 with the same results. It is unexpected behavior that a different version of the duplicated component is rended if the duplicate components are in a theme or in a module.
Recommend that we close this issue and create a new issue where SDC shows a warning message when two or more components have the same name.
- ๐บ๐ธUnited States mcolebank
We started to implement a warning and logger if a duplicate component was found. Following Drupal patterns we wanted to use dependency injection using ContainerInjectionInterface instead of using Drupal services but DirectoryWithMetadataDiscovery.php is calling YamlDirectoryDiscovery with only 2 arguments. This threw an error because we were increasing the arguments.
In the `web/core/lib/Drupal/Component/Discovery/YamlDirectoryDiscovery.php` we were trying to add this code:
if ($files) { // Keep track of duplicates. $duplicates = []; foreach ($files as $file => $provider) { // If a file is empty or its contents are commented out, return an empty // array instead of NULL for type consistency. try { $data = Yaml::decode(file_get_contents($file)) ?: []; } catch (InvalidDataTypeException $e) { throw new DiscoveryException("The $file contains invalid YAML", 0, $e); } $data[static::FILE_KEY] = $file; $componentid = $this->getIdentifier($file, $data); // Check for duplicates, warn user and log if found. if (isset($duplicates[$componentid])) { \Drupal::messenger()->addWarning('Duplicate component found: ' . $componentid); \Drupal::logger('YamlDirectoryDiscovery')->warning('Duplicate component found: ' . $componentid); } $duplicates[$componentid] = $componentid; $all[$provider][$componentid] = $data; $file_cache->set($file, $data); } } return $all; }
- Merge request !116593468881: adding code to check for duplicate component names and warning user and logging โ (Open) created by Unnamed author
- ๐จ๐ทCosta Rica estebanvalerio.h
Worked with @mcolebank debugging and updating code including dependency injection to avoid calls like `\Drupal::messenger()`, thanks @ultimike , @markie and @rfay for their guidance
Also thanks to https://gregrusson.com/ and @tylerhastain for debugging this issue. - ๐บ๐ธUnited States mradcliffe USA
I am marking this as Needs work because we still need to add explicit test coverage based on the issue summary and we probably want to make the messenger string a translatable markup string with a placeholder.
- ๐บ๐ธUnited States mcolebank
We have added translation. Needs review and testing.