Decide if we need to add plugin-system to core. #41
Comments
|
Can we get a more descriptive/compelling use case? |
|
I updated the issue summary to be more descriptive. There are some discrepancies between how Views and CTools discover "plugins": Views uses an info hook, while CTools uses a .inc file inside the plugin itself with a function that matches a naming convention. Views' approach works well as long as you don't want to reuse the plugin somewhere else (which isn't common for Views), but something like a Panels layout you might want to use on multiple sites. However, since you could just move the containing module instead of the individual layout, I'm not sure how steep that requirement is. |
|
As I said in another issue, why not just implement PSR4, and convert the existing plugin system to use that (like core is doing anyway). If you feel that strongly against annotations, kill them too and use yaml discovery. Reinventing the plugin system (that the ctools people built for d8 anyway) while trying to maintain any resemblance of d8 compatibility is going to be impossible. |
|
I'd agree with @bojanz. Plugins are one of the things I actually like about D8, just not the more complex directory structure since they're not intended to be used outside of Drupal. |
|
I agree. Sounds like a plan to me. |
|
The things I find bad with the plugin system are:
|
|
Please keep in mind that in D8's plugin system, the discovery process is pluggable. That means annotations are optional. It is assumed (and this is a big assumption because I find myself with a massive lack of time) that CTools in D8 would ship with alternate discovery mechanisms that emulate its ways of handling plugins to reduce the impact of updates. That addresses joachim point #2: annotations are not required, as long as there's an alternate discovery mechanism. I can't address parts #3 or #4 but they look addressible. D8s plugin architecture is very loosely coupled to anything; it should be possible to pick it up and use it. Also, I would recommend against info hooks for plugins. Views is the reason I switched away from info hooks for plugins -- they are very hard to keep separate. I would be in favor of .info, .yaml or .json files over info hooks in any architecture. (also, probably over .inc as well, because of the unique problems there). |
|
ok, so thank you for all the Plugin System love, you have no idea how much I appreciate it. To address Joachim's specific issues, most of this is already solved in D8. Folder Structure: PSR4 is at least underway and greatly simplifies this and https://drupal.org/node/2084513 will likely go in today or tomorrow and make PSR4 a possibility from the Plugin side. Annotations: chx is trying to get terminal commas into Doctrine right now: doctrine/annotations#11 No comments? Custom annotation classes support all the documentation in the world. You should be using them, which should eliminate your "single point of documentation" comment as well. Quicksketch was heavily involved in pushing us this way and I'm very happy with it. Defining a new plugin in D8 requires you to define a plugin manager (most extend DefaultPluginManager and just need to define a __construct)) You SHOULD make a custom annotation class as outlined above for all the reasons therein, and then the easiest way to manage the class on going is by putting it into the DIC. So yes, that's 2 php files and one yml entry in a file that already exists (obviously discussing D8 here). CTools by comparison depended upon your approach. OO plugins were a little different, but you had to define the same sort of directory structures we do in D8 (though it was generally just plugins/$type in the module) and then do ctools_include('plugins') and then know that include and how to ask it to get all your plugins of a type. Plugins themselves depended heavily on magic naming of methods or array key value alignment (which is essentially an OO interface). If you used procedural plugins that definition could exist with the functions that would satisfy the plugin system's needs, if you used OO, then we separated the metadata from the class (ick), which is why the vast majority of plugins we wrote were procedural. We only adopted OO plugins when the inheritance was absolutely essential. Merlin has outlined some great points above, I'd point out that json suffers from ALL the things that Joachim outlined about annotations, and unlike annotations, there's not a solution for json... there is a solution for annotations and D8 has largely implemented it. Also as merlin points out, discovery is 100% pluggable. Obviously I'm pretty sold on annotations, but there's a YamlDiscovery in Drupal as well if you just prefer that. I'd point out that incredible amount of overhead that went into making yaml translatable for CMI and point out that Annotations do NOT have this problem. Our solution was really simple and worked really well. You only get that if your metadata is integrated to php fairly deeply, otherwise you have to build some construct of your own like CMI's context system. I AM trying to separate Plugins out to be composer-able for this cycle. https://drupal.org/node/2065481 is really the first issue that needs to go in to make that happen. I hope I at least illuminated some of our basic reasoning here from the Plugin side. Eclipse |
|
We don't need PSR4 because plugins are not designed to work on anything outside of Drupal, and most likely nothing non-drupal will be able to work with plugins either. PSR4 is for interopability between different projects, so doesn't apply to modules, plugins, or anything that's a Drupal-internal. Let's just put them neatly inside a plugins directory. And yes, death to annotations! |
|
Jen, Plugins were designed, from day 1, to work outside of Drupal, so yes, I absolutely want to see them have a life beyond drupal and have already used them in a few non-drupal experiments with fantastic results. PSR-4 is just another autoloading paradigm. |
|
Drupal 8 is doing a lot of things right - it's just doing most things differently. What can't be "fixed" is the decision that was made to leave most of the existing contributors behind. Backdrop aims to provide all the people who would leave because of that decision (people who have already left D8 - like quicksketch - and all the other developers we've talked to who already have exit strategies, they are learning Node.js, Ruby, and other systems in their spare time to avoid having to "learn" Drupal 8). This gives those people someplace (a Drupal-based place) to go. Back to the issue at hand... It's important to stay focused on the goal here. I think using plugins in a system other than Drupal is an extreme edge case. I don't think that should weigh very heavily on our decision on what to do with Backdrop internally. If people want to use Backdrop plugins in some other system, they can solve that problem then :) Can we avoid having any namespaces at all, and thus not need PSR-4? |
|
The intent was to "attract a higher tier" of developer. The reality is that we are leaving the lower tier behind. We want to go the other way with Backdrop. It's a different product for a different audience. :) |
|
I like namespaces. They can be useful in any project. They're future proof. I don't like:
Just my 2c. |
|
It's not a global var in CTools, it's local. |
AFAIK I can't put a line of comment within the doctrine annotation. So I can't do this:
- @Plugin(
- id = "ham_sandwich",
- foobar = "This is an example value that is defined in the annotation.",
- // I need to explain why this is set to 426.
- calories = 426
- )
|
How can you write documentation about something you don't understand? |
|
"Can we avoid having any namespaces at all, and thus not need PSR-4?" Sure... though as chx already pointed out, not really, and there's a php embraced standard for class autoloading already out there (and being improved for the verbosity of its directory structure). Anything else is a non-essential drupalism and something additional to maintain that is a.) hard and b.) not really a good use of your time. yeah, I'm in irc day and night, I've explained plugin in detail, even the really high level concepts like interaction with Config Entities to ANYONE and EVERYONE who has asked. I've made myself ridiculously available, in fact, probably TOO available from my employer's perspective, and I continue to do so. If you don't know plugins yet it's not because the resources weren't there (as there actually is pretty good docs both on d.o and in change requests around things that have changed to plugins, and given the number of plugin systems in D8 that I DIDN'T write (hint it's over 20) other devs are getting it too. Understand, this isn't me trying to say "don't fork", because this is how open source goes. This is me saying "that's a bogus argument". Furthermore, devs write docs on stuff they don't understand ALL the time as part of learning it, so again I'd say "bogus argument". Most of quicksketch's comments on improving annotations for plugins and various plugin critiques came from his initial experience with them. A developer's thoughts and reactions during the first interaction are CRUCIAL and precious gifts as an API writer. As a perfect example of this: http://youtu.be/NiO9Z8s3yRA was Chris Weber's attempt at creating a block plugin for the first time. Furthermore, "Plugins" is really just an OO approach to the less formalized drupal info hook & subsequent dependent hooks like hook_block_info() and hook_block_save(). Drupal is essentially emulating an OO style process when you see this paradigm, and there are language features which we've utilized that can do all of that for us and re-organize the code into something I (at least) would consider more sane. Sure D8 ends up creating a lot (perhaps a ridiculous amount) of files, but by and large, those files' uses are very specific, and straight forward. With regard to comments on annotations. Sure, you can't do that but you could do: /**
As far as YamlDiscovery goes (which appears to be the actual problem in the issue you cited, as I've mentioned before Annotations has a great solution for the documenting problem), I'll say this: Any metadata layer that is not backed up by some other file structure (annotations have an Annotation class, info hooks had the *.api.php convention) will have this problem. XML, json, yml, info, whatever... they'll all have this problem. They'll all also have translation difficulties to overcome. Much is this has to do with my Annotation justification, as the lesser of all the evils. Even static methods as metadata would have this problem without some convention, only Annotations have a natural place for this to all exist built in by default. CTools methodology, as @merlinofchaos has already pointed out, is not what you seem to think it is. Annotations is a perfectly good approach for all the reasons I've listed above (even solves problems the other approaches have serious difficulties with like documenting the ArrayPI and translations), so the fact that you "don't like it" is sort of... disheartening. Lots of people, myself included, worked REALLY hard to make Annotations as good a solution as possible, and after all the dust has settled, it's still the best by a long shot. I'm sorry it has a weird syntax. As @chx has already pointed out, contributing to Doctrine Common and making it better is probably the best solution, and honestly that's a REALLY good solution and I applaud chx to no end for doing it. Finally, I have to quote this: "[I don't like:] every function to be a class and every class to have 4 meta methods (that would be 1 simple _info hook in D7)" That's the very definition of hook_block_info() and the subsequent hook_block_view/save/configure/delete/form/validate/submit methodology is it not? That is NOT simple, that is, in fact, really really obtuse, and while, for the weekend hacker "just implementing a hook" is sort of cool, maintaining the crap that goes on under the hood of that mess of spaghetti is... obnoxiously difficult, especially when you want to begin doing something useful with it, and when we asked the developer to learn our obfuscated OO principles for the sake of "learning" instead of ACTUAL OOP then all we really did was further remove them from the rest of the php contributing world, which perhaps explains exactly why so many of the developers mentioned here in have "exit strategies" to non-php systems, because the VAST majority of php systems out there are adopting php 5.3.x+ methodologies, Composer, PSR-0/4 autoloading, and some are even standardizing on Symfony's routing system. Zend is putting out these sorts of components, if that's any indication of the php climate, Drupal 8 is even using one. I say all this to point out that rejecting these things isn't rejecting Drupal 8 so much as it's rejecting php, granted there are some questionable implementations of stuff in D8, but that's not what this is all about so, I submit to everyone that spending some time in the greater php world might bring some perspective to D8. It certainly has for me. (and I'm NOT just talking about Symfony here) Eclipse |
|
@joachim-n Since it's come up several times in the comments here, there is an issue for comments inside of annotations: doctrine/common#264 |
|
Not much to contribute, but having worked with or written the different form of D7 plugins extensively, and having played with D8 plugins enough to understand them, there is a lot about the D8 plugins that I actually get and understand, and wished I could have had in D7. Namespaces are good just so they can be reliably autoloaded and personally I have no problem with them, as long as PSR-4 is used. Having proper interfaces are great. Where we get into the managers and crazy stuff that happens there, the less I understand. I don't enjoy annotations at all. The reality is that many of my plugins in D7 require dynamic results in the whatever_hook_info() that would gather the available "plugins". I find annotations difficult to understand, see what possible values can be used. Mostly I find when we introduced the different annotation 'types' to be the most confusing. I would rather be able to copy/paste some lines from another plugin and be able to tweak what I need in another new plugin type implementation. |
|
Right, so I'll own that problem. It's a documentation problem, and a code example problem. I'm sort of surprised the managers are at all confusing. I'd love to take that conversation on in irc some time. Eclipse |
That's great, and I really appreciate that you're on hand to answer questions. But surely it is more efficient to spend time writing code docs, so the code is easier to understand? I was at a Drupal Drinks last night, where I showed the example plugins I've made. People asked me what these lines are:
use Drupal\Component\Plugin\PluginBase;
use Drupal\Core\Annotation\Translation;
use Drupal\Component\Annotation\Plugin;
and I had to say "I don't know what they do, you just need them" :( It would take you or someone else who's been building the plugin system less than 5 minutes to add a single comment line above each of those to make it easier to understand. Then everyone stands a better chance of understanding it. It would take someone like me who is trying to understand the system hours to do that. The idea that documentation is best written by people who DON'T understand the system at hand is ludicrous. I don't think it's given credence in any field other than programming. It's simply a totally inefficient way of doing it. If brain A contains information, and you want brains B, C... Z to also have it, then brain A provides the information. You don't tell brain B to go beat its head against a wall for hours in order to figure it out first, because there is someone who already knows that! I write documentation in my code as I write the code. I write explanations for future-me-in-6-months, for anyone else who might want to use it or extend it, and finally to help me be clearer about what the code is doing. I don't understand why everybody doesn't do that. |
|
Certainly in an example module, every 'use foo' needs to say why it's there. I need a brain A over here (https://drupal.org/node/1668362) to supply those bits of detail. I'm happy to receive that data as scrappy breadcrumbs -- I can deal with polishing it for clarity. And in my own code I'd probably have a single line comment above every 'use', just a quick reminder of why it's there. It's quicker than having to go look at the class. And also, the class tells you just what the class is for, not why you're wanting it specifically in the place where the 'use' is.
Have you seen some of them? See https://drupal.org/node/2087103. |
|
@EclipseGc I was hoping that I know the ctools global isn't a global, but in that file it's global, which looks weird. Would fit perfectly in an info hook like the rest of Drupal. I'm hoping Backdrop would do something like: Maybe I'm being very naive, but that looks simpler... |
|
Well I'll just put this out there that if we don't want to use namespaces and PSR-4, that I propose the following for plugins and plugin discovery: |
|
info hook discovery certainly exists, however your "simpler" separates the metadata from the code that will actually run, and it's very very valuable having the metadata on the code that will run for the plugin. Having them in a predictable directory structure also leads to DX gains since I can look at my Plugin dir and see exactly what types of plugins are provided by a module. Furthermore, in the long run we should be looking to remove the module file entirely. We have the tools available in D8 now, and just haven't completely embraced them. Hopefully for D9 we can do exactly that, and then we don't have any procedural code sitting in a file that must ALWAYS be loaded whether it's relevant or not (cause that's what a module file is). For all the extra code D8 has, it's average memory footprint is actually smaller than D7 (I can't claim credit for all of that, but Plugins didn't hurt in that regard). All of that to say this: Drupal, is leveraging php's PSR-0 (and potentially PSR-4 soon) autoloading. The rest of the php community at large is already using that approach. You don't know what a "use" statement is? don't know what a "namespace" statement is? that's ok. But it's not a "Drupalism" it's not a "Symfonyism" it's a phpism. If I chose to leverage that system in order to facilitate discovery, all the better. If you don't like that, that's fine, it's why we worked so hard to make "Discovery" a swappable mechanism. Build your own if you think you can do better, I promise not to take offense, but I also promise this: I put in a LOT of time and effort researching, coding and at this point, streamlining, the approach I felt would be best long term. Annotations were suggested by DamZ and @bojanz, chx supported my efforts, has contributed to Doctrine in order to make it happen, and continues to do so to better the experience. Neclimdul, effulgentsia, quicksketch himself, timplunkett & myself have all worked on the Annotations DX to make it better as well on the Drupal side and the community itself seems pretty split (I'd say about 50/50) between loving or hating annotations. I maintain that they're totally worth the investment of time on your part (and on mine) so I'd still heavily encourage using them, but of course, discovery is swappable specifically so that I can ultimately defer when people balk, so... write your own discovery. It's pretty easy, but beware the maintenance nightmare that is a bunch of definitions in a single info hook. :-) Eclipse |
|
so "use" and "namespace" statements are all part of php 5.3.x+ (I think). They have nothing to do with Plugins, Symfony, or anything else except PHP itself. When you see a "namespace Drupal\Component\Plugin;" that means that our "class PluginBase" exists in that namespace, so the class' full name is Drupal\Component\Plugin\PluginBase. This means that if that class weren't abstract, you could literally do: $plugin = new \Drupal\Component\Plugin\PluginBase();Have a use statement simplifies this further. It acknowledges that there are classes you need and you don't really want to type that long long name each time. So when you do: use Drupal\Component\Plugin\PluginBase;What you're really doing is bringing that class into scope so to speak so that you can more easily refer to it in your code as simple "PluginBase". So if our "BlockBase" class were to extend that we'd do: <?php
/**
* @file
* Contains \Drupal\block\BlockBase.
*/
namespace Drupal\block;
use Drupal\Component\Plugin\PluginBase;
abstract class BlockBase extends PluginBase {}
?>When you see this you're literally saying "use Drupal\Component\Plugin\PluginBase as PluginBase;". You could sub in any name you want in that "as" section and you'd be able to refer to this class as that from now on, so...: use Drupal\Component\Plugin\PluginBase as Foo;
abstract class BlockBase extends Foo {}This is totally kosher. Hopefully this helps you read some of what you're seeing better, but this is all php specific stuff and has nothing to do with Symfony, Composer, Plugins or even Drupal 8. This is just a convention in the wider php community that all these frameworks and tools have adopted. I'd encourage you all to adopt it too. Eclipse |
|
Thanks for your input Kris. I really appreciate your feedback in the queue, and considering the amount of experience you've had with Plugins in D8. Though some of the comments are definitely targeted more at Drupal 8's direction than where we want to head (still useful though).
I think this is a D8/9 goal, not something we're pursuing in Backdrop any time soon.
My benchmarking does not support this. On average, D8 takes twice as much memory per page load for something like a node with fields on it, or the front page view with 10 nodes. Additionally, if you're using APC, it doesn't matter if an individual page uses only some of the available *.php files, since ultimately, 100% of all the PHP files on your site need to be able to fit inside the APC cache size. The more code you have, the more space it uses in APC, even if it's not all loaded at once. Thanks for your continued input, I'd just like to reaffirm some areas where we're probably not going to be taking Backdrop. I'm not sure what to do about autoloading at this point. PSR-0 and D7's class registry seem equally hated. PSR-4... might be an option but I still think even the use of namespaces might be too complex. The door is wide-open on options for autoloading (or avoiding autoloading entirely). If we do end up with slashes mapping to directories, we'll definitely be looking into Mark Sonnabaum's recommendations of short namespaces. i.e. views/views_query instead of Drupal/views/views_query or backdrop/cache instead of Drupal/Core/Cache/Cache, if possible. PSR-4 solves nested directory problems, but namespaces in Drupal 8 are still too long. Ultimately, I'd love to see an efficient class-discovery mechanism that is Backdrop-specific. We already know a lot of information about our own system, perhaps that can help us build an autoloader that is less verbose than the requirements of PSR-0/4. I'm not sure that's possible, but I'd like to attempt to find it. |
|
Nate, Absolutely, I'm happy to be available for any of this sort of stuff if it's welcome. Obviously I'm speaking a bit out of both sides of my mouth here since one the one hand I've put a lot of work in to D8 and on the other hand, I'm doing a little bit of advising here, so there's definitely some unclear bleed over in some of what I've said, and I apologize for that. With regard to the benchmarking, I will 100% gladly admit that I'm not a performance/benchmarks guy, but I've definitely seen benchmarks to that affect. I'll see if I can dredge them up so that I don't look like I'm just making crap up ;-) It could be that I've mistaken one state for another. In any event, your point about APC is well taken. As far as autoloading goes, I said my peace in irc, but I'll restate it here for the sake of posterity and anyone reading this: Not embracing PSR-0/4 limits your ability to utilize the modern code that's being generated in the wider php community today, and in addition to that, it cripples any efforts you might make to generate such components within Backdrop. The plugin system in D8 is a perfect example. I really truly hope you all use it. I'm trying to make sure other php projects can use it. But if I succeed at that, Plugins will be a PSR-0/4 citizen, not a drupal module style solution, and the interplay of those things, while similar, would require an awful lot of tangential architecture that you'd not use anywhere else if you didn't use PSR-0/4 for your own loading. Also, it's worth mentioning that you have two basic styles of autoloading available to you. Drupal 7 does essentially "class registration" through info files, Drupal 8 (actually Composer) leverages magic naming conventions with some directory mapping. Those are basically the two options if you don't want to leverage code generation and put all classes into a single file and just load that file all the time. In any event, I'll be here to advise :-) Eclipse |
|
I think it's interesting that it's been implied further up that one of the reasons people aren't writing docs for their own code is that they lack the ability to write clear docs. Then perhaps there are also abilities that people lack:
The thing about the gang of 'use' statements is not to do with 'what does PHP do here'. It is to do with a newbie developer opening up a plugin class, and being confronted immediately with more complex names that they don't understand. Your brain hits things like Annotation\Translation, DisplayPluginBase, PluginBag, etc etc. It makes one recoil -- perhaps you don't understand that experience, but I am telling you it exists. We all understand how successive UX hurdles make more and more users turn away and give up, based on how much they want to accomplish the task: if my bank website throws them up, I'll plow on. If it's a surveymonkey form someone linked me to, I'll drop it at the first fiddly bit. DX is the same. For every hurdle, we lose developers. We lose the people who just thought 'Hey I'll look at this Drupal thing', and then we lose the hobbyists, and so on until we lose the contrib devs. Stop telling me the code works for you and listen to other people saying it doesn't work for them. |
|
I'm not exactly sure about your continued statements regarding docs, whether you're discussing plugins specifically or D8 in general. There's a whole section on d.o of documentation on D8 Plugins, and has been for months and months. It needs updating, no doubt, but it's still generally good. With regard to your DX/UX analogy, ok sure, but the difference is of course that as developers, our job is essentially to be constantly learning. When you see a new language feature, you should go learn it, not complain that someone made use of it, and again I'm just trying, desperately, to point out the temperature of the rest of the php world. As some defense of this: https://github.com/doctrine/common/blob/master/lib/Doctrine/Common/Reflection/StaticReflectionClass.php And this is just a small sampling other php projects out there. I didn't include any Symfony code here (so, it's not Symfony) I didn't include any D8 code. Yes D8 is using all of these libraries, but just spend a little time on https://packagist.org/ browsing the various repos they reference and you'll see, this is common place in php today. All of this goes back to my main point, php left Drupal behind, and now we're just playing catch up. Do we all have a lot to learn? YES! Was it painful at times to learn it? YES! But it was also totally worth it, as learning new things often are. All I can do is encourage you to stop complaining about language features, and instead spend time learning what they mean and how they're used. Eclipse |
|
We're talking about the plugin system being complex and how more needs to be done to make lower the barrier to entry. And I don't think you've understood my point about how it's not the language features, or how they're used, but the density and complexity they present. Here's an example I've just spotted: I have been working with Zen theme today. Download it and open it up. Look at all the redundant code: the comment-out examples, the links to documentation, etc etc. Zen does a lot to lower the barriers to developers. Now themes tend to have to do that a lot, because a lot of the people working with themes are not very experienced coders. But that is the sort of barrier-lowering that the plugin system should do a little more of. |
|
@joachim-n There's nothing about the plugin system that's not classic OO except the discovery mechanism, which as I've already said is completely swappable and for the record is only 2 methods. The burden of learning here is QUITE SMALL. Furthermore, the documentation on the corresponding interface is quite good, and clear: The same is true for most of the plugin system: The Factory Interface (which instantiates plugins): One of the Implementations of that interface: The Plugin Inspection interface implemented by PluginBase: Even the PluginManagerInterface, which just extends 3 other interfaces, has pretty extensive documentation on it: And so on and so on and so on. You're totally right, the presentation I was referencing in my head was alexpott's presentation on cyclomatic complexity, which for the record was REALLY great. Eclipse |
|
I was researching more today on exactly what D8 plugins provide. Why are they desired functionality at all? Are they necessarily OO, or is it because OO lends itself to the plugin concept? @EclipseGc actually answered these questions pretty effectively in the Drupal.org post for Why Plugins?. Though the "official" doc is terrible, Kris's response in the comments is excellent. If you haven't, I suggest reading the whole post, but it can be summarized with these two sentences:
Plugins are essentially nothing but a discovery mechanism (in D8 they're annotations) and replacement for info hooks and callbacks. I'm not sure that we should be looking at replacing those concepts in Backdrop. In the original Drupal 8 plugin issue summary, there's a laundry list of info hooks that were eventually replaced with plugins: hook_block_info(), hook_image_effect_info(), hook_filter_info(), hook_action_info(), and others. Here's the thing, if we create a plugin system that is similar to D8, we'll likely slip down the same slope of converting pretty much every info hook to a plugin. I'm not sure that should be something we invest in for Backdrop, maybe not in the initial release or even ever. |
|
The problem with doing it all non-OO is that the number of callbacks soon makes things messy. When defining a block meant just a hook_block() with an $op switch inside, it was nicely contained. The $op parameter was removed from most things in D7, and with good reason, but it means that you need more functions. And you either have to go down the route of having a set of hooks (as for blocks), or have the info hook define callbacks. Commerce has quite a few instances of that pattern. The advantage of having named callbacks is that you can hope to reuse them, and that if you provide X different 'non-OO plugins' in your info hook, you don't have to still have a switch() for each ID in all of the subsidiary hooks. But it does mean you need a lot of functions, and you need to try to name them consistently to avoid clashes and maintain sanity. Wrapping those all up into a class is a nice way to group them together. If nothing else, it improves DX: you can clearly see all the functions that belong together, no need to carefully devise function names, no need to list them in the callback. The ability to then use inheritance is a nice bonus. And class autoloading means that we get the same functionality as having magically-named .inc files, but all the loading is done by one system. Annotations on the other hand I find faffy. The problem with discovery was that loading all plugin classes caused memory problems, but I think we could stand to have discovery outside of the plugin class: either a classic info hook, or a mymodule.plugins.yml file, or something like that. I know that splits up the plugin and its definition, but in D8 registering a service is separate from the service class; in D7 the info hook is apart from the other hooks and callbacks -- so this would be no different. |
|
Thanks @ joachim-n, great feedback. Some CTools plugins are similar in that regard. They use classes for the plugins but use _info() hooks for discovery. It's not a bad combination really. If we take that approach though, it doesn't necessarily mean a plugin "system", it could just be "info hooks that reference classes". Basically, a half-way point where you use OO when it's appropriate to save yourself the trouble of specifying 5 different callbacks (or some equally-awkward hook naming pattern). |
|
CTools D7 also comes with a 'file discovery': you implement hook_ctools_directory (IIRC) and tell it that any module's sandwich plugins live in a subfolder /plugins/sandwich. CTools then just reads all .inc files in any module with that arrangement of subfolders. You then have to have the plugin info at the top of the file in a bare variable. I've implemented it that way a few times. It's less nice than the info hook, but it's quicker! |
|
I have a hard time arguing with your basic logic. The BIG benefit of the plugin system is the OO aspects of it. I guess, personally, I've longed for inheritance in so many different info hooks so often that this seemed like an obvious need to me. Likewise, since we were going to have to re-architect the block system to allow for multiple instances of the same block, it didn't make any sense to keep with the procedural status quo. In both blocks and ctools content_types, a lack of inheritance has been a huge failing, time and again, and adopting it should (in the long run) cut down on how much code you have to maintain (not to mention give you a sane place to put some of the WTF that our procedural OO style approach currently sprinkles all over kingdom come). Obviously, I sort of have a horse in this race, but I also respect your basic premise, I just think it's worth the effort, even after having done it and it being a huge pain in the ass. At least you could totally benefit from all that and short cut things significantly. With regard to the discovery, I think we've beat that horse to death, but I'll correct the little tidbits about ctools that are wrong just for the record: Implementing hook_ctools_plugin_directory($owner, $type) requires you to pass back a directory string per plugin type definition. This is an important distinction because we don't have any sort of natural namespacing on our plugin types. For example, I think both ctools and panels define a "cache" module. In this case you can't simply return "plugins/$type"; because the $owner will be ctools some times and panels other times and these plugin implementations are actually very different. During the D8 cycle, this was VERY difficult to communicate. For annotation based discovery, we had to solve essentially the same problem, which, for a time, consisted of a very very similar solution (and an enforced extra directory). In D8 today, we utilize the Annotation class & the Directory as the two points of distinctness in order to only pick up plugins we actually want to use. An info hook obviously has none of these problems, but we value the placement of metadata on the plugin enough to go to this effort. This means adding new plugins of a type is as simple as putting a class in a particular dir, and annotating it properly. (as opposed to manually registering it via some completely different file) Based on where I THINK backdrop is going, you solution is likely to be
This isn't particularly onerous, though I'd argue it's more so that putting a class in a dir. Yes, my version is definitely more "magic" but it's the sort of magic that the rest of the php world at large is adopting, and so shouldn't be too weird really (though Sonnabaum will argue that our specific annotations implementation is weird I think it should be pretty easy for most of the Drupal-experienced to pick up and it's improved a LOT since @quicksketch's initial feedback). Ultimate, even ctools plugins are usually not OO, and the ones that are are a very strange implementation. I like them, but I think they're more confusing that what we did in D8, and the only groups I can think of off hand who have implemented them are ctools itself (for export_ui plugins) and DevSeed for Feeds module. I'm sure there are others, but it's really uncommon. Ultimately, though, my entire argument is based on OO for the sake of inheritance. If you're not buying that argument then I sort of have nothing to add :-( Eclipse |
|
Agree with @EclipseGc's probable solution, except:
I'd like to see namespaces for that. No registry (D7). No forced folder structure either though (D8)! Just tell the plugin/info thing where to find your block/feed/action class. If your module has 30 plugins over 6 types, it'll probably have subfolders. If it has 1 block, it might not have any subfolders. Whatever the module developer wants. (Easy is the Backdrop premise, isn't it?) Like in #41 (comment) and #41 (comment) This one's important, but it's also the highest priority IMO. Every other Big Thing will depend on the plugin system. (E.g. some kind of PluginBase or PluginInterface would be nice. Or not?) |
|
Namespaces ARE extra folders to a certain extent. hook_autoload() (or some name) is something @quicksketch is already toying with with good results (or at least that's my impression) and it seems that people have misunderstood the reasons for "forced folder structure". If you open up a folder with dozens of classes in it, which are what plugin type? You have no clue. Beyond the fact that you CAN name the classes, nothing FORCES you to. Furthermore it's actually a DX win (in my opinion) to open up someone's class library and see a Plugin directory there, and then open that and find Block, Action, Condition, etc. It's RIDICULOUSLY clear what sort of plugins exist, and I don't have to guess, not even a little bit. The fact that that supports our "discovery" is a really awesome benefit imo, but the greater win is actually from the DX side. If you find that arbitrary and distasteful, then I'd encourage you to do more work with plugins. I've done a LOT with ctools over the years and having that particular structure in place there made my life a LOT easier when it came time to deal with other people's code. Eclipse |
|
Over in the class registry issue, I posted a long comment (#77 (comment)) that outlines the purposes of a plugin system. Here's the key point:
As far as "loading" goes, D8 uses PSR-0-based autoloader to handle the loading portion of things. Plugins in D8 are essentially reduced to Discovery (based on naming conventions and a directory structure) and Definition (in D8 they use Doctrine-based annotations). In Backdrop, we tentatively plan on also using an autoloader, but as for discovery and definition, we'll probably stick with *_info() hooks for this functionality. The benefits of _info hooks include:
There are downsides as well of course, but considering our goal of iterative development, if we adopt a wide-scale plugin system and convert everything to it, we're only going to go down the same rabbit hole of massive conversions. So for now, let's put to rest the concept of a "plugin system". All we need are _info() hooks for the purposes of discovery and definition. Loading can be done via the autoloader (if it's a class), or simply specified in the info hook via a "file" property as we've done for non-OO code in existing info hooks. The concept of a plugin is meant to be a shortcut and pattern for consistency, but comes at the cost of introducing an additional concept and converting things to use them. |
|
Just for the record, there is also https://drupal.org/project/renderkit with /cfr, which in many ways works as a "plugin system", but without the overhead on plugin implementations. |
A plugin architecture forms the foundation of both CTools and Views in Drupal 7. In order to include Views in core, we may need a (hopefully mostly-compatible) plugin system to load these files.
The general consensus on D8's plugin system is that it's quite good, other than the PSR-0 directory structure and the use of Annotations for discovery. Until we decide otherwise, perhaps the typical _info() hook structure is the fastest/most-compatible way forward.
Or alternatively, do we even actually need plugins? What do they provide that we don't have already?
The text was updated successfully, but these errors were encountered: