Cms: Remove plugin handles?

[brandonkelly]

In Craft 3, plugins can be identified by 3 different unique attributes:

• their handle
• their Yii module ID (currently auto-generated by Craft as a kebab-case version of the handle)
• their Composer package name

Most places in the Craft codebase that currently deal with plugin handles could fairly easily start dealing with module IDs instead, which makes me think that we should consider removing handles altogether.

The only thing that would be tricky is figuring out what to do with template variables. Currently all plugins can optionally have a template variable, accessible via craft.pluginHandle.

Part of me is thinking we should just stop automatically giving plugins a seat on the craft variable though. Instead we could just start firing a registerComponents event from craft\web\twig\variables\CraftVariable::init(), where plugins (or just regular old modules even) could register new craft.xyz components. Downside is we won’t be able to guard against two plugins trying to add the same component; upside is the component ID is no longer strictly tied to a plugin handle, and a single plugin could even add multiple components if it wanted.

[selvinortiz]

If craft.vendor.pluginHandle, vendor.pluginHandle or vendor.plugin.variable (which might be a way for plugins to register multiple template components) are not viable options, and we have to stick with having everything in craft.pluginHandle, I guess the risk of name collision is a reasonable one to accept.

If the binding/registration into the craft variable is left up to the developer and Craft is no longer able to enforce a naming convention based on a handle or module id, we should at least propose something that everyone follows.

[brandonkelly]

Sure, convention would be good, and we could note that in the docs.

[lindseydiloreto]

@selvinortiz The craft.pluginHandle collision risk already exists in Craft 2, and I've never yet heard of an actual incident.

If someone ends up in a situation where they have two conflicting pluginHandles, there's a good chance that they need to change something anyway. A situation like that is most likely to stem from (A) two plugins which do the same thing, or (B) a custom plugin which the developer created.

Undoubtedly, that would be an edge case either way.

[selvinortiz]

Yeah, I think most developers in Craft 2 are aware of the potential name collision and it's easy for us to name our plugin uniquely.

It's worth pointing out that (A) and (B) are not the only possible cases here. The most likely scenario is (C). _Where someone wants to use two plugins because they do different things_. For example, many people are gonna be writing Reports plugins for Craft Commerce and you could very well want to have two of them because both have features that the other one does not._ They both might be named reports.

If it's not worth having, or if it's not easy to implement something like vendor.variable, I really don't have an issue with craft.pluginHandle. Not a big deal either way.

However, this thread is more focused on whether or not the plugin handle thing should be removed. and I say yes, _let's nuke it_ ;)

[brandonkelly]

@lindseydiloreto craft.pluginHandle conflicts are not possible because you can’t have two plugins installed with the same handle. So Selvin is correct that this introduces that possibility. But I think the odds are pretty slim that two plugins that register the same component ID would ever be installed on the same Craft project at the same time.

[brandonkelly]

Worth noting that you can access a plugin instance from your templates in Craft 3 (along with any of its components/services) like this:

craft.app.getModule('module-id').componentName...

Maybe we could provide a global module() function that simplifies it:

module('module-id').componentName…

Then encourage plugins to defining functionality one time in components/services, where it’s available to other PHP code as well as templates

[brandonkelly]

…which makes me wonder, if we did that, would it even be worth having a way for plugins to add components to the craft variable? Thinking not.

[selvinortiz]

@brandonkelly Exactly what I was thinking. In Craft 2, template variables provided a _blackbox/safety blanket_ if you only wanted to expose certain things to the templates. In Craft 3, you're already giving access to everything, so it's not a big deal if all services/modules are accessible by the template.

I would not mind it if we dropped the whole template variable concept alltogether ;)

[selvinortiz]

@brandonkelly Where do you think that leaves us with Twig filters and functions?

[brandonkelly]

That’s still possible, by registering a Twig extension.

[brandonkelly]

And Twig extensions can register new global variables too.

[selvinortiz]

Gotcha! Twig extensions will still be useful then 👍

[michaelrog]

For all my C3 stuff, I plan to register my own template variable as a 'shortcut' (i.e. printmaker.whatever as an alternative to craft.printmaker.whatever). Obviously, if there's a going convention for registering variables in a centralized way, I'd conform to that also, but bottom line, I'm with Selvin: I'd support a decision to do away with craft.handle-style variables altogether.

[nateiler]

I like the idea of removing the plugin handle. I thought they were only used for translations...but seems somewhat redundant with the module Id.

