Downloading the plugin as a composer package does not contain the vendor directory and autoloads

[cherrygot-personal]

Downloading the plugin as a composer package does not contain the vendor directory and autoloads

[justlevine]

The issue isn't about storing the vendor folder in .git, it's that when composer installs a library (whether it's from git or packagist or a release zip), the library's dependencies get installed in the top-level vendor folder.

Because wp-graphql-content-blocks checks for explicit files in the mysite/wp-content/plugins/wp-graphql-content-blocks/vendor folder, users who are using composer to manage their WordPress build (as they should in Enterprise) are mistakenly told they don't have the deps, even though they just exist in a top-level path (e.g. my-site/wp-content/vendor ).

Since this plugin uses Composer internally, we can skip the file-check, and instead check if the autoloaded class exists. I wouldn't copy it verbatim, but if you're looking for inspiration, see how WPGraphQL core handles it

[justlevine]

We do something similar here:

https://github.com/wpengine/wp-graphql-content-blocks/blob/main/includes/WPGraphQLContentBlocks.php#L95-L150

is something we are missing?

That first else statement is the problem. It displays the admin notice if the autoloader file isnt located in a specific directory path.

The ideal behavior would be to instead check class_exists( 'WPGraphQL\Registry\TypeRegistry' ) or a similarly primary autoloaded class to confirm the autoloader is working, regardless of what path it's being loaded from.

[justlevine]

@cherrygot-personal

So two solutions come to mind,

1. We put the plugin on a registry. Well, that's something I have no idea how to do. (But love to give it a try)

composer install my-package-from-registry will still install any composer.json deps in the package in to a top-level vendor, so this won't solve the problem.

I checked and there is a mapping to the includes/ folder of the repo like so,
[...]
So I'm a bit surprised why this would be happening. The reason why I'm not thinking that it may have to do something with it being an mu-plugin is because I'm using wp-graphql as mu-plugin as well, it resolves just fine.

Just because one plugin works well as an mu-plugin doesnt mean another one will. Ultimately it comes down to a race condition about when the mu-plugin loads vs when Composer's autoloader gets injected. Bonus, it only takes a few minutes to try and rule out 😇.

As a general rule you should mu-plugins for custom plugins and install actual plugins as regular plugins (you can use an mu-plugin to force-activate a plugin, but I can't imagine a situation where you wouldn't want the ability to disable a plugin (e.g. for debugging).

[cherrygot-personal]

Okay, found the culprit. The file https://github.com/wpengine/wp-graphql-content-blocks/blob/main/includes/Utilities/WPGraphqlHelpers.php doesn't follow PSR4 standards.

The name of the file should be WPGraphQLHelpers.php instead of WPGraphqlHelpers.php, same as the class is named.

[theodesp]

Hey @cherrygot-personal currently we do not provision the plugin in packagist or the wordpress plugins registry and you should be using the releases tab since the project source code does not contain a vendor folder.

Another option is to just install the composer dependencies by running the composer install --no-dev within the plugin folder.

[justlevine]

Hey @cherrygot-personal currently we do not provision the plugin in packagist or the wordpress plugins registry and you should be using the releases tab since the project source code does not contain a vendor folder.

Another option is to just install the composer dependencies by running the composer install --no-dev within the plugin folder.

@theodesp is the above an acknowledgement of the bug in how this plugin checks for the existence of its autoloader (looking for files in a fixed plugin-relative folder), instead of if the autoloaded classes exist) or a clarification about intentional design and intended behavior?

Enterprise- headless sites should really be using Composer to manage their backend dependencies, so I'd assume we'd want to reduce that friction as much as possible?

[justlevine]

@cherrygot-personal does that error go away if you install it as a normal plugin instead of an mu-plugin? I'm requiring it as a vcs repository on a Bedrock build, and apart from the annoying dashboard message that the deps are missing when they aren't, the plugin works fine.

Since mu-plugins run earlier in the wp lifecycle, I'm guessing that (or something with your composer) is the cause of the missing class exception.

[theodesp]

Composer

Hey @justlevine we do have Composer for managing backend dependencies but we don't ship them as part of the source code.

I'm not familiar with the proper conventions here but my assumption is that vendor folder is similar to node_modules folder and should not be part of the git repo.

[cherrygot-personal]

@theodesp

I agree, I checked my composer file again and it is indeed not loaded from some package registry like packagist or wpackagist. I'm using it just like justlevine, via repositories property with type github.

{  ...
    "repositories": [
          ...
          {
                "type": "github",
                "url": "<repo-url>"
          }
          ...
    ]
    ....
}

So this does make sense that you wouldn't go for git track vendor/, just like you wouldn't do so for node_modules/.

So two solutions come to mind,

1. We put the plugin on a registry. Well, that's something I have no idea how to do. (But love to give it a try)
2. We use some sort of lifecycle hook and run the autoload dump command.

I'm using the second approach at the moment, but in my project repo, like so

"scripts": {
    "post-install-cmd": [
         "cd wordpress/web/app/mu-plugins/wp-graphql-content-blocks/; composer dump-autoload -o",
         ...
    ]
}

My idea, not sure how feasible though, is to hook the dump autoload command to the life-cycle of the repo instead of it being the burden of the consumer of the repo. Again, that's just a fancy idea, I'm not sure if that's how composer can work.

@justlevine I have not tried using it as a normal plugin via composer. Like I've used it as normal plugin by downloading zip from the release page, but not as composer dependency. But there's a quick fix with post-install-cmd so I never bothered to try that. I mentioned it because, as you said, big budget applications using composer will face this problem too.

And btw, thanks for the info, I didn't know that autoloads are generated at root level "vendor/" directory. I checked and there is a mapping to the includes/ folder of the repo like so,

return array(
    'WPGraphQL\\ContentBlocks\\' => array($baseDir . '/wordpress/web/app/mu-plugins/wp-graphql-content-blocks/includes'),
    ...
);

So I'm a bit surprised why this would be happening. The reason why I'm not thinking that it may have to do something with it being an mu-plugin is because I'm using wp-graphql as mu-plugin as well, it resolves just fine.

[theodesp]

The issue isn't about storing the vendor folder in .git, it's that when composer installs a library (whether it's from git or packagist or a release zip), the library's dependencies get installed in the top-level vendor folder.

Because wp-graphql-content-blocks checks for explicit files in the mysite/wp-content/plugins/wp-graphql-content-blocks/vendor folder, users who are using composer to manage their WordPress build (as they should in Enterprise) are mistakenly told they don't have the deps, even though they just exist in a top-level path (e.g. my-site/wp-content/vendor ).

Since this plugin uses Composer internally, we can skip the file-check, and instead check if the autoloaded class exists. I wouldn't copy it verbatim, but if you're looking for inspiration, see how WPGraphQL core handles it

We do something similar here:

https://github.com/wpengine/wp-graphql-content-blocks/blob/main/includes/WPGraphQLContentBlocks.php#L95-L150

is something we are missing?

[cherrygot-personal]

@justlevine I tried installing as a plugin and the same issue pops up.

[12-Dec-2023 16:06:21 UTC] PHP Fatal error:  Uncaught Error: Class "WPGraphQL\ContentBlocks\Utilities\WPGraphQLHelpers" not found in /var/www/wordpress/web/app/plugins/wp-graphql-content-blocks/includes/Registry/Registry.php:224

And the admin notice too:
WPGraphQL Content Blocks appears to have been installed without its dependencies. If you meant to download the source code, you can run `composer install` to install dependencies. If you are looking for the production version of the plugin, you can download it from the [GitHub Releases tab.](https://github.com/wpengine/wp-graphql-content-blocks/releases)

[justlevine]

@justlevine I tried installing as a plugin and the same issue pops up. [...] And the admin notice too

Admin notice is the aforementioned bug, nothing you can do on your end to fix.

If the still doesnt exist by the time you're loading your plugins, then it points to a problem with your Composer config. To debug, I'd suggest the following steps:

1. Run composer dump-autoload --no-dev --optimize (in case any of your previous troubleshooting threw it for some loops).
2. Delete your dependencies (including your composer-installed plugins), and run composer install --no-dev --optimize-autoloader (sometimes Composer gets tripped up by caching when the deps arent in the normal vendor dir).
3. Change the repository type from github to vcs. (will rule a repo bug between .gitattributes vs composerJson.export

If you're still running into issues, the bug isn't with Composer (which is how the Registry class gets loaded so it's currently partially working), and will require some investigation.

[cherrygot-personal]

Hey @justlevine , sorry for responding late on this issue.

So yeah, I went through the checklist and first deleted all the dependencies in vendor/ and installed plugins, and then generated autoloads. Still, no luck :/

Tried changing the github to vcs, but the issue remains the same.

However, this issue is turning out to be a very strange one for me. And looks like an edge case that makes me doubt if it's very specific to my machine or something. Here's what I found:

When I replace the WPGraphQLHelpers name with something else, let's say TestHelpers and the file to TestHelpers.php, the thing works absolutely fine. No exceptions, warnings or errors. All that with just one name change!

Just to make sure that it's not conflicting with any class names that I've defined locally, I ran a search in my work folder and I found nothing suspicious.

Even if I, let's say, generate the autoloads for the plugin in its directory and remove this line from $classMap array,

        'WPGraphQL\\ContentBlocks\\Utilities\\WPGraphQLHelpers' => __DIR__ . '/../..' . '/includes/Utilities/WPGraphqlHelpers.php',

it starts going haywire. Is this even reproduce-able?

[linucks]

Just so say that I've encountered this problem too. I'm working on a wordpress install using Bedrock and have the following in my composer.json:

    {
      "type": "package",
      "package": {
        "name": "wpengine/wp-graphql-content-blocks",
        "version": "2.0.0",
        "type": "wordpress-plugin",
        "dist": {
          "url": "https://github.com/wpengine/wp-graphql-content-blocks/archive/refs/tags/v2.0.0.zip",
          "type": "zip"
        }
      }
    }

....

 "require": {
 ...
    "wpengine/wp-graphql-content-blocks": "^2.0.0",

I can fix it by going into /var/www/html/web/app/plugins/wp-graphql-content-blocks and running composer install, but it would be nice not to have do that.

[cherrygot-personal]

Hey everyone,

I see that there is a v3.0.0 release of the plugin. I was wondering if this issue can be taken into account for the next patch release? As for the summary, justlevine's comment about not needing vendor/ in the repo makes sense, since composer will automatically resolve it. So we don't need the vendor directory as such.

The solution for this issue mentioned in my screenshot is just to rename the file WPGraphqlHelpers.php to WPGraphQLHelpers.php. It'd be great that it's considered.

Thanks :)

[mindctrl]

@cherrygot-personal we just merged #201 with this fix. It'll be in the next release. Thanks for reporting this and for your patience.