Callbacks API 2.0

[SachaG]

Currently:

runCallbacks(callbackName, iterator, arg1, arg2, arg3…);

Proposal:

runCallbacks({ 
  name,  // the name of the callback
  iterator,  // the object that will be iterated on
  properties, // arguments passed down to each iteration (i.e. { arg1, arg2, arg3… })
  options, // callback options such as sync/async/concurrent
});

[SachaG]

Ping @ochicf

[SachaG]

Oh also I think it makes sense to use the type name instead of the collection name:

movies.edit.async -> movie.update.async

[mattblackdev]

runCallbacks({
  // properties
  arguments, // [arg1, arg2] 

[SachaG]

@mattblackdev the reason I didn't use arguments is because it's a reserved word in JS.

[mattblackdev]

@SachaG 🙈

[x5engine]

@SachaG 🙈

[ochicf]

Hi! Sorry for the late response, I've been kind of busy this week and I wanted to do it properly.

I like the proposal of having a single object as parameter for runCallbacks. Some comments/questions:

• properties would be an object or an array? I'm guessing an array, since it fits better with how parameters in functions are handled.
• following on renaming properties, since arguments is reserved, maybe args, parameters or params would fit better? I think they better represent of what they actually are.
• other options could be spread directly to the runCallbacks options

So, the signature would look like this:

runCallbacks({ 
  name,  // the name of the callback
  iterator,  // the object that will be iterated on
  parameters, // or `args`, `params`. arguments passed down to each iteration (i.e. { arg1, arg2, arg3… })
  ...options, // callback options such as sync/async/concurrent
});

As for options, I'll refer to the last discussion we had about this topic, and more specific to my post in https://github.com/VulcanJS/Vulcan/issues/1949#issuecomment-389111037. Long story short, the options I think we have:

• order of execution, can be consecutive (as reducers, being executed one after the other and passing the result value) or concurrent (being executed without guaranteeing the order without any relation between the callbacks).
• syncronicity, referening to what kind of callbacks can be added to a given hook. Basically, when running callbacks syncronously it should only consider the syncronous results of added callbacks. This allows the runner of the hook to not break its sync context (as it may not be in their hand to do so). When running a hook in an async context, all callbacks are allowed.

An option would be to have different specific runners, being consecutive.sync, consecutive.async, concurrent the ones that cover the requirements exposed above.

runCallbacks({ 
  name,
  iterator,
  parameters, 
  runs: 'consecutive.sync|consecutive.async|concurrent', // defaults to 'consecutive.sync'?
  ...otherOptions, // if any
});

That gives me an idea to maybe offer a simple API to add custom runners? That way anyone could add their callback runner if they need to. I think it's kind of complex so I'm not going to enter this rabbit hole right now, but if you guys think it is a good idea tell me and I'll ellaborate.

About the signature of an added callback, I'm guessing we would keep the same as we have right now, being function addedCallback(iterator, ...parameters) { return iterator; }.

Tell me what you guys think!

[SachaG]

properties would be an object or an array? I'm guessing an array, since it fits better with how parameters in functions are handled.

An object I think, otherwise we'll just end up with the same backwards compatibility issues when something changes (i.e. the second argument was modifier but it's now data).

following on renaming properties, since arguments is reserved, maybe args, parameters or params would fit better? I think they better represent of what they actually are.

Good point. Maybe args then?

[ochicf]

So, the added callbacks would be like this then, function callbackName(iterator, argsAsObject) {}, right?

[SachaG]

Yes exactly, the callbacks would be the same as now except that all args would be "compressed" into the second argument.