refactor(rpc-methods): Improve type safety and overhaul types

Author: rekmarks

eslint.config.mjs

@@ -129,6 +129,13 @@ const config = createConfig([
     },
   },
 
+  {
+    files: ['**/test/**/*', '**/*.test.ts', '**/*.test.tsx'],
+    rules: {
+      '@typescript-eslint/explicit-function-return-type': 'off',
+    },
+  },
+
   {
     files: ['**/*.types.test.ts'],
     rules: {

packages/cli/test/bundles.ts

@@ -11,7 +11,6 @@ export const invalidTestBundleNames = ['bad-vat.fails'];
 
 const testRoot = new URL('.', import.meta.url).pathname;
 
-// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
 const makeTestBundleRoot = async () => {
   const stageRoot = resolve(tmpdir(), 'test');
 
@@ -32,7 +31,6 @@ const makeTestBundleRoot = async () => {
   return stageBundleRoot;
 };
 
-// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
 export const makeTestBundleStage = async () => {
   const stageBundleRoot = await makeTestBundleRoot();
 
@@ -44,7 +42,6 @@ export const makeTestBundleStage = async () => {
     return join(stageBundleRoot, `${bundleName}.js`);
   };
 
-  // eslint-disable-next-line @typescript-eslint/explicit-function-return-type
   const getTestBundleSpecs = (testBundleNames: string[]) =>
     testBundleNames.map((bundleName) => ({
       name: bundleName,

packages/cli/test/integration/serve.test.ts

@@ -53,7 +53,6 @@ describe('serve', async () => {
   });
 
   describe('server', () => {
-    // eslint-disable-next-line @typescript-eslint/explicit-function-return-type
     const makeServer = (root: string = testBundleRoot) => {
       const port = getServerPort();
       const { listen } = getServer({

packages/create-package/src/cli.test.ts

@@ -74,9 +74,7 @@ describe('create-package/cli', () => {
         tsConfig: {},
         tsConfigBuild: {},
         nodeVersions: '>=18.0.0',
-        // TODO: Replace `any` with type
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-      } as any);
+      } as utils.MonorepoFileData);
       vi.spyOn(utils, 'finalizeAndWriteData').mockResolvedValue();
 
       expect(
@@ -100,9 +98,7 @@ describe('create-package/cli', () => {
         tsConfig: {},
         tsConfigBuild: {},
         nodeVersions: '>=18.0.0',
-        // TODO: Replace `any` with type
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-      } as any);
+      } as utils.MonorepoFileData);
       vi.spyOn(utils, 'finalizeAndWriteData').mockResolvedValue();
 
       expect(

packages/extension/src/kernel-integration/ui-connections.test.ts

@@ -85,7 +85,6 @@ class MockBroadcastChannel {
 
 vi.stubGlobal('BroadcastChannel', MockBroadcastChannel);
 
-// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
 const makeMockLogger = () =>
   ({
     ...console,

packages/kernel-test/src/supervisor.test.ts

@@ -14,7 +14,6 @@ const makeVatSupervisor = async ({
 }: {
   handleWrite?: (input: unknown) => void | Promise<void>;
   vatPowers?: Record<string, unknown>;
-  // eslint-disable-next-line @typescript-eslint/explicit-function-return-type
 }) => {
   const commandStream = await TestDuplexStream.make<
     VatCommand,

packages/rpc-methods/src/RpcClient.test.ts

@@ -1,16 +1,8 @@
-import { string } from '@metamask/superstruct';
 import { jsonrpc2 } from '@metamask/utils';
 import { describe, it, vi, expect } from 'vitest';
 
 import { RpcClient } from './RpcClient.ts';
-
-const getMethods = () =>
-  ({
-    method1: {
-      method: 'method1',
-      result: string(),
-    },
-  }) as const;
+import { getMethods } from '../test/methods.ts';
 
 describe('RpcClient', () => {
   describe('constructor', () => {
@@ -23,19 +15,19 @@ describe('RpcClient', () => {
   describe('call', () => {
     it('should call a method', async () => {
       const client = new RpcClient(getMethods(), vi.fn(), 'test');
-      const resultP = client.call('method1', 'test');
+      const resultP = client.call('method1', ['test']);
       client.handleResponse('test:1', {
         jsonrpc: jsonrpc2,
         id: 'test:1',
-        result: 'test',
+        result: null,
       });
 
-      expect(await resultP).toBe('test');
+      expect(await resultP).toBeNull();
     });
 
     it('should throw an error for error responses', async () => {
       const client = new RpcClient(getMethods(), vi.fn(), 'test');
-      const resultP = client.call('method1', 'test');
+      const resultP = client.call('method1', ['test']);
       client.handleResponse('test:1', {
         jsonrpc: jsonrpc2,
         id: 'test:1',
@@ -50,20 +42,20 @@ describe('RpcClient', () => {
 
     it('should throw an error for invalid results', async () => {
       const client = new RpcClient(getMethods(), vi.fn(), 'test');
-      const resultP = client.call('method1', 'test');
+      const resultP = client.call('method1', ['test']);
       client.handleResponse('test:1', {
         jsonrpc: jsonrpc2,
         id: 'test:1',
         result: 42,
       });
       await expect(resultP).rejects.toThrow(
-        'Invalid result: Expected a string, but received: 42',
+        'Invalid result: Expected the literal `null`, but received: 42',
       );
     });
 
     it('should throw an error for invalid responses', async () => {
       const client = new RpcClient(getMethods(), vi.fn(), 'test');
-      const resultP = client.call('method1', 'test');
+      const resultP = client.call('method1', ['test']);
       client.handleResponse('test:1', 'invalid');
       await expect(resultP).rejects.toThrow('Invalid JSON-RPC response:');
     });
@@ -83,8 +75,8 @@ describe('RpcClient', () => {
   describe('rejectAll', () => {
     it('should reject all unresolved messages', async () => {
       const client = new RpcClient(getMethods(), vi.fn(), 'test');
-      const p1 = client.call('method1', 'test');
-      const p2 = client.call('method1', 'test');
+      const p1 = client.call('method1', ['test']);
+      const p2 = client.call('method1', ['test']);
       client.rejectAll(new Error('test error'));
       await expect(p1).rejects.toThrow('test error');
       await expect(p2).rejects.toThrow('test error');

packages/rpc-methods/src/RpcClient.ts

@@ -1,62 +1,35 @@
 import { makePromiseKit } from '@endo/promise-kit';
-import type { Struct } from '@metamask/superstruct';
 import { assert as assertStruct } from '@metamask/superstruct';
 import { assertIsJsonRpcResponse, isJsonRpcFailure } from '@metamask/utils';
-import type { Json, JsonRpcParams } from '@metamask/utils';
+import type { JsonRpcParams } from '@metamask/utils';
 import { makeCounter } from '@ocap/utils';
 import type { PromiseCallbacks } from '@ocap/utils';
 
-export type MethodSignature<
-  Method extends string,
-  Params extends JsonRpcParams,
-  Result extends Json,
-> = (method: Method, params: Params) => Promise<Result>;
-
-type ExtractMethodName<
-  Methods extends MethodSignature<string, JsonRpcParams, Json>,
-> = Methods extends (
-  method: infer Method,
-  params: JsonRpcParams,
-) => Promise<Json>
-  ? Method
-  : never;
-
-type ExtractParams<
-  Methods extends MethodSignature<string, JsonRpcParams, Json>,
-> = Methods extends (method: string, params: infer Params) => Promise<Json>
-  ? Params
-  : never;
-
-type ExtractResult<
-  Methods extends MethodSignature<string, JsonRpcParams, Json>,
-> = Methods extends (
-  method: string,
-  params: JsonRpcParams,
-) => Promise<infer Result>
-  ? Result
-  : never;
-
-export type MethodSpec<Method extends string, Result extends Json> = {
-  method: Method;
-  result: Struct<Result>;
-};
-
-type MethodSpecs<Methods extends MethodSignature<string, JsonRpcParams, Json>> =
-  Record<
-    ExtractMethodName<Methods>,
-    MethodSpec<ExtractMethodName<Methods>, ExtractResult<Methods>>
-  >;
+import type {
+  MethodSpec,
+  ExtractParams,
+  ExtractResult,
+  ExtractMethod,
+  MethodSpecRecord,
+} from './utils.ts';
 
 type RpcPayload = {
   method: string;
   params: JsonRpcParams;
 };
 
-type SendMessage = (messageId: string, payload: RpcPayload) => Promise<void>;
+export type SendMessage = (
+  messageId: string,
+  payload: RpcPayload,
+) => Promise<void>;
 
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-export class RpcClient<Methods extends MethodSignature<string, any, any>> {
-  readonly #methods: MethodSpecs<Methods>;
+export class RpcClient<
+  // The class picks up its type from the `methods` argument,
+  // so using `any` in this constraint is safe.
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  Methods extends MethodSpecRecord<MethodSpec<string, any, any>>,
+> {
+  readonly #methods: Methods;
 
   readonly #prefix: string;
 
@@ -66,20 +39,16 @@ export class RpcClient<Methods extends MethodSignature<string, any, any>> {
 
   readonly #sendMessage: SendMessage;
 
-  constructor(
-    methods: MethodSpecs<Methods>,
-    sendMessage: SendMessage,
-    prefix: string,
-  ) {
+  constructor(methods: Methods, sendMessage: SendMessage, prefix: string) {
     this.#methods = methods;
     this.#sendMessage = sendMessage;
     this.#prefix = prefix;
   }
 
-  async call<Method extends Methods>(
-    method: ExtractMethodName<Method>,
-    params: ExtractParams<Method>,
-  ): Promise<ExtractResult<Method>> {
+  async call<Method extends ExtractMethod<Methods>>(
+    method: Method,
+    params: ExtractParams<Method, Methods>,
+  ): Promise<ExtractResult<Method, Methods>> {
     const response = await this.#createMessage({
       method,
       params,
@@ -90,10 +59,23 @@ export class RpcClient<Methods extends MethodSignature<string, any, any>> {
       throw new Error(`${response.error.message}`);
     }
 
-    assertResult(response.result, this.#methods[method].result);
+    this.#assertResult(method, response.result);
     return response.result;
   }
 
+  #assertResult<Method extends ExtractMethod<Methods>>(
+    method: Method,
+    result: unknown,
+  ): asserts result is ExtractResult<Method, Methods> {
+    try {
+      // @ts-expect-error - TODO: For unknown reasons, TypeScript fails to recognize that
+      // `Method` must be a key of `this.#methods`.
+      assertStruct(result, this.#methods[method].result);
+    } catch (error) {
+      throw new Error(`Invalid result: ${(error as Error).message}`);
+    }
+  }
+
   async #createMessage(payload: RpcPayload): Promise<unknown> {
     const { promise, reject, resolve } = makePromiseKit<unknown>();
     const messageId = this.#nextMessageId();
@@ -128,19 +110,3 @@ export class RpcClient<Methods extends MethodSignature<string, any, any>> {
     return `${this.#prefix}:${this.#messageCounter()}`;
   }
 }
-
-/**
- * @param result - The result to assert.
- * @param struct - The struct to assert the result against.
- * @throws If the result is invalid.
- */
-function assertResult<Result extends Json>(
-  result: unknown,
-  struct: Struct<Result>,
-): asserts result is Result {
-  try {
-    assertStruct(result, struct);
-  } catch (error) {
-    throw new Error(`Invalid result: ${(error as Error).message}`);
-  }
-}

packages/rpc-methods/src/RpcService.test.ts

@@ -1,36 +1,7 @@
-import { number, string, tuple } from '@metamask/superstruct';
 import { describe, it, expect } from 'vitest';
 
 import { RpcService } from './RpcService.ts';
-import type { Handler } from './RpcService.ts';
-
-const getHooks = () =>
-  ({
-    hook1: () => undefined,
-    hook2: () => undefined,
-    hook3: () => undefined,
-  }) as const;
-
-type Hooks = ReturnType<typeof getHooks>;
-
-const getHandlers = () =>
-  ({
-    method1: {
-      method: 'method1',
-      implementation: () => null,
-      params: tuple([string()]),
-      hooks: ['hook1', 'hook2'] as const,
-    } as Handler<Hooks, 'method1', [string], null>,
-    method2: {
-      method: 'method2',
-      implementation: (hooks, [value]) => {
-        hooks.hook3();
-        return value * 2;
-      },
-      params: tuple([number()]),
-      hooks: ['hook3'] as const,
-    } as Handler<Hooks, 'method2', [number], number>,
-  }) as const;
+import { getHandlers, getHooks } from '../test/methods.ts';
 
 describe('RpcService', () => {
   describe('constructor', () => {