Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notification is only displayed at most once per interval #209

Open
slorber opened this issue Jul 1, 2021 · 3 comments
Open

Notification is only displayed at most once per interval #209

slorber opened this issue Jul 1, 2021 · 3 comments

Comments

@slorber
Copy link

slorber commented Jul 1, 2021

As of 5.1.0, notifications are displayed at most once per interval period, because the fetched update is deleted once "emitted" (ie set to this.update)

		this.update = this.config.get('update');

		if (this.update) {
			// Use the real latest version instead of the cached one
			this.update.current = this.packageVersion;

			// Clear cached information
			this.config.delete('update');
		}

This is a bit weird to me, wouldn't it be better if the update message was displayed consistently as long as there is an update? Some users might miss that there is an available update quite easily.

Some workarounds I'm using to ensure that once an update gets fetched, it remains visible, and trying to fetch it before the first interval:

// Hacky way to ensure we check for updates on first run
// Note: the notification will only happen in the 2nd run anyway
if (
  !notifier.disabled &&
  Date.now() - notifier.config.get('lastUpdateCheck') < 50
) {
  notifier.config.set('lastUpdateCheck', 0);
  notifier.check();
}

if (notifier.update && notifier.update.current !== notifier.update.latest) {
  // Because notifier clears cached data after reading it, leading to notifier not consistently displaying the update
  notifier.config.set('update', notifier.update);

  // Display notification
}

Having a more convenient way to achieve all this could be useful

@sindresorhus
Copy link
Member

This behavior is intentional. When we made this package, we wanted it to be as unobtrusive as possible.

That being said, I'm open to having an option added that changes the behavior, but I still believe the current behavior is the best default.

@sindresorhus
Copy link
Member

This is a bit weird to me, wouldn't it be better if the update message was displayed consistently as long as there is an update?

What if the user cannot upgrade for some reason? Then they will be bothered by an update notification they every time they run your CLI.

@slorber
Copy link
Author

slorber commented Jul 1, 2021

Considering there's a way to disable the update-notifier already, they can just temporarily disable it.

I can also expose my own way to allow disabling this through our own config, so that it's clearly documented for my users. The notification may even link to that config option doc so that user can disable it very easily

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants