[helpers.js] The centralized function that creates timers should not use createTimerWithArgument

[rkoshak]

With createTimerWithArgument becoming deprecated, we need to change to use an alternative.

@florian-h05, switching to using a function generator inside this helper function should be sufficient, right? Something like:

const createTimer = (when, func, arg, name, key) => {
  const timeout = time.toZDT(when);
  if (name === null || name === undefined) {
    if (global.ruleUID !== undefined) { // Use UI ruleUID and key if available
      name = 'ui.' + global.ruleUID + ((key !== undefined) ? '.' + key : '');
    } else if (global['javax.script.filename'] !== undefined) { // Use filename and key if available
      name = 'file.' + global['javax.script.filename'].replace(/^.*[\\/]/, '') + ((key !== undefined) ? '.' + key : '');
    }
  }
  return actions.ScriptExecution.createTimer(name, timeout, (arg) => { func(arg); } );
};

I'm not sure that mimics createTimerWithArgument sufficiently to be non-breaking though. It might impact some of the other library classes. Thankfully it shouldn't impact any users.

Maybe it makes sense to just push the function generators to the other classes and just support without argument here?

[florian-h05]

I am wondering whether we should even use a function generator, because when I look at how createTimerWithArgument works, it is actually affected by mutations:

var timeout = time.toZDT().plusSeconds(10);

var myVar = "Hello world";

actions.ScriptExecution.createTimerWithArgument(timeout, this, function (context) {
  console.log(context.myVar); // Logs Hello mutation
});

myVar = "Hello mutation";

This leads in fact to the same result as:

var timeout = time.toZDT().plusSeconds(10);

var myVar = "Hello world";

actions.ScriptExecution.createTimer(timeout, function () {
  console.log(myVar);
});

myVar = "Hello mutation";

Using a function generator, mutations do not affect the callback:

var timeout = time.toZDT().plusSeconds(10);

var myVar = "Hello world";

function funcGnr (msg) {
  return function () {
    console.log(msg)
  }
}

actions.ScriptExecution.createTimer(timeout, funcGnr(myVar));

myVar = "Hello mutation";

[rkoshak]

So we can either just use createTimer and drop the support for the passed in argument entirely, or use the function generator but document that the behavior will be different from createTimerWithArgument and that mutations do not affect the callback (which I thought was the whole point of createTimerWithArgument so now I wonder what's the point of it in the first place).

Though I'm wondering if this is a valid test. In the first example you are passing the whole context and I would expect myVar to be mutable in that case (pass by reference). What happens if you just pass myVar by itself? Is it still mutable or does it become fixed?

Anyway, this is all internal to the library so we can do what ever we want. We could even use setTimeout but I think that would have a bigger overall impact across the library.

[florian-h05]

I thought was the whole point of createTimerWithArgument so now I wonder what's the point of it in the first place

When I worked on a JS Scripting add-on PR a few weeks ago, I also had a conversation about this with jpg0. He thinks, that createTimerWithArgument is there because in Java/Rules DSL, there is not such a simple way of including state in callback functions as in JS, see https://github.com/openhab/openhab-addons/pull/13695#discussion_r1020755441.

What happens if you just pass myVar by itself? Is it still mutable or does it become fixed?

var timeout = time.toZDT ( ) . plusSeconds ( 10 ) ;
var myvar = "Hello world";
actions .ScriptExecution.createTimerwithArgument(timeout, myVar, function (theVar) {
console.log (theVar); / / Logs Hello mutation
};
myVar = "Hello mutation";

It logs Hello world, so it is fixed.

Anyway, this is all internal to the library so we can do what ever we want.

This is a good point for us, now we have to decide what we need here: a. fixed vars b. mutable vars

I would prefer to move the callback handling (in the means of function generator, yes or no?) to the consumers of the helper function to have the flexibility to choose in each case.

[rkoshak]

I would prefer to move the callback handling (in the means of function generator, yes or no?) to the consumers of the helper function to have the flexibility to choose in each case.

Me too. That's the most flexible. So we just need to remove arg from the function arguments, switch over to use createTimer() and update anything that was using arg for some reason.

[florian-h05]

Yes. Do you want to open a PR or should I open one?

[rkoshak]

Either way. I don't think there is a hurry on this. I'm not sure I'll have a chance to get to it before Monday though.

[florian-h05]

Okay, I'll see if I can open a PR this weekend.