Problem/Motivation

We recently updated our Components module to the latest version and faced an issue with the GraphQL Twig module. In the Components module the way how you define the location of components has changed from the key component-libraries to the key components. The change was done in the following issue: https://www.drupal.org/project/components/issues/3091762

However GraphQL Twig module also looks for the key components from the active theme's info file, leading to a conflict. For example if you have defined custom namespaces for your components you would see the following warning as we are not providing a string/path to the check performed in GraphQL Twig.

Warning: is_dir() expects parameter 1 to be a valid path, array given in Drupal\graphql_twig\Template\Loader\Loader->findTemplate() (line 129 of modules/contrib/graphql_twig/src/Template/Loader/Loader.php).

Steps to reproduce

1. Update to/use the latest version of the Components module
2. Define component namespaces according to the new way in your theme's info file: https://www.drupal.org/project/components/issues/3091762
3. You will see the warning reported above when browsing the frontend

Proposed resolution

For now let's add a check for if we are actually including components with a hash in the component name (which you can do when using Fractal components) in the Drupal templates before scanning all the templates in the components folder. We can also add a check for if the parameter used in is_dir() is a string to avoid the parameter warning, but in reality if you are using Fractal components and the latest version of the Components module you will have issues anyway since the "components" definition is in conflict with each other.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

gaards created an issue. See original summary.

gaards’s picture

Status: Active » Needs review
martijn de wit’s picture

Encountered the same problem. The patch from the merge request is working great.

martijn de wit’s picture

Status: Needs review » Reviewed & tested by the community

saschaeggi made their first commit to this issue’s fork.

saschaeggi’s picture

Status: Reviewed & tested by the community » Fixed

Thanks!

Status: Fixed » Closed (fixed)

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