Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add function_exists('add_action') check #1271

Merged
merged 1 commit into from May 23, 2019

Conversation

salcode
Copy link
Collaborator

@salcode salcode commented May 23, 2019

Add function_exists('add_action') check to initialization file. When
loading CMB2 via Composer (and autoloading init.php), the init.php will
be loaded every time. Usually this is not a problem, however if we are
also using Composer to load a PHP development tool (e.g. PHP Codesniffer
(phpcs)), when the development tool runs it is NOT in the context of
WordPress.

Running CMB2's init.php outside of the context of WordPress
results in a fatal error because the function add_action() does not
exist. By adding this function_exists() check, we can avoid this fatal
error when autoloading init.php outside of the context of WordPress.

See #1270

Description

Check that the add_action() function exists before calling it in init.php.

Motivation and Context

Fixes #1270.

Risk Level

This seems to be a relatively low risk change. Since we're checking if the function add_action() exists directly before calling it, the only behavior change we should get is avoiding throwing this fatal error.

Testing procedure

develop tests are failing

The tests on the develop branch (before adding any work of my own) are currently failing.

FAILURES!
Tests: 210, Assertions: 690, Failures: 6.

Adding my changes to master

I cherrypicked the one commit in this PR onto the master branch and ran phpunit there.

All tests passed when my change was introduced into the master branch.

...............................................................  63 / 210 ( 30%)
............................................................... 126 / 210 ( 60%)
............................................................... 189 / 210 ( 90%)
.....................                                           210 / 210 (100%)

Time: 9.44 seconds, Memory: 44.00MB

OK (210 tests, 690 assertions)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

Add function_exists('add_action') check to initialization file. When
loading CMB2 via Composer (and autoloading init.php), the init.php will
be loaded every time. Usually this is not a problem, however if we are
also using Composer to load a PHP development tool (e.g. PHP Codesniffer
(phpcs)), when the development tool runs it is NOT in the context of
WordPress.

Running CMB2's init.php outside of the context of WordPress
results in a fatal error because the function add_action() does not
exist. By adding this function_exists() check, we can avoid this fatal
error when autoloading init.php outside of the context of WordPress.

See CMB2#1270
@jtsternberg jtsternberg merged commit eaaa32f into CMB2:develop May 23, 2019
@jtsternberg
Copy link
Member

Seems legit. Thank you @salcode

@johnbillion
Copy link
Contributor

I don't believe this change makes much sense. Using Composer to autoload a WordPress plugin with side effects is incorrect and shouldn't be fixed by coding around the side effects in every plugin.

See my comment on the same issue reported against the action-scheduler for my reasoning: woocommerce/action-scheduler#303 (comment)

@salcode
Copy link
Collaborator Author

salcode commented Jun 10, 2019

While I recognize this solution is not ideal, I think it is the most practical approach. In the Action Scheduler thread I've added my thoughts in more detail:
woocommerce/action-scheduler#303 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add function_exists( 'add_action' ) check to initialization file
3 participants