Auto-detect/generate plugin "prop"

[mk-pmb]

Hi, I wondered about the "prop" requirement in "Writing plugins" and for me it didn't "become clear, when [I saw] the code". Why won't the plugin system add the "prop" property to my plugin factory in case it's missing? On most systems any function has a name property that can be used, but my gut feeling is that you should add some unique ID (e.g. counter) anyway to avoid conflict between multiple plugins that happen to have the same name, like when you use multiple copies of a "recoverSpellPoints" plugin for different kinds of magic.

Example:

function registerPlugin(plug) {
  var propUid;
  if (!plug.prop) {
    propUid = (+registerPlugin.propUid || 0) + 1;
    registerPlugin.propUid = propUid;
    plug.prop = (plug.name || 'anon') + '#' + propUid;
    console.log('registered with generated prop:', plug.prop);
  } else {
    console.log('registered with existing  prop:', plug.prop);
  }
}

var PluginA = (opts) => {};
var PluginB = (opts) => {};
var PluginC = (opts) => {};

registerPlugin(PluginA);    // prints: registered with generated prop: PluginA#1
registerPlugin(PluginA);    // prints: registered with existing  prop: PluginA#1
registerPlugin(PluginB);    // prints: registered with generated prop: PluginB#2
registerPlugin(PluginB);    // prints: registered with existing  prop: PluginB#2
registerPlugin(PluginC);    // prints: registered with generated prop: PluginC#3
registerPlugin(PluginC);    // prints: registered with existing  prop: PluginC#3

In an execution environment where functions don't have .names, they'll just be all "anon" but still get unique .props.

Update: Fixed to avoid repeated console messages. (They weren't silent, I just didn't see the "×2".)

[tdzienniak]

I don't think that autogenerating plugin prop is a good idea. You will have to remeber the generated prop in some variable and use it throughout whole codebase. Something like:

const pluginProp = registerPlugin(myPlugin)

game = Entropy({
  [pluginProp]: {
    configVar: 'foo'
  }
})

game[pluginProp].pluginFn()

I find this tedious. I can add this feature (or you can, via PR) if it's something that you realy need. But I think that guessing prop name form functions .name property is a good idea and I will implement this.

Concerning plugins with same prop name, I think that this should not be allowed and system should throw an error in this case. However, I can modify registerPlugin method to look like this:

const registerPlugin = (plugin, pluginProp) => {}

This will allow to register multiple plugins with same prop name by asigning different prop names by hand at register time.

[mk-pmb]

You will have to remember the generated prop in some variable

That's automatic: You just use myPlugin.prop.

and use it throughout whole codebase.

If you need to repeat it, you'll need a variable anyways, since using magic string constants would make your code hard to maintain. In your code example, using the pluginProp variable only serves as a minor performance boost to avoid these property lookups:

game = Entropy({
  [myPlugin.prop]: {              // <- lookup 1
    configVar: 'foo'
  }
})

game[myPlugin.prop].pluginFn()    // <- lookup 2

The same applies for having the value returned from registerPlugin. Same effect without function result:

registerPlugin(myPlugin);
const pluginProp = myPlugin.prop;

I find this tedious.

I failed to guess "this" when comparing to a magic string constant approach. Which aspect did you mean to be tedious?

As general "tedious", having to repeat the plugin name by any means might count. For lookup 1, we could probably find a way to provide a config object at the time we register the plugin. We can then modify the config object if required, as its data will only take effect later when Entropy() uses it to configure the game. For cases where it's inconvenient or impossible to create the object at registerPlugin time, we could instead provide a function which Entropy() can call to obtain the config object, passing the plugin instance as first argument or this context. For lookup 2, it looks like that's how you obtain a reference to your actual plugin instance. If you use that often, you should alias it for performance:

const game = Entropy(…);
const pluginInst = game[myPlugin.prop];
pluginInst.pluginFn();

However, since that plugin instance is created by registerPlugin(), it would seem more familiar to me to have the new instance returned from that:

const pluginInst = registerPlugin(myPlugin);

Example code with all suggested features combined:

function myConfig() {
  var plugInst = this;
  return (makeHoroscopeForPlugin(plugInst) || { luck: 'off scale' });
}

const pluggedIn = registerPlugin(myPlugin, { prop: 'customProp', cfg: myConfig });
const game = Entropy();
pluggedIn.pluginFn();

[tdzienniak]

I think it's just just a matter of preference. You are right, that syntax game[myPlugin.prop].pluginFn() is better from maintainability point of view, but I simply don't like that syntax, its my preference. :) However, I can add that feature, so people like you, who prefer that syntax, can use it. Thus, everybody will be happy, I think.

Regarding your last point, returning plugin instance from register function, it's simply not possible, because plugin is connected with Entropy instance, which is not available at the time plugin is registered. I should note, that plugins are just a syntactic sugar, at least at that point. You could write:

const game = Entropy();

const myPlugin = (gameInstance, pluginConfig) => {
  return {
    myFn() {
      // do something with gameInstance
    }
  };
}

const pluginInstance = myPlugin(game, {})

and achieve same effect as using Entropy's plugins. Entropy's plugin functionality just instanciates plugins automatically when game instance is created, that's it.