-
Notifications
You must be signed in to change notification settings - Fork 37
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: Added support for types in overlay.openAsync #88
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Hi! Thank you for your interest in contributing to overlay-kit. This looks good to me overall. One concern I have is that allowing arguments for I'll take some time to think this over a bit more. |
I also agree that allowing unmount to resolve promises can cause confusion. If so, how about thinking about this feature a little later? I think I could suggest changing the unmount behavior so that it cannot resolve as before. |
I will close this PR. Thanks! @jungpaeng |
Description
The existing overlay.openAsync must always respect generic passing and close(param:T).
This creates unnecessary syntax even in situations where you just need to await.
and I also wanted to provide a way to resolve for use cases that only use unmount, so I created this API.
One thing to note about this usage is that the Promise is settled with the value at the time of the first call.
That is, in this use case, the resolve value is:
Changes
To improve this and provide a little better usability I modified the type some more. and also implemented an unmount function for openAsync.
How Has This Been Tested?
Testing for type changes was performed on major changes.
I wrote test code to test the added behavior.
Types of changes
Checklist
Further Comments
Internal types have become more complex to provide more advanced type support.
Because typescript considers the false type to be expandable on void, we had to use the ternary operator twice.
Is there a better way?
Thank you!