Questions: Should we recommend self-registering plugins or manually registering plugins? about scully HOT 8 CLOSED

Self-registering plugins would make them easy to consume.

Manually registered plugins might allow for some flexibility such as the type name in your example or configuration options, for example to preregister some common option like API path and credentials.

from scully.

Thank you, Sander. I'm thinking of creating some content about plugins.

from scully.

To add clarity: you would consume both of these the same way.

For a plugin that queries a CMS or an API somewhere, your sample plugin config looks the same for both of these examples:

// In your scully.config.js
exports.config = {
    ...,
    routes: {
        "/sample/:sampleId": {
            type: "samplePlugin",   // or the name you gave it when you manually register it
            sampleId: {
                cmsCredentials: {
                    publicKey: "1234",
                    privateKey: "abcd",
                },
                query(db){
                    return db.collection('sampleIds'); // in the database was firebase... example
                },
                property: 'item.id'
            }
        }
    },
}

from scully.

@SanderElias @beeman @LayZeeDK @SanderElias @jorgeucano @Splaktar what do you all think?

from scully.

The problem with "self-registration" is that somehow Scully needs to be able to pull those in.
That requires a list of available plugins somewhere. Such a list also needs maintenance.
First of all, plugins themselves need to put their main in this list.
Also, if a plugin is removed it needs to be taken off this list.
The list itself needs to be in a common-place, like angular.json, package.json or scully.config.js

I would like to propose a middle ground, that doesn't need a list, and makes it easy to pull them in:
in scully config.js:

require('./extraPlugin/extra-plugin.js')(registerPlugin);

in ./extraPlugin/extra-plugin.js'

/** plugin code here **/
module.exports = registerPlugin => registerPlugin('router', 'extra', extraRoutesPlugin);

Works with #107
This would keep the flexibility and would put in the "list" inside the scully.config.js
Also, if we go this route, I can provide something like this:

npm run scully addPluginToConfig "myNpmProvidedPlugin"

which will add the line:

require("myNpmProvidedPlugin")(registerPlugin);

to the scully.config.js if is not yet there.
Plugin authors can add this to their package.json install script.

from scully.

After talking to you this evening, I feel like simplifying the codebase by removing the dependency on vm will make this problem largely go away. If all contexts point to the same version of registerPlugin, then we will eliminate our errors here.

Sander and I spoke about the advantages of using vm and running the community/plugin code inside of the vm. It protects us from cowboy code that can break us. But it doesn't solve the bigger, harder to fix issues that will eventually arrive. So for now, it seems like we may punt and remove vm and use a simple require for now.

from scully.

Are the docs still representative of how you want plugins to be registered? https://github.com/scullyio/scully/blob/master/docs/plugins.md

from scully.

@LayZeeDK, yes no change there. However you need to import/require registerPlugin now.
(have a look at this)

from scully.