Configurable polling timeout in watch.browser

[Messj1]

Hi

Nice project. Love the modular architecture. :+1:

One thing i was missing was an configurable timeout. Would be nice in production mode to have bigger polling timeout than in development since it get then changed less often. :wink:

So in watch.browser.js the hard coded value should be replaced by a variable, maybe a function parameter. https://github.com/Download/ulog/blob/2aa34192c1804a143eaf0931fc196c712549a8f0/mods/config/watch.browser.js#L18

Then it could be implemented as a setting parameter: https://github.com/Download/ulog/blob/2aa34192c1804a143eaf0931fc196c712549a8f0/mods/config/index.js#L28-L29

BTW: There seems to be a bug :bug: in the code. The interval get not stopped if someone call ulog.set("log_config", {}); https://github.com/Download/ulog/blob/2aa34192c1804a143eaf0931fc196c712549a8f0/mods/config/index.js#L43-L44

Maybe a destructor callback would do the trick. At the end, watch.browser.js would look something like that:

module.exports = function(ulog, pollingTimeout) {
  const intervalId = window.setInterval(watchFnc, pollingTimeout);
  return () => {
      window.clearInterval(intervalId);
  }
}

[Download]

Hi @Messj1 Yes a configurable timeout is definitely a good idea! Thanks for your report! It sounds like you already have a pretty good idea of the code changes you wanr ro make. Would you like to try to implement this? I can help you of course if you have questions.

[Messj1]

@Download I'm not sure why there are two nested setTimeout in the interval function. Is there a reason to use asynchronous function call?

https://github.com/Download/ulog/blob/2aa34192c1804a143eaf0931fc196c712549a8f0/mods/config/watch.browser.js#L11-L16

[Download]

Ah yes. This is not pure, but there is an issue with the notify sometimes taking too long, so I tried to split up the work. Chrome complains sometimes about setTimeout handler taking too long to complete. This happens when it hovers around the 50ms mark.

[Messj1]

@Download I think this doesn't work since it seems to be a problem with the ext core function in notify :raised_eyebrow: