-
Notifications
You must be signed in to change notification settings - Fork 635
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 methods or guidelines for plugin developers to avoid conflicts/loading order issues #3028
Comments
As @khalwat says, teaching new plugin developers some best practices for plugin setup might reduce errors like the above. That could be done through standardized functions that isolate each part of this process, additional documentation, or more detailed examples. I also agree that something like a priority system would only be kicking the problem further down the line. These kinds of side effects should be avoided early on. I'll admit that I did not initially realize this was a problem, and will resolve this for Fallback Site. To add to that, in an ideal world, I believe all of a plugin's event handlers would be registered within the Finally, if there was a way to raise some kind of notice for event handlers being attached after their events were already fired, it'd make tracking down any issues as a result of this behavior much easier. |
As of the next release, If similar issues crop up in other methos, we can do the same for them. |
I'm still having an issue using
But I'm still getting a 404 when I hit a front-end entry page. If I comment out the code hitting Let me know if there are any other details I can provide or if you'd rather have me open a new issue. |
@davist11 Where is the NotFoundHttpException getting thrown? |
Here's the full stack trace
|
Can you give us access to the site? (Git repo or just your composer.lock + composer.json files, plus any custom module/plugin code, plus a database backup.) If so pleaese send to [email protected] |
Email sent, thanks! |
Related: #10441 |
There are a number of parts of Craft that fire events to allow plugins and other things to register things like AdminCP routes or site routes.
The problem is that if any plugin calls a service method that causes it to initialize itself, that means that no other plugins will be able to register additional things. For example, in the FallbackSite plugin does this:
https://github.com/charliedevelopment/craft3-fallback-site/blob/master/src/FallbackSite.php#L39
Since it's calling
Craft::$app->getUrlManager()
that will cause the UrlManager to init itself, which means any plugins that load after it will be unable to register site or AdminCP routes.I don't view this as a problem with the FallbackSite plugin, but rather with how Craft is handling things.
Similarly, if any plugin uses Twig to render something in its
init()
method, any plugins that load after it will be unable to register a new Twig extension, because Twig has already been initialized.There are numerous other services/parts of Craft that work like this; you can register new things until someone calls a function that uses that service/part, at which time it initialize itself, and no further things can be registered.
Proposal
I think this behavior should be documented, otherwise we're going to be seeing no end of "plugin conflicts" or have something like WordPress where plugins have a loading "priority" (which never works out) or the bad old days of OS X extension conflicts/loading order.
But I think just documenting it isn't enough. I think what makes sense is the base plugin class should implement a new method called, perhaps,
install()
which Craft will call after it has loaded all plugins and done any other setup work.It then should be documented that in your
init()
method you shouldn't be calling any Craft services that could cause them to initialize themselves.You should register anything you want there, such as site/AdminCP routes, Twig extensions, etc., but not call any Craft services. Maybe there should even be an additional
register()
method to explicitly be where you do this?Rather all of that code that you need to actually call Craft's various services should be in the
install()
method (or whatever it ends up being called).Thoughts?
The text was updated successfully, but these errors were encountered: