WebDevStudios / oops-wp

A collection of abstract classes, interfaces, and traits to promote object-oriented programming practices in WordPress.
57 stars 9 forks source link

Wrap example require in file_exists() check in README.md #28

Closed salcode closed 4 years ago

salcode commented 4 years ago

Currently the example in README.md is

require_once __DIR__ . '/vendor/autoload.php';

If our plugin is loaded as a dependency, e.g. the entire WordPress website is under version control and our plugin is loaded by a composer install in the root of the project, then all of the plugin dependencies will be located in one vendor directory in which case there will be no file at __DIR__ . '/vendor/autoload.php'.

With no file at __DIR__ . '/vendor/autoload.php' our require_once statement will throw a fatal error in this scenario.

If we wrap it in a file_exists() check, then we'll skip trying to load our auto-loader local to this plugin (with the idea that if everything is being installed from the root of the project, the auto-loader for the shared vendor directory is being loaded)

jmichaelward commented 4 years ago

file_exists checks are a necessary part of the PHP developer's toolset. Closing this out, as I deem this an educational issue outside of the scope of this particular library.

salcode commented 4 years ago

My apologies @jmichaelward, I did a poor job of describing this issue. Below I've tried to outline the steps that cause an issue a little more clearly here.

Steps to Recreate

Prerequisites

Start with a WordPress install.

We'll be using the files in this gist in the following steps.

Add My Demo Plugin Via Composer

Add the composer.json file to the root of the project.

e.g.

curl -O https://gist.githubusercontent.com/salcode/219d0c62636b782fb2f5138fe2d16ad8/raw/composer.json

and run

composer install

Add mu-plugin to load shared autoloader

Because we ran composer install from the root of the project, all dependencies will use a shared vendor directory. In this case, the directory is at wp-content/vendor (because of the configuration in composer.json).

When using a shared vendor directory we need to include the autoloader from it. Here I am doing this by adding a plugin at wp-content/mu-plugins/mu-plugins-autoload.php.

e.g.

mkdir wp-content/mu-plugins && curl -o wp-content/mu-plugins/mu-plugins-autoload.php https://gist.githubusercontent.com/salcode/219d0c62636b782fb2f5138fe2d16ad8/raw/mu-plugins-autoload.php

Activate the Plugin

At this point if we activate the Salcode OOPS-WP Demo plugin, we get a fatal error.

Fatal error: require_once(): Failed opening required '/app/public/wp-content/plugins/oops-wp-demo/vendor/autoload.php' (include_path='.:') in /app/public/wp-content/plugins/oops-wp-demo/salcode-oops-wp-demo.php on line 24

Screen Shot 2019-12-27 at 13 31 21

Why we get the fatal error

This fatal error occurs because the line

require_once __DIR__ . '/vendor/autoload.php';

source

is looking for the autoload file in the vendor directory of the plugin (rather than the shared vendor directory).

Suggestion

Because this edge case is easy to miss (and copy-and-pasting from the README.md is a common way to get started), I think we wrap the require_once for the autoloader with an is_readable() check.

jmichaelward commented 4 years ago

@salcode Thanks for clarifying this issue. I insist on closing this once more, because the example you provided doesn't in fact follow the sample instructions:

  1. Run composer require webdevstudios/oops-wp from a directory.
  2. Within that directory, add require_once __DIR__ . '/vendor/autoload.php';

If step 1 is done within a plugin, a theme, the mu-plugins directory, or from within the WordPress root using a standard Composer configuration, step two can be done safely from within that same directory by adding the line to the plugin file, theme file, an mu-plugin, or wp-config.php.

In your case, I would advise placing the require statement in wp-config.php and modifying the path to point to the vendor directory, since that's where you configured Composer to install it, and then bypass adding the require statement in the plugin altogether because it's not needed. That's a lot of information to include to the README, info which is better served by both the Composer and PHP official documentation.