I don't like having all service method available via twig. I view Twig variables as a consuming (read only) mechanism. Allowing create/update/delete seems to circumvent access controls that a controller should enforce. Yes, the training wheels are off, but I would like to enforce best practices.

Would it make sense to allow plugins to manage / register twig variables via an event? The event could have a property that is key based and we could construct the dot notion...example

$variables['vendor']['plugin'] = Foo::class

Would result in

vendor.plugin.method

[nateiler]

Also, not plugin related directly, but I would like to use a project specific module vs plugin moving forward. Having some consistency (via events) between the two would be really nice...where they're exactly the same except one was initialized via the plugin service and the other was not. Maybe this is already the case, but it came to mind (the twig variable method makes me think not)

[brandonkelly]

I don't like having all service method available via twig. I view Twig variables as a consuming (read only) mechanism. Allowing create/update/delete seems to circumvent access controls that a controller should enforce.

@nateiler Yeah, that’s how we felt originally, too, but there’s not a great way to enforce it, short of how we did it in Craft 2 where you create separate wrapper classes for services that only expose the “safe” methods. It works on paper, but proved to be a maintainability nightmare, so we opted to just add expose the entire Application instance via craft.app instead for Craft 3. We’re still a little concerned about the sketchy possibilities it opens up, and may add a config setting that you have to manually enable if you want it available on the front end, as a way to warn people before using it too liberally. I suppose if we did, the module() function also ought to be stuck behind that config setting, and at that point module('module-id') becomes a much less attractive alternative to craft.pluginHandle. Hmm…

[angrybrad]

and may add a config setting that you have to manually enable if you want it available on the front end, as a way to warn people before using it too liberally.

The more I think about this, the more I think we should do it.

[narration-sd]

Would tend to agree, as Brandon's mention of 'sketchy' possibilities certainly seems to raise the idea of such with teeth.

Freeing ability to sert values others might use has some appearance of a security hazard, especially as Craft extends into money (commerce and business apps) areas.

Open tends to sound good, but as far as I'm aware the majority of security problems are actually with inside personnel. Then keeping control with administrrators of whether 'just anyone' scripting can write to variables just feels like a very sound idea.

[nateiler]

It works on paper, but proved to be a maintainability nightmare, so we opted to just add expose the entire Application instance via craft.app instead for Craft 3.

Yes, managing the v2 mirror of twig variables -> service components can easily become a nightmare. I thought craft.app.* was perfect until I realized one could loop across all the elements and delete them via twig.

1) As mentioned above, enabling service components via setting is a good option. Is it safe to assume the base Yii2 application components (request, session, etc) would be available regardless of this setting?

2) Another option that popped into mind is extending each the service classes and extending the create/update/delete operations by throwing exceptions...not sure if this would be less or more of a nightmare.

3) Or there is the event approach. At the end of the CraftVariable init an event could be triggered and allow plugins to add their own variables via a component key => Value::class array. This could also be the way to define these sketchy variable services...along with third party plugins.

4) Finally, as @michaelrog brought up, I'm tending to lean towards defining a non craft. variable. Not sure the performance implications of this yet. This could also be the de facto standard for all third parties (maybe there is an abstract extension to assist in defining this).

Plugin devs should be pretty comfortable with events (it seems everything is event based except the Plugin::defineTemplateComponent()) so #3 or #4 shouldn't be overly cumbersome.

We also might be getting a little off topic for this particular module handle issue. Sorry.

[brandonkelly]

Is it safe to assume the base Yii2 application components (request, session, etc) would be available regardless of this setting?

No; the setting would determine whether craft.app is available on the front end (plus the module() function). All of the application components branch off of that.

We also might be getting a little off topic for this particular module handle issue. Sorry.

Hah, no worries; template variables are pretty much the last place plugin handles matter, so we need to solve this before we can go forward with removing handles.

[brandonkelly]

Next part we need to figure out is how to handle the transition from plugin handles to package names.

Craft 2 and 3 keep track of which plugins are installed/enabled via their handles handle in the plugins table. So if/when we move to package names, there will need to be some way to associate the existing data with the now-handleless plugins.

Option 1

Rename the handle column to name, but keep existing values in-tact. Put zero work into trying to help the transition for existing plugins. Initially it will look like no plugins are installed anymore, and people will have to manually update those values as part of the Craft 3 migration process. Or plugins could do that for them from their beforeInstall() functions:

