-
Notifications
You must be signed in to change notification settings - Fork 4
Description
Let's take the set action:
require('chainy').require('log')
.addExtension('set', 'action', function(value, newValue){
return newValue
})
.create()
.set('blah')
.log() // blah
It needs to accept that value
argument, as there is no way to turn it off.
Unfortunately, the new args
taskOption added in v1.5.0 of Chainy, won't cover it, as:
Chainy.require('log')
.addExtension('set', {
type: 'action',
taskOptions: {
args: []
}
}, function(value, newValue){
return newValue
})
.create()
.set('blah')
.log() // null
Will output null
as no arguments would be received.
This is something that I was considering about when releasing v1.5.0, however, v1.5.0 is still and improvement and offers cool stuff, despite this oversight.
What we probably need to do, is add a new special argument, like Chainy.injectArgumentsAsArguments
, and Chainy.injectArgumentsAsArgument
, then change the default action args to [Chainy.injectChainDataAsArgument, Chainy.injectArgumentsAsArguments]
.
This should maintain a lot of backwards compat, introduce some power, and accomplish the use case. With args: []
being changed to args: [Chainy.injectArgumentsAsArguments]
instead.
The other option is to allow for args: false
to be a special case, which means don't apply the action default arguments, but still apply the task options default arguments.
Another approach, which is similar to the args: false
suggestion, which I initially coded before v1.5.0 but then dropped, was a new task
extension type, and a new chain.task(method, opts)
. That will be the same as the action extension, but without the argument requirement. However, after implementation it proved to be an anti-pattern to a sort, as the user should have control over the argument insertion for the most part, and could cause confusion for implementors about which type they want. However, considering v1.5.0 is out with new args handling, this could be worth reconsideration as the anti-pattern may be subverted by the new args handling.
Need to consider this and weigh it up.