feat: support not to auto check update & manual check update

[renhaoyeh]

Implemented the issue #164

• autoCheck - which defaults to true. It can be set to false to disable automatic update checks.
• checkForUpdates() - manually check for updates.

However, I did not implement the final installUpdate function
Because the correct installation process should be executed after the file has been downloaded

autoUpdater.once('update-downloaded', () => {
  autoUpdater.quitAndInstall();
});

[renhaoyeh]

Friendly ping! Any feedback on this?

[erickzhao]

@rhy3h Hey! Haven't gotten around to reviewing it yet, thanks for the ping. Hopefully can get that done for you in the next week or two?

[renhaoyeh]

@erickzhao Hey! No worries at all, I really appreciate you taking the time. I just started contributing to PRs recently, so I’d love to know if I’m doing things correctly. No rush at all!

[renhaoyeh]

Just checking in again—no rush at all, and really appreciate your time. Would love any feedback whenever you get a chance!

[dariuszjastrzebski]

@erickzhao have you had chance to check this PR? This changes gain greater control over when and how updates are managed in the application. I would be grateful for your feedback

[renhaoyeh]

FYI
I just find out linux do not support auto update
so I skip the test when in linux platform

[erickzhao]

Suggested change

	/**
	* @param {Boolean} autoCheck Decides whether to automatically check for updates
	* Defaults to `true`.
	*/
	/**
	* @param {Boolean} autoCheck Decides whether to automatically check for updates
	* @default true
	*/

[renhaoyeh]

In other comments

update-electron-app/src/index.ts

Lines 98 to 107 in c5f1d43

	/**
	* @param {Object} logger A custom logger object that defines a `log` function.
	* Defaults to `console`. See electron-log, a module
	* that aggregates logs from main and renderer processes into a single file.
	*/
	readonly logger?: L;
	/**
	* @param {Boolean} notifyUser Defaults to `true`. When enabled the user will be
	* prompted to apply the update immediately after download.
	*/

those writing style uses indentation
so I do that too
which one should I follow

[erickzhao]

Suggested change

	if (autoCheck) {
	if (autoCheck === true) {

I wonder if we want to be stricter about actually passing true rather than something truthy.

[renhaoyeh]

update-electron-app/src/index.ts

Line 216 in c5f1d43

if (opts.notifyUser) {

I follow this, so I didn't do strict equal
I agree with you, it might need stricter

[erickzhao]

We should add a JSDoc comment for the new manual checkForUpdates function as well.

[renhaoyeh]

https://github.com/rhy3h/update-electron-app/commit/701979399a780de42665774432cd19145c781ad4
please see this commit

[MarshallOfSound]

Blocking comment on IMO bad api surface

[renhaoyeh]

Friendly ping!

[SoulKa]

How about just setting the updateInterval <= 0 to disable auto updates? This would omit the need for a new property. @MarshallOfSound is that what you meant with bad API?

We've just introduced your auto updater module in our application and I would very much welcome more control over the update process as suggested in this PR since we're trying to be gentle on the internet usage. Our current solution is just to set a very high interval, but that's a hack in my opinion.

[peppescg]

Hey guys, I am really interesting to this PR, cause I would like to stop the checkForUpdates as well ( my 2c cause updateInterval is expecting a string || undefined we can use empty string in order to skip the checks)

---

I have this weird behavior, looking to the logs I saw an issue where the update (triggered every 10m as default), is available but during the downloading I receive the update-not-available event.

[info]  [update] checking for updates...
[info]  update-available; downloading...
[info]  [update] update is available, starting download...
[info]  update-not-available

I am handling notifyUser: false use case, showing the modal and performing manually the dialog and the autoUpdater.quitAndInstall().

Do you have an idea about this use case?

[renhaoyeh]

Hi, just wondering if there’s anything else I should do to get this PR ready for merge.
Please let me know if any changes or updates are needed.
Thanks!