adopt EReturn type #9011

turadg · 2024-02-28T23:23:22Z

Description

While working on another PR, I had to type out ReturnType<Awaited<ReturnType< again and I wished for a utility type.

This PR first made an ExoObj type but has changed to use EReturn now available in Endo.

EReturn type endojs/endo#2659

Security Considerations

n/a

Scaling Considerations

n/a

Documentation Considerations

Not necessary

Testing Considerations

CI

Upgrade Considerations

n/a

erights

Reluctantly LGTM. Please read the comment about other steps that should also happen, if the assumption in that comment applies. If it does not apply --- if ExoObj really would only be of interest starting at agoric-sdk --- then nevermind and this PR unreservedly LGTM.

Would be nice if the doccomment also made clear which of these cases applies.

erights · 2024-02-28T23:41:33Z

packages/internal/src/types.d.ts

@@ -18,3 +18,5 @@ export declare class SyncCallback<

  public isSync: true;
 }
+
+export type ExoObj<T> = Awaited<ReturnType<Awaited<ReturnType<T>>>>;


It is not obvious to me why this pattern repeatedly appears, or at least slight simpler ones subsumed by this one. Please add a doccomment explaining what this type means and when it should be used.

Is that reason somehow specific to agoric-sdk, or would it apply and be useful to other users of @endo/exo? If so,

I'm sure you already know my feelings about introducing it this way to agoric-sdk.

But I understand not wanting to wait for another round of endo release and agoric-sdk sync. Attn @kriskowal @ivanlei

Please add a TODO at least here, that we expect this type to migrate to @endo/exo

Please do an equivalent PR for @endo/exo, assuming that's the right place for ExoObj to eventually end up. Or at least file an issue with the need to do that and assign it to me.

What has me at a loss is that we cannot abstract away the location that ExoObj is imported from without loss of info in the IDE. Instead, each use location mentions @agoric/internal, all of which will need to change once we've completed the deprecation dance. I don't know what to suggest about that, but if you have any clever ideas, please let me know.

I should also mention that since I don't yet understand why this pattern repeats or when it should be used, I don't yet have an opinion of whether ExoObj is a good name.

The name ExoObj does need more thought and I hope it’s not necessary to name. This is, I believe, an auto-unwrapping problem and @michaelfig has been very clever with those kinds of problems.

@kriskowal when you say "not necessary to name" do you mean some other way to get VaultDirector from prepareVaultDirector? I'm having a hard time imagining how that's possible without a utility type. Codegen could, but that's unnecessarily complex.

I withdraw my hope that we can avoid the need for a utility. For ExoObj<T>, what is T generally that requires this nested Awaited and ReturnType unboxing? Is this utility recursive or limited to a depth of two? Will there be a need for three levels? My hope is was that T was always an Exo (thus the name) and that we might change something about T to obviate the need for a utility.

I looked, and the actual case is subtler than class vs instance. If it were class vs instance, we'd only need ReturnType<ReturnsType< without the Awaited<s. Rather, the direct class creation is wrapped with a function that defines one class per call to the function and returns some function making function, like the maker function returned by the class definition. If this wrapping function needs to await anything in its logic around the class creation, then the wrapping function is made async and it returns a promise for the instance making function.

If the instance making function is the one returned by the class definition, fine. But it itself might be a wrapper around the base instance making function. It still returns one instance each time it is called. But if any of the other things it does needs to await anything, then it returns a promise for the instance instead.

That accounts for both Awaited<s. When there are no promises in the way, the extra Awaited<s are harmless.

@turadg , is that right?

More than 57bce86 ?

Yes please. There's a lot going on here!

I haven't given thought to makeExo which makes a usable obj. I could make this type conditional on whether the param type is a callable function.

Please don't fold makeExo into the same case. It isn't and should not look like it is.

makeExo is at the same level as the second phase, of an instance making function that makes an instance each time it is called. For makeExo itself, we would only need ReturnType<. But the instance making function might be a wrapper around makeExo that does some other stuff around the call to makeExo. If any of that might need to await something, then the wrapping function will be async and we'd need the Awaited<ReturnType<. Does that make sense?

Does that make sense?

What you said in that paragraph matches my understanding, but I don't see how it supports the previous statement,

Please don't fold makeExo into the same case. It isn't and should not look like it is.

