Closed SpadarShut closed 1 year ago
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'];
This way the function still cannot return a promise, so it seems the parentheses won't fix the issue.
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?
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
.
Thank you, got it! I just tried it out, and my previous suggestion https://github.com/ndresx/react-countdown/pull/222#issuecomment-1320875489 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
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.
And yes, your code fixes my issue.
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!
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.
Thanks again!
Recent changes have made it illegal for onComplete callback to be async. This should fix the issue.