Skip to content

Conversation

@rekmarks
Copy link
Member

@rekmarks rekmarks commented Sep 24, 2024

The command / message types have frustrated us on various occasion (exhibit A), and this is in part because they blur abstraction boundaries between the kernel and the platform it runs on. Here we attempt to delineate this abstraction boundary by extracting these types into a new "kitchen drawer" package (@ocap/utils), and refactoring them such that only a smaller set needs to be shared between different packages. In addition, the Command-family of types and enums receives an overhaul.

Changes in detail:

  • Extracts use case-specific types and utilities from @ocap/streams into a new package, @ocap/utils
  • Extract single-package types into their respective packages from @ocap/utils
  • Rename command types, see utils/src/types.ts
    • The Command type itself blurs our ideal abstraction boundary by implementing low-level plumbing like ping alongside porcelain such as `callCapTp. Fixing this is deferred to future work.
    • Notably, the string enum of command names is now named CommandMethod, and the actual command payload Command. Various unnecessary generic parameters have been removed.

Commits are individually reviewable.

@rekmarks rekmarks marked this pull request as ready for review September 24, 2024 01:18
@rekmarks rekmarks requested a review from a team as a code owner September 24, 2024 01:18
@rekmarks rekmarks requested a review from FUDCo September 24, 2024 01:18
Comment on lines 28 to 58
const envelopeKit: ReturnType<
typeof makeStreamEnvelopeKit<
typeof envelopeLabels,
{
command: VatMessage;
capTp: CapTpMessage;
}
>
> = makeStreamEnvelopeKit<
typeof envelopeLabels,
{
command: VatMessage;
capTp: CapTpMessage;
}
>({
command: isVatMessage,
capTp: isCapTpMessage,
});

export type StreamEnvelope = GuardType<typeof envelopeKit.isStreamEnvelope>;
export type StreamEnvelopeHandler = ReturnType<
typeof envelopeKit.makeStreamEnvelopeHandler
>;

export const wrapStreamCommand: typeof envelopeKit.streamEnveloper.command.wrap =
envelopeKit.streamEnveloper.command.wrap;
export const wrapCapTp: typeof envelopeKit.streamEnveloper.capTp.wrap =
envelopeKit.streamEnveloper.capTp.wrap;
// eslint-disable-next-line prefer-destructuring
export const makeStreamEnvelopeHandler: typeof envelopeKit.makeStreamEnvelopeHandler =
envelopeKit.makeStreamEnvelopeHandler;
Copy link
Member Author

@rekmarks rekmarks Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I ran into what appears to be a cursed TypeScript bug. It doesn't believe that these types can be inferred, and instead require explicit annotation. I think we can leave it be for now as I have hopes that we can simplify this situation later.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #86. I think this is what happens when you don't export the "seminal types" @SMotaal mentioned here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hehe, good branch name.

Copy link
Member Author

@rekmarks rekmarks Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were a number of cursed bugs that surfaced as the error I got for this, but they appear to have been resolved, and this appears to have been legitimate (ref: microsoft/TypeScript#42873 (comment)).

grypez and others added 3 commits September 23, 2024 22:09
Apparently typescript needs these types explicitly exported in order to
do inference in other packages.
Copy link
Contributor

@grypez grypez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good step toward figuring out where things ought to land.

The stream envelope kit enforces compile and runtime constraints on the types of messages which can pass through the stream in either direction. I expect that eventually we will have a thing-stream-envelope.ts declaration for every thing with a distinctly typed stream, but let the stream-envelope.ts here serve as a prototype.

Co-authored-by: grypez <143971198+grypez@users.noreply.github.com>
Copy link
Contributor

@grypez grypez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@rekmarks rekmarks merged commit b6b23ce into main Sep 24, 2024
@rekmarks rekmarks deleted the rekm/utils-package branch September 24, 2024 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants