default timeout not passed onto terminal-notifier

[clawconduce]

If timeout or wait are not provided, options does not have a timeout set after this line:

https://github.com/mikaelbr/node-notifier/blob/master/notifiers/notificationcenter.js#L72

So if no timeout is provided to terminal-app, the property doesn't exist, and doesn't get passed to terminal-notifier. If it's not set, terminal-notifier never closes. Ideally, if neither wait or timeout are passed in, the value should be set to 5 so it gets set.

I can make a PR, but I am not sure if I should make this change only for the notificationcenter? I can put it in mapToMac or just before calling utils.fileCommandJson, or maybe it should go somewhere so all the notifiers set the default?

[adrienharnay]

hHy, any news on this? Still having memory leaks using jest's watch mode

[mikaelbr]

Hey! I think there should be some sort of default timeout in any case as there's no sticky support. So providing a timeout always would be smart. But it shouldn't have memory leaks anyhow. The terminal-notifier app (used by the package) should eventually quit and clear all memory usage (and should't use that much memory anyways). Sounds like a bug there

[mikaelbr]

If adding timeout like @clawconduce suggest works, we could do that as a temporary workaround

[SimenB]

If a timeout is needed, a PR adding that to Jest is most welcome. However, sane defaults in this module probably makes more sense? (if I've understood the issue correctly)

[mikaelbr]

Hey, sorry, I was talking about a default in node-notifier, yeah. Or more specifically, terminal-notifier should have had built in safety making this a non-issue. But I think it is more feasible to build a workaround into this project.

[mikaelbr]

Added some default timeout now. Can anyone who has observed this issue test the master branch?

[kevinelliott]

I do not ever see my notifications wait even when wait: true is passed in and no timeout defined. If I define something like timeout: 100 I never see it wait 100 seconds.

[jordan-mcrae]

Timeout is also not working for me on OSX 10.14.6.

Edit: Quick fix for anyone running into the same issue...

There's an open PR for setting timeout to false to fix this issue. The wait config option did not work for me, but this open PR did.

You can specify the PR branch in your package.json if you're strapped and need this to work immediately.

package.json entry:
"node-notifier": "https://github.com/jnielson94/node-notifier.git#overwritable-timeout"

MVP Example config:

const Notifier = require('node-notifier');

Notifier.notify({
  title   : 'Hello',
  message : 'World',
  timeout : false,
});

Link to PR: #271

[jnielson94]

v6 was released a few hours ago and includes the default timeout again (and the capability to overwrite that default using false as the timeout option). I'm going to go ahead and close this issue, but feel free to comment here (or open a new issue) if there's still problems with the timeouts.

[Crazie-ash]

Is there any possibilities to change the default timeout from ~5secs to 100 secs....
I am not exactly sure about the correct configuration...
Any sample solutions, kindly let me know @jnielson94

Below is my sample:-

const Notifier = require('node-notifier');

Notifier.notify({
title : 'Hello',
message : 'World',
timeout : 100,
});

[jp185318]

I do not ever see my notifications wait even when wait: true is passed in and no timeout defined. If I define something like timeout: 100 I never see it wait 100 seconds.

hi, did u find a solution for this .