Allow onComplete to be async

[SpadarShut]

Recent changes have made it illegal for onComplete callback to be async. This should fix the issue.

[coveralls]

Coverage remained the same at 100.0% when pulling 2c12017 on SpadarShut:patch-1 into 6df039c on ndresx:master.

[ndresx]

Hi @SpadarShut, thanks for the PR. I think the issue you are experiencing lies somewhere else - the return type is missing some parentheses and should look like this. It shouldn't have anything to do with async?

  readonly onComplete?:
    | ((timeDelta: CountdownTimeDelta, completedOnStart: boolean) => void)
    | LegacyCountdownProps['onComplete'];

[SpadarShut]

This way the function still cannot return a promise, so it seems the parentheses won't fix the issue.

[ndresx]

But it doesn't return a promise in the first place (it's not async, it's also not returning anything). What's your use case to maybe understand this a little better?

[SpadarShut]

My use case is that I want to do
onComplete={async () => {...}} but cannot do this now, because the types require the function to be void.

[ndresx]

Thank you, got it! I just tried it out, and my previous suggestion #222 (comment) seems to be fixing the problem. The issue right now is that while void can overlap with Promise<void> (as it was before), the accidentally added return type () => void cannot anymore.

I'm happy to merge your branch if you could make this change and ideally confirm that this would suit your needs, otherwise, I can also fix it myself, of course (let me know)!

Here's also an example of what's most likely happening: https://www.typescriptlang.org/play?target=99#code/MYewdgzgLgBODCIC2AHANgUyhmBeGAhhAJ5jAwAUAlHgHwwDeAvgFCiSwhoAmA8mIlSZsALkrU6MAG4gAltxr4EydFgwBuGAHotMAAoAnDFNkgArhGkYDEU5EogA1lTbhoMMBgDu-QatGUivQy8jAAPuJB0nIKeHACKsIa2roAct5WNnaUAGYEsmiyYADmVEA

[SpadarShut]

What's the trick with parentheses? Why does it work?

And I can see you've creaated another PR for the fix, so feel free to close this.

[SpadarShut]

And yes, your code fixes my issue.

[ndresx]

Thanks for confirming. I made a mistake the last time I adjusted the types for this callback, so without the parentheses, the typing looks like this, meaning the return type can be either void, or the typing for LegacyCountdownProps['onComplete'], which is a function on its own (() => void).

  readonly onComplete?: (...) => void | LegacyCountdownProps['onComplete']; // (...) => void | () => void

With parentheses:

  readonly onComplete?:
    | ((timeDelta: CountdownTimeDelta, completedOnStart: boolean) => void)
    | LegacyCountdownProps['onComplete'];
 // ((...) => void) | (() => void)

If you don't want to change it here anymore, I will close this PR; that's fine. I mainly created the other one because I'm also planning to update some example dependencies. And thank you again for reporting this problem!

[SpadarShut]

Thank you for the explanation and for the lib itself!

And I was just wondering why ((...) => void) is different from (...) => void and allows to return a promise.

Also if it helps to make a patch release faster, I've updated my PR.

[ndresx]

Thanks again!