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

PSR-4 loader conflict with older implementation #3852

Closed
Rarst opened this issue Mar 18, 2015 · 11 comments
Closed

PSR-4 loader conflict with older implementation #3852

Rarst opened this issue Mar 18, 2015 · 11 comments

Comments

@Rarst
Copy link
Contributor

Rarst commented Mar 18, 2015

Hello from WordPress land with crazy edge case.

The user of one of plugins had reported following error:

Fatal error: Call to undefined method Composer\Autoload\ClassLoader::setPsr4() in C:\xampp\htdocs\mywebsite.com\wp-content\plugins\mailchimp-top-bar\vendor\composer\autoload_real.php on line 33

The working theory is that another plugin present in the system is also using Composer autoload, but at old pre-PSR-4 version.

The logical attempt was to try optimized autoload, but turns out that despite all the classes getting compiled into classmap file, the PSR-4 instructions are still retained and loaded (thus still causing crash).

cc @dannyvankooten @coenjacobs

@Seldaek
Copy link
Member

Seldaek commented Nov 19, 2015

Don't know if this is still an issue, but I guess/hope not, so closing. There isn't much we can do anyway IMO.

@Seldaek Seldaek closed this as completed Nov 19, 2015
@dangoodman
Copy link

This is still the issue. I am a WordPress plugin author too and I receive reports from my users about the error. Conditions are same: WordPress and two plugins using different Composer's ClassLoader class versions.

The solution is to generate unique ClassLoader class name in the same way it's done with ComposerAutoloaderInitXXX class (e.g. ClassLoader6072567f9203e3a1f3e5cb62ba47c22b). Do you see any issues with it?

@Seldaek
Copy link
Member

Seldaek commented Jan 19, 2016

I don't think that's needed as we only include that file if it isn't declared yet.

@dangoodman
Copy link

Right. That's why the issue happens:

  1. Plugin A loaded first loads its outdated ClassLoader without PSR-4 support.
  2. Plugin B loaded next does not load its newer ClassLoader version, due to the behavior you mentioned.
  3. Plugin B fails with the fatal error in its ComposerAutoloaderInitXXX class on the line $loader->setPsr4($namespace, $path);

@Seldaek
Copy link
Member

Seldaek commented Jan 19, 2016

I see, but that's not something we want to fix I think.. if people randomly embed code from composer it's not really going in the direction we want. Both plugins should be installed via composer and then this would not happen..

@dangoodman
Copy link

I'm not sure I get you about random composer's code embedding. Both plugins in the example above use composer as it is supposed to: require('vendor/autoload.php').

Anyway, I agree that ideally both plugins should be installed with Composer. However, in real life it's not feasible.

So, in other words composer is not suitable for embedded projects, like Wordpress plugins. It's sad since it does not seem such a big deal to make Composer working for them too.

@Seldaek
Copy link
Member

Seldaek commented Jan 19, 2016

Pretty much every project except wordpress at this point works in the same way: you have a composer.json and declare the list of plugins you want, and they get installed, and that's that. One autoloader, then some plugins/libraries/whatever packages.

If every wordpress plugin embeds a composer autoloader, it's because the ecosystem is broken and wordpress core doesn't give a damn about moving forward. It's not something I am willing to waste time on I'm sorry..

As a solution though, I would suggest you try out and spread the word about https://roots.io/bedrock/ - as it provides exactly what I described above for wordpress.

@dangoodman
Copy link

Almost everything in PHP world is broken, including PHP itself and Wordpress.

Thanks for Bedrock. No doubt it's a nice thing. However, as a plugin author I have nothing to do with that. Asking my plugin users to switch to Bedrock is like asking Composer's users switch to Mac OS to able to use it.

There is another show stopper though: no support for multiple versions of a library in a single project. Guys in discussion #2609 suggest to rebuild the world to fix it ;)

@dangoodman
Copy link

dangoodman commented Jan 20, 2016

I have made a plugin for Composer fixing the initial error. Although, it does not fix all the WordPress issues, it's one step closer to that. Maybe it would be useful for someone like me coming here from google.

@mikeschinkel
Copy link

@Rarst JMTCW, but using Composer inside plugins is a bad idea because PHP and WordPress have no way to manage multiple versions of the same code in one site installation. But then you know that. :-)

@route1rodent
Copy link

So apparently we have no other choice than:

A) Create and use a custom fork of composer that can do exactly this
B) Refactor own ClassLoader class name manually and give it another name (eg. with the same random string as the other generated classes). Then bundle that into the final plugin.
C) Build our own autoloader using the generated autoloader data and only include that one, or use phpab which is basically doing that using a closure.
D) Ask the non-developer people (like journalists, etc) to migrate their entire Wordpress ecosystem to something like Bedrock if they want to use a WP plugin.

Software Development comes from the need of solving real world problems. With any of these options seems that we are creating more problems than solving them, only for not willing to give the final developers the choice of deciding how we want to use this tool.

Bad practices will be always there no matter how restrictive we are, all depends on how people uses the tools they have.

I am totally agree with @dangoodman and his solution #3852 (comment) with a default to the current behavior of naming it ClassLoader to not break compatibility.

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

No branches or pull requests

5 participants