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

Fixed auto-loader to skip non-WP-Auth0 classes #465

Merged
merged 1 commit into from
May 24, 2018
Merged

Conversation

joshcanhelp
Copy link
Contributor

Fixes #461

@joshcanhelp joshcanhelp added this to the 3.6.0 milestone May 22, 2018

// Anything that's not part of the above and not name-spaced can be skipped.
if ( 0 !== strpos( $class, 'WP_Auth0' ) ) {
return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should you log something on this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely not. This is called for all class instantiations. It's not an error if an non-WP-Auth0 class is being called. We want to escape quickly.

$source_dir . 'admin/',
$source_dir . 'exceptions/',
$source_dir . 'wizard/',
$source_dir . 'initial-setup/',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this last comma will be ignored.

Copy link
Contributor Author

@joshcanhelp joshcanhelp May 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. It's a PHP standard/style to add a comma on the last line of an array

$source_dir . 'initial-setup/',
);

foreach ( $paths as $path ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how often this method would be called, but I suppose several times per execution. Is there any way of iterating through all the files in a more efficient way? Maybe accepting an array of "classes to require" so you can iterate once and require them all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

several times per execution

Correct

Maybe accepting an array of "classes to require"

That would defeat the purpose of the autoloader. This is here to "magically" include the necessary files for whatever class you might call. There is a more efficient way to do this using Namespaces but that would require a lot more work (I have it on my list)

foreach ( $paths as $path ) {
if ( file_exists( $path . $class . '.php' ) ) {
require_once $path . $class . '.php';
return true;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to return false?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope!

@joshcanhelp joshcanhelp merged commit 66af992 into dev May 24, 2018
@joshcanhelp joshcanhelp deleted the fix-autoloader branch May 24, 2018 19:48
@joshcanhelp joshcanhelp mentioned this pull request Jun 5, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants