Skip to content
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

Is there a reason for instantiating a custom plugin to get its name? #280

Open
firemuzzy opened this issue May 16, 2019 · 3 comments
Open
Labels

Comments

@firemuzzy
Copy link

I do not all the intricacies of JS but the following line seems weird to me

https://github.com/taskrabbit/node-resque/blob/1a2def9f46f9be5a80a5dd8630b89d23aff2ca53/lib/pluginRunner.js#L24

Is there a reason why the plugin is instantiated to just get name?
New-ing a custom plugin to just get its name breaks error sanity checks in my plugin's constructor to make sure required options are not null.

Are there any problems with it being
pluginName = PluginRefrence.name

If so, I can create a pull request.

@evantahler
Copy link
Member

I believe that case exists if the plugin is actually a class.
By all means, if you can get it to work without instantiation, go for it! Please include a test to ensure that it's working

@jkhaak
Copy link

jkhaak commented Jun 3, 2019

I was wondering the same thing earlier. It turned out, that the name cannot be resolved reliably unless there is an instance of the plugin itself. The name attribute is computed in the object constructor.

// [...snip...]
class Plugin {
  constructor (worker, func, queue, job, args, options) {
    this.name = 'CustomPlugin' // <-- defining default name for the custom plugin
    this.worker = worker
    this.queue = queue
    this.func = func
    this.job = job
    this.args = args
    this.options = options
  }
// [...snip...]
}

Also the deriving classes should override the name attribute in the following way (copied the example from examples/customPluginExample.js):

class MyPlugin extends NodeResque.Plugin {
  constructor (...args) {
    super(...args)
    this.name = 'MyPlugin' // defining name of the plugin
  }

  beforePerform () {
    console.log(this.options.messagePrefix + ' | ' + JSON.stringify(this.args))
    return true
  }
}

Do note this problem exists only with reference type plugins.

NB the example doesn't have the constructor defined which in turn breaks the options object. I added the constructor here for completeness.

Solution could be to create a static getter function for the plugin class. Then the name could be resolved without instantiating the class. For example:

class MyPlugin extends NodeResque.Plugin {
  static get name () {
    return 'MyPlugin'
  }
}

However, this kind of change would break the current plugin API.

@glensc
Copy link
Contributor

glensc commented Apr 13, 2020

perhaps it could be added as optional interface (if method name exists, use it)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants