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

feat: add restartDelay option #68

Merged

Conversation

BangDori
Copy link
Contributor

@BangDori BangDori commented Mar 13, 2025

Why

The restartAfterInstall option in OTA updates had a hardcoded timeout of 300ms before restarting the app.
This made it feel like the app restarted too quickly, preventing a smooth transition.

How

To improve flexibility, restartDelay is set by the developer.

Related Issue

[AS-IS]

setTimeout(() => {
  resetApp();
}, 300);

[TO-BE]

setTimeout(() => {
  resetApp();
}, option?.restartDelay ?? 300);
default user-defined

left - default (300ms) / right - user-defined (3000ms)

- Declared as optional, consistent with other types in UpdateOption
- Added comments for bettwr clarity
- Applied `??` operator to set default(=300ms) for `undefined` or `null`
- Ensures restartDelay applies the user-defined value when provided
- Enforces type safety: only `number` values are allowed, preventing invalid types
src/index.tsx Outdated
@@ -154,7 +154,7 @@ async function downloadBundleUri(
if (option?.restartAfterInstall) {
setTimeout(() => {
resetApp();
}, 300);
}, option?.restartDelay ?? 300);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it better when use || instead of ??

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I compared the ?? and || operators, and it seems that || is more appropriate as it covers all falsy values. I'll apply the change right away! Thanks! 😊

* Delay in milliseconds before restarting the app after installing the update.
* Default: 300ms.
*/
restartDelay?: number;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It great if you can help me update at readme also

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! I'll add it right away!

@BangDori BangDori changed the title feat: add restartDelay configurable with nullish coalescing feat: add restartDelay option Mar 14, 2025
@vantuan88291
Copy link
Owner

all good, thank for your PR

@vantuan88291 vantuan88291 merged commit 1f13f90 into vantuan88291:main Mar 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants