Configurable polling timeout in watch.browser

[Messj1]

Hi

Nice project. Love the modular architecture. 👍

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. 😉

So in watch.browser.js the hard coded value should be replaced by a variable, maybe a function parameter.

ulog/mods/config/watch.browser.js

Line 18 in 2aa3419

}, 350)

Then it could be implemented as a setting parameter:

ulog/mods/config/index.js

Lines 28 to 29 in 2aa3419

	var settings = grab(this, 'settings', {})
	name = settings[name] && settings[name].config || name

BTW: There seems to be a bug 🐛 in the code. The interval get not stopped if someone call ulog.set("log_config", {});

ulog/mods/config/index.js

Lines 43 to 44 in 2aa3419

	set: function(name) {
	if (name === 'log_config') config.update(this)

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?

ulog/mods/config/watch.browser.js

Lines 11 to 16 in 2aa3419

	setTimeout(function(){
	var changed = update(ulog.config, cfg)
	if (changed.length) setTimeout(function(){
	notify(ulog, changed)
	}, 0)
	}, 0)

[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 🤨