I meant the result of a makeExo call, just as the others are results of defineExo calls. E.g.,

agoric-sdk/packages/governance/src/question.js

Lines 82 to 100 in fd864a4

/** @type {BuildQuestion} */

const buildQuestion = (questionSpec, counterInstance) => {

const questionHandle = makeHandle('Question');

/** @type {Question} */

return makeExo('question details', QuestionI, {

getVoteCounter() {

return counterInstance;

},

getDetails() {

return harden({

...questionSpec,

questionHandle,

counterInstance,

});

},

});

};

So supporting,

/** @typedef {ExoObj<typeof buildQuestion>`} Question

That has only one level of extracting the return type, vs two with the Exo class case.

Are you ok with the ExoObj supporting both levels? WDYT of Michael's idea to have the fully general EReturn?

kriskowal

Assuming that @agoric/internal is not de facto external, putting ExoObj there means we can punt the question of a good name, but I would encourage having the naming and documentation conversation early. The name should communicate why it exists and when you would use it in opposition to the alternatives.

The name shouldn’t include the Obj abbreviation. Having to guess whether an abbreviation in play makes all names effectively unguessable and at least harder to memorize.

Can you take a pass at documenting ExoObj?

packages/inter-protocol/src/reserve/assetReserveKit.js

kriskowal · 2024-02-28T23:59:47Z

packages/internal/src/types.d.ts

@@ -18,3 +18,5 @@ export declare class SyncCallback<

  public isSync: true;
 }
+
+export type ExoObj<T> = Awaited<ReturnType<Awaited<ReturnType<T>>>>;


The name ExoObj does need more thought and I hope it’s not necessary to name. This is, I believe, an auto-unwrapping problem and @michaelfig has been very clever with those kinds of problems.

michaelfig · 2024-03-03T20:46:23Z

packages/internal/src/types.d.ts

+  /**
+   * Utility for the type of an Exo object once prepared and instantiated.
+   */
+  type ExoObj<T> = Awaited<ReturnType<Awaited<ReturnType<T>>>>;


With a little recursion, this becomes useful not just for Exos:

Suggested change

/**

* Utility for the type of an Exo object once prepared and instantiated.

*/

type ExoObj<T> = Awaited<ReturnType<Awaited<ReturnType<T>>>>;

/**

* Find the eventual return type of a specimen, recursing to function

* return values and promise fulfilments.

*/

type EReturn<T> = T extends (...args: any[]) => infer R ? EReturn<R> :

T extends PromiseLike<infer U> ? EReturn<U> :

T;

That would be more general, yes. I think one for Exos has some didactic value so even if we had this I'd suggest having both.

What use cases do you see for a general EReturn?

On second thought, since having EReturn would cover all the cases I don't think it's worth naming another type.

@kriskowal would you be ok with an EReturn type in Endo? You expressed some concern earlier about recursive types.

const makeHi = () => Promise.resolve('hi' as const); type Hi = EReturn<typeof makeHi>; const hi = await makeHi(); expectType<'hi'>(hi); expectType<Hi>(hi);

Is this also the case?

const lol = async () => async () => async () => 'lol'; type LoL = EReturn<typeof lol>; expectType<'lol'>(LoL);

That would seem weird to me, recursively unwrapping both promise and return type. I would think it safer to be more specific than an arbitrary number of niladic applications.

Is it possible to have appropriate type extractors with names that resemble the corresponding constructors? I like having Return in the name. I recall @erights had a table with all the Exo constructor patterns named. Maybe that would help inform a family of return-type-of operator names.

That would be the case, yes.

I would think it safer to be more specific than an arbitrary number of niladic applications.

What hazard(s) are you defending against?

Is it possible to have appropriate type extractors with names that resemble the corresponding constructors?

Certainly. It's just more to write, maintain and require people to learn.

I would expect EReturn<T> to be shallow, so it could be used to eventually capture the next function signature. It would be useful for remote functions. Can suggest reserving and pitch EUltimateReturn<T>?

endojs/endo#2659

packages/internal/src/index.js

Lifting my foot.

incidental ## Description Migrated from Agoric/agoric-sdk#9011 Define an `EReturn` type for the `Awaited<ReturnType<` pattern. ### Security Considerations none ### Scaling Considerations none ### Documentation Considerations This will appear in TSdoc ### Testing Considerations has tests ### Compatibility Considerations none ### Upgrade Considerations none

cloudflare-workers-and-pages · 2025-01-24T19:02:48Z

Deploying agoric-sdk with Cloudflare Pages

Latest commit:	`f442482`
Status:	✅ Deploy successful!
Preview URL:	https://57b08b3a.agoric-sdk.pages.dev
Branch Preview URL:	https://ta-exoobj.agoric-sdk.pages.dev

View logs

turadg requested review from erights and michaelfig February 28, 2024 23:23

turadg force-pushed the ta/ExoObj branch 2 times, most recently from 47867d0 to 42d2a7f Compare February 28, 2024 23:26

erights approved these changes Feb 28, 2024

View reviewed changes

kriskowal previously requested changes Feb 29, 2024

View reviewed changes

michaelfig reviewed Mar 3, 2024

View reviewed changes

packages/internal/src/index.js Outdated Show resolved Hide resolved

turadg mentioned this pull request Dec 11, 2024

EReturn type endojs/endo#2659

Merged

turadg added 3 commits January 24, 2025 10:58

chore(deps): bump endo/eventual-send

bac134c

refactor(types): adopt EReturn type

59ea1ca

chore(deps): bump endo/far

c1378b2

turadg force-pushed the ta/ExoObj branch from 57bce86 to c1378b2 Compare January 24, 2025 19:02

turadg requested a review from a team as a code owner January 24, 2025 19:02

turadg requested a review from AgoricTriage January 24, 2025 19:02

turadg changed the title ~~feat(types): ExoObj utility~~ adopt EReturn type Jan 24, 2025

turadg marked this pull request as draft January 24, 2025 19:06

fixup! chore(deps): bump endo/eventual-send

f442482

turadg marked this pull request as ready for review January 26, 2025 18:06

turadg requested review from michaelfig and kriskowal January 26, 2025 18:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adopt EReturn type #9011

adopt EReturn type #9011

turadg commented Feb 28, 2024 •

edited

Loading

erights left a comment

erights Feb 28, 2024

erights Feb 28, 2024

kriskowal Feb 28, 2024

turadg Feb 29, 2024

kriskowal Feb 29, 2024

erights Feb 29, 2024

erights Feb 29, 2024

erights Feb 29, 2024

erights Feb 29, 2024 •

edited

Loading

turadg Mar 4, 2024

kriskowal left a comment

kriskowal Feb 28, 2024

michaelfig Mar 3, 2024

turadg Mar 4, 2024

turadg Mar 18, 2024

kriskowal Mar 22, 2024

turadg Mar 22, 2024

kriskowal Mar 22, 2024

turadg Dec 11, 2024

cloudflare-workers-and-pages bot commented Jan 24, 2025 •

edited

Loading

	/** @type {BuildQuestion} */
	const buildQuestion = (questionSpec, counterInstance) => {
	const questionHandle = makeHandle('Question');

	/** @type {Question} */
	return makeExo('question details', QuestionI, {
	getVoteCounter() {
	return counterInstance;
	},
	getDetails() {
	return harden({
	...questionSpec,
	questionHandle,
	counterInstance,
	});
	},
	});
	};

-  /**
-   * Utility for the type of an Exo object once prepared and instantiated.
-   */
-  type ExoObj<T> = Awaited<ReturnType<Awaited<ReturnType<T>>>>;
+  /**
+   * Find the eventual return type of a specimen, recursing to function
+   * return values and promise fulfilments.
+   */
+  type EReturn<T> = T extends (...args: any[]) => infer R ? EReturn<R> :
+    T extends PromiseLike<infer U> ? EReturn<U> :
+    T;

adopt EReturn type #9011

Are you sure you want to change the base?

adopt EReturn type #9011

Conversation

turadg commented Feb 28, 2024 • edited Loading

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

erights left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erights Feb 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kriskowal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cloudflare-workers-and-pages bot commented Jan 24, 2025 • edited Loading

Deploying agoric-sdk with Cloudflare Pages

turadg commented Feb 28, 2024 •

edited

Loading

erights Feb 29, 2024 •

edited

Loading

cloudflare-workers-and-pages bot commented Jan 24, 2025 •

edited

Loading