protected function beforeInstall(): bool
{
    // Check if the plugin is already installed with the old handle
    $id = (new Query())
        ->select('id')
        ->from('{{%plugins}}')
        ->where(['name' => 'oldhandle'])
        ->scalar();

    if ($id !== false) {
        Craft::$app->db->createCommand()->update('{{%plugins}}', [
            'name' => 'package/name'
        ], ['id' => $id]);

        // Prevent double-installation
        return false;
    }

    return true;
}

This is the cleanest approach as it will be a clean break from handles for Craft. But also the most inconvenient in the short run, since it's a breaking change for existing Craft 3 plugins, causing Craft to “forget” which plugins are installed until the values are updated.

Option 2

Rename handle column to name, but keep the existing values in-tact. Keep a $handle property on plugins, and support for extra.handle in composer.json, but make it optional, only used by plugins that had supported earlier versions of Craft. When Craft is initializing plugins at the beginning of the request, if it can't find one by its package name, it can check if any plugins have a matching $handle property instead as a fallback. If it finds one, go ahead and update the name column to the actual package name for the plugin.

This would be less taxing on plugins, but would come with slight hits to code complexity and performance. And we’d have to decide how long this fallback stuff gets to stick around for, and what will happen after we remove it.

Option 3

We could start preparing for this now in Craft 2, by adding a new name column to the plugins table, and give plugins a getPackageName() function where they can start identifying themselves by that. When a plugin is installed/updated, Craft 2 could start calling that function and populating the name values.

Then for Craft 3, the migration would populate any empty name values with the handle values (just so it's clear which rows were meant for which plugins, for those that never got a Craft 2 update).

With this there’s still a burden on plugins to get updated, but just a simple getPackageName() function, and just for Craft 2. It’s still a clean break for Craft 3, like option 1. (It would be just as inconvenient for existing Craft 3 installs as option 1, but hey it’s a beta.)

[angrybrad]

@brandonkelly I want to vote for option 1. It's the cleanest. No-one _should_ be using C3 on production. But I just ran the numbers and there look to be about 400(?!) public domains running Craft 3. I'd guess 150 of those are live (non testing) sites. We could shoot an email to the owners of the license.key files and give them a head's up.

Of the 37 Craft 3 plugins that would need to be updated, 10 of them are owned by us. And don't think it'd be too much to ask given the beta status still.

[narration-sd]

Thinking options over twice, I agree with Brad @takobell.

Even though you know I'm usually the stalwart for adaptive code....

Craft 3 is not a small breaking change for plugins period, so option 3 doesn't sound worth the play. Nor does option 2, similar thinking.

Craft 3 is an unusual form of beta, due to your guys' capability, immersed in actually listening, progressive design. This will not be the first time something changes in a worthwhile way, for what this uncovers.

tl;dr: most straightforward, sounds best :)

[michaelrog]

I'm also a big fan of Option 1, not least because I actually like the default of legacy plugins being 'not installed' until they explicitly 're-install' themselves after migrating to the new package-name-based setup.

[narration-sd]

good point, if memory seems to say they already kind of do that, as no package.json to begin with, not to say startup code wrong name and in wrong place. So that the response is a quiet no-op.

What would seem good, and of the adaptive line, would be to mark the old ones with something like a red-softcorners-boxed tag on the plugins/packages page.

Which itself is a place I've wondered about for the packages-only idea; believe I've proposed something where such packages have an identifying interface they have to support, so that reflection can identify them as needing to be managed in a plugins-like way....

[brandonkelly]

Update: In the end we decided it would be best to just merge the concept of “handles” and “module IDs”, so going forward the two will be synonyms for each other (though we will mostly just refer to them as “plugin handles”).

We toyed with the idea of making them user-defined (in the plugin settings or something), to be consistent with normal Yii module IDs, which are always defined within the application config rather than the module itself. But it would have made it more annoying to specify plugin template paths and CP section URLs, and there was no good solution for defining the plugin’s translation category if its handle/module ID isn’t known ahead of time, so ultimately we decided against this. Which means that plugin handles will still need to be unique, especially for inclusion in the Store.

The update will bring a couple minor breaking changes:

• Plugin template base paths will need to begin with the new handle format, e.g. foo-bar/settings rather than fooBar/settings (Craft 3 already supports this style, so you can make these changes now without breaking existing installs).
• Plugin config files will need to be named after the new handle format, e.g. element-api.php instead of elementapi.php.
• Plugin translation categories will use the new handle format, e.g. Craft::t('aws-s3', 'Message') instead of Craft::t('awss3', 'Message').
• Plugin translation files will need to be named with the new handle format, e.g. aws-s3.php rather than awss3.php.