-
Notifications
You must be signed in to change notification settings - Fork 68
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
chore: Update Promise Kit README #2317
Conversation
@@ -1,3 +1,71 @@ | |||
# Promise Kit | |||
|
|||
A helper for making promises | |||
The promise-kit package provides a simple abstraction for creating a promise and its corresponding resolve and reject functions, allowing for easier promise management. It exports, `makePromiseKit` which is a utility function used to create a Promise and its associated resolver and rejector functions. This is particularly useful in asynchronous programming, where you might need to create a promise and resolve or reject it at a later point in time. | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mhofman Would you contribute a word about why you would use this instead of Promise.withResolvers
this year?
It may be worth stating up front that this is a “ponyfill” for Promise.withResolvers
that makes certain accommodations to ensure that the resulting promises can pipeline messages through @endo/eventual-send
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kriskowal I can append this text at the end of above if that is desirable:
“ponyfill” for Promise.withResolvers that makes certain accommodations to ensure that the resulting promises can pipeline messages through @endo/eventual-send.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good, thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's actually more than this. It also fixes resolver leaks in common implementations. Once withResolvers
get introduced later this year, we should consider making this a full shim (this package is already a shim for race
which is similarly leaky)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah it also creates HandledPromise
s, however I admit I still get confused about the layering there, @michaelfig might be more helpful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about this, a repaired HandledPromise.withResolvers
would do what makePromiseKit
does since it defers to the this
to decide what the constructor is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HandledPromise
is an augmentation of Promise
that supports eventual operations. It does not rely on any other packages, but the @endo/eventual-send
package provides a shim for it in addition to the makeE
eventual send operator maker. I'm not yet certain where makeE
actually belongs; I'd like to have just one version of it that (like the version in @agoric/vow/src/E.js
) can support a higher-than-eventual-send layer's implementation of pseudo-promises.
makePromiseKit
uses HandledPromise
if available; otherwise just Promise
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, should I add the following or is there a more accurate statement that needs to be added? Or leave it as is?
This is a “ponyfill” for Promise.withResolvers that makes certain accommodations to ensure that the resulting promises can pipeline messages through @endo/eventual-send.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this verbiage is fine and we can refine in follow-up. Please merge at your leisure.
92b8ac7
to
23e6b4b
Compare
Update Promise Kit README
fixes #2318