feat: add support for objects containing toPromise method #1998
Conversation
|
@limonte it's still under development. I need some guidance in the tests creation. Could you help me there? BTW, this is more or less the correct flux to implement this? |
sweetalert2.d.ts
Outdated
| @@ -337,6 +337,10 @@ declare module 'sweetalert2' { | |||
| const version: string | |||
| } | |||
|
|
|||
| interface Promisify<T> { | |||
| readonly toPromise: () => T; | |||
There was a problem hiding this comment.
We don't require the toPromise member to be readonly do we?
There was a problem hiding this comment.
In fact, readonly for this case doesn't matter at all.
sweetalert2.d.ts
Outdated
| @@ -387,7 +391,7 @@ declare module 'sweetalert2' { | |||
|
|
|||
| type Awaited<T> = T extends Promise<infer U> ? U : T; | |||
|
|
|||
| type SyncOrAsync<T> = T | Promise<T>; | |||
| type SyncOrAsync<T> = T | Promise<T> | Promisify<T>; | |||
There was a problem hiding this comment.
I think we can inline Promisify here, since it's only used once, and probably will only ever be used once. This makes it easier to see what SyncOrAsync is. This also saves us needing to name it.
src/utils/utils.js
Outdated
| export const isPromise = (arg) => arg && Promise.resolve(arg) === arg | ||
|
|
||
| export const promisify = (arg) => hasToPromiseFn(arg) ? arg.toPromise() : Promise.resolve(arg) |
There was a problem hiding this comment.
The verb "promisify" has been used a lot to describe converting a callback-based API to a Promise-based API. For example, NodeJS's util.promisify, Bluebird's Promise.promisify, pify, and more. I suggest renaming to something else. Ideas: toPromise or asPromise.
There was a problem hiding this comment.
I've updated it to asPromise.
@rafaelss95 What in particular do you need guidance on? Are you familiar with automated testing in general? Were you able to run the tests locally? Maybe you need help choosing which tests to update?
Not totally sure what you mean by correct "flux".. Can you clarify? Or maybe this doesn't concern me. |
Yep, I wasn't sure which tests should be updated. I've checked the https://github.com/sweetalert2/sweetalert2#contributing and it doesn't contain anything about the tests.
As it's my first contribution to this repository, in addition to |
# [9.15.0](v9.14.4...v9.15.0) (2020-06-13) ### Features * add support for objects containing toPromise method ([#1998](#1998)) ([0d33441](0d33441))
|
🎉 This PR is included in version 9.15.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
# [9.15.0](sweetalert2/sweetalert2@v9.14.4...v9.15.0) (2020-06-13) ### Features * add support for objects containing toPromise method ([sweetalert2#1998](sweetalert2#1998)) ([3360a33](sweetalert2@3360a33))
Closes #1982.