feat(vowTools): add asVow helper #9577
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mhofman
reviewed
Jun 24, 2024
packages/vow/src/vow-utils.js
Outdated
Comment on lines
88
to
99
| const kit = makeVowKit(); | ||
| try { | ||
| kit.resolver.resolve(fn()); | ||
| } catch (e) { | ||
| kit.resolver.reject(e); | ||
| } | ||
| return kit.vow; |
There was a problem hiding this comment.
I think we could simply support not resolving a vow to a vow with the following:
Suggested change
| const kit = makeVowKit(); | |
| try { | |
| kit.resolver.resolve(fn()); | |
| } catch (e) { | |
| kit.resolver.reject(e); | |
| } | |
| return kit.vow; | |
| let result; | |
| try { | |
| result = fn(); | |
| } catch (e) { | |
| result = Promise.reject(e); | |
| } | |
| if (isVow(result)) { | |
| return result; | |
| } | |
| const kit = makeVowKit(); | |
| kit.resolver.resolve(result); | |
| return kit.vow; |
Deploying agoric-sdk with
|
| Latest commit: |
0cdcd5f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d8e8131b.agoric-sdk.pages.dev |
| Branch Preview URL: | https://pc-as-vow.agoric-sdk.pages.dev |
mhofman
approved these changes
Jun 25, 2024
packages/vow/test/asVow.test.js
Outdated
| const testVow = watch(Promise.resolve('payload')); | ||
| const testVowAsVow = asVow(() => testVow); | ||
|
|
||
| t.is(testVow, testVowAsVow, 'asVow does not rewrap vows'); |
There was a problem hiding this comment.
So technically we should do a "passable compare" here. The tagged vow could be a different object, but the nested payload must contain the same remotable. Not sure if we have a common way to do such passable comparisons.
There was a problem hiding this comment.
The closest thing I'm aware of is getInterfaceOf, which doesn't quite seem to fit the bill for a "passable compare".
In this testing context, where there's no disconnection, this was a cheap way to verify the behavior. I added a comment with this feedback and context above this line.
- adds asVow() helper function that coerces the result of a function to a Vow - see comment from @mhofman for inspiration: #9454 (comment)
gibson042
pushed a commit
that referenced
this pull request
Jul 2, 2024
gibson042
added a commit
that referenced
this pull request
Jul 2, 2024
git rebase-todo ``` # Branch pc/as-vow label base-pc-as-vow pick b6b5f5f feat(vowTools): add asVow helper pick 0cdcd5f feat(vowTools): asVow should not wrap a vow as a vow label pc-as-vow reset base-pc-as-vow merge -C 0ad10c6 pc-as-vow # feat(vowTools): add asVow helper (#9577) # Branch pc/watch-utils-as-promise label base-pc-watch-utils-as-promise pick bf430a1 feat(watchUtils): add asPromise helper # To resolve conflicts in the following commit: # * In packages/vow/src/types.js, keep inbound changes and (after that) the # definition of VowTools. # * In packages/vow/test/watch-utils.test.js, keep only inbound changes and # limit them to start at "asPromise converts a vow to a promise" (i.e., # just the additions of # https://github.com/Agoric/agoric-sdk/pull/9620/files#diff-fd7cabcc7d1097036e2981c3262aa20612bec349f6e418d53324fc33b1b26946 ). pick c940d5c feat(vowTools): asPromise helper for unwrapping vows pick 8c27c67 feat(watchUtils): handle non-storables pick 3d5a3f3 feat(types): EVow pick 1c0b964 refactor: don't re-use index pick 274df18 fix(vow): clearer stored/non-stored values pick ff92211 refactor: 'extra' field for future properties label pc-watch-utils-as-promise reset base-pc-watch-utils-as-promise merge -C 8edf902 pc-watch-utils-as-promise # feat(vows): improve handling of ephemeral values (#9620) ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
refs: #9449
Description
asVowhelper toVowToolsto ensure we always return vows, even in the event of early synchronous errors.ChainAccountKitagainst feat: ChainAccountKit returns vows #9562