From 2e63d675ae039ff4fb0ccedf319b3447f127d2df Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Fri, 17 Oct 2025 15:19:19 -0700 Subject: [PATCH 01/18] feat: Make MiddlewareContext type safe --- .../src/v2/MiddlewareContext.test.ts | 31 ++++++++--- .../src/v2/MiddlewareContext.ts | 51 ++++++++++++++----- 2 files changed, 61 insertions(+), 21 deletions(-) diff --git a/packages/json-rpc-engine/src/v2/MiddlewareContext.test.ts b/packages/json-rpc-engine/src/v2/MiddlewareContext.test.ts index afec7d8112c..ccadf604a5a 100644 --- a/packages/json-rpc-engine/src/v2/MiddlewareContext.test.ts +++ b/packages/json-rpc-engine/src/v2/MiddlewareContext.test.ts @@ -1,14 +1,9 @@ import { MiddlewareContext } from './MiddlewareContext'; describe('MiddlewareContext', () => { - it('is a map', () => { - const context = new MiddlewareContext(); - expect(context).toBeInstanceOf(Map); - }); - it('can be constructed with entries', () => { const symbol = Symbol('test'); - const context = new MiddlewareContext([ + const context = new MiddlewareContext<{ test: string; [symbol]: string }>([ ['test', 'value'], [symbol, 'value'], ]); @@ -21,6 +16,26 @@ describe('MiddlewareContext', () => { expect(Object.isFrozen(context)).toBe(true); }); + it('type errors and returns undefined when getting unknown keys', () => { + const context = new MiddlewareContext<{ test: string }>(); + // @ts-expect-error - foo is not a valid key + expect(context.get('foo')).toBeUndefined(); + }); + + it('type errors and throws when assertGet:ing unknown keys', () => { + const context = new MiddlewareContext<{ test: string }>(); + // @ts-expect-error - foo is not a valid key + expect(() => context.assertGet('foo')).toThrow( + `Context key "foo" not found`, + ); + }); + + it('type errors when setting unknown keys', () => { + const context = new MiddlewareContext<{ test: string }>(); + // @ts-expect-error - foo is not a valid key + expect(context.set('foo', 'value')).toBe(context); + }); + it('assertGet throws if the key is not found', () => { const context = new MiddlewareContext(); expect(() => context.assertGet('test')).toThrow( @@ -29,14 +44,14 @@ describe('MiddlewareContext', () => { }); it('assertGet returns the value if the key is found (string)', () => { - const context = new MiddlewareContext(); + const context = new MiddlewareContext<{ test: string }>(); context.set('test', 'value'); expect(context.assertGet('test')).toBe('value'); }); it('assertGet returns the value if the key is found (symbol)', () => { - const context = new MiddlewareContext(); const symbol = Symbol('test'); + const context = new MiddlewareContext(); context.set(symbol, 'value'); expect(context.assertGet(symbol)).toBe('value'); }); diff --git a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts index 5bfa1ac1985..9ef1ce8b91e 100644 --- a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts +++ b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts @@ -1,29 +1,54 @@ /** - * A append-only context object for middleware. Its interface is frozen. + * An context object for middleware that attempts to protect against accidental + * modifications. Its interface is frozen. * - * Map keys may still be deleted. The append-only behavior is mostly intended - * to prevent accidental naming collisions. + * Map keys may not be directly overridden with {@link set}. Instead, use + * {@link delete} to remove a key and then {@link set} to add a new value. * - * The append-only behavior is overriden when using e.g. `Reflect.set`, - * so don't do that. + * The override protections are circumvented when using e.g. `Reflect.set`, so + * don't do that. */ -export class MiddlewareContext extends Map { - constructor(entries?: Iterable) { +export class MiddlewareContext< + KeyValues extends Record, +> extends Map { + constructor( + entries?: Iterable, + ) { super(entries); Object.freeze(this); } - assertGet(key: string | symbol): Value { - if (!this.has(key)) { + get(key: K): KeyValues[K] | undefined { + return super.get(key) as KeyValues[K] | undefined; + } + + /** + * Get a value from the context. Throws if the key is not found. + * + * @param key - The key to get the value for. + * @returns The value. + */ + assertGet(key: K): KeyValues[K] { + if (!super.has(key)) { throw new Error(`Context key "${String(key)}" not found`); } - return this.get(key) as Value; + return super.get(key) as KeyValues[K]; } - set(key: string | symbol, value: Value): this { - if (this.has(key)) { + /** + * Set a value in the context. Throws if the key already exists. + * {@link delete} an existing key before setting it to a new value. + * + * @throws If the key already exists. + * @param key - The key to set the value for. + * @param value - The value to set. + * @returns The context. + */ + set(key: K, value: KeyValues[K]): this { + if (super.has(key)) { throw new Error(`MiddlewareContext key "${String(key)}" already exists`); } - return super.set(key, value) as this; + super.set(key, value); + return this; } } From 8f4208f30dd405774e0f3ab3a81081cc94dc5858 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Fri, 17 Oct 2025 16:21:25 -0700 Subject: [PATCH 02/18] WIP: Generic context --- .../src/asV2Middleware.test.ts | 8 +- .../src/v2/JsonRpcEngineV2.test.ts | 107 ++++++++++++++---- .../json-rpc-engine/src/v2/JsonRpcEngineV2.ts | 59 ++++++---- .../src/v2/MiddlewareContext.test.ts | 6 +- .../src/v2/MiddlewareContext.ts | 4 +- .../src/v2/asLegacyMiddleware.test.ts | 11 +- .../src/v2/compatibility-utils.test.ts | 8 +- .../src/v2/compatibility-utils.ts | 2 +- 8 files changed, 147 insertions(+), 58 deletions(-) diff --git a/packages/json-rpc-engine/src/asV2Middleware.test.ts b/packages/json-rpc-engine/src/asV2Middleware.test.ts index 5d317e83d9d..a8d3510f297 100644 --- a/packages/json-rpc-engine/src/asV2Middleware.test.ts +++ b/packages/json-rpc-engine/src/asV2Middleware.test.ts @@ -4,6 +4,7 @@ import type { JsonRpcRequest } from '@metamask/utils'; import { JsonRpcEngine } from '.'; import { asV2Middleware } from './asV2Middleware'; import { JsonRpcEngineV2 } from './v2/JsonRpcEngineV2'; +import type { MiddlewareContext } from './v2/MiddlewareContext'; import { getExtraneousKeys, makeRequest } from '../tests/utils'; describe('asV2Middleware', () => { @@ -90,7 +91,10 @@ describe('asV2Middleware', () => { }); legacyEngine.push(legacyMiddleware); - const v2Engine = new JsonRpcEngineV2({ + const v2Engine = new JsonRpcEngineV2< + JsonRpcRequest, + MiddlewareContext> + >({ middleware: [ ({ context, next }) => { context.set('value', 1); @@ -98,7 +102,7 @@ describe('asV2Middleware', () => { }, asV2Middleware(legacyEngine), ({ context }) => { - observedContextValues.push(context.assertGet('newValue')); + observedContextValues.push(context.assertGet('newValue')); expect(Array.from(context.keys())).toStrictEqual([ 'value', diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts index 328909cf41a..998d88a8209 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts @@ -1,8 +1,8 @@ /* eslint-disable n/callback-return */ // next() is not a Node.js callback. -import type { JsonRpcId, NonEmptyArray } from '@metamask/utils'; +import type { Json, JsonRpcId, NonEmptyArray } from '@metamask/utils'; import { createDeferredPromise } from '@metamask/utils'; -import type { JsonRpcMiddleware } from './JsonRpcEngineV2'; +import type { JsonRpcMiddleware, ResultConstraint } from './JsonRpcEngineV2'; import { JsonRpcEngineV2 } from './JsonRpcEngineV2'; import { MiddlewareContext } from './MiddlewareContext'; import { @@ -285,10 +285,15 @@ describe('JsonRpcEngineV2', () => { }); it('accepts an initial context', async () => { - const initialContext = new MiddlewareContext(); + const initialContext = new MiddlewareContext>(); initialContext.set('foo', 'bar'); + const middleware: JsonRpcMiddleware< + JsonRpcRequest, + string, + MiddlewareContext> + > = jest.fn(({ context }) => context.assertGet('foo')); const engine = new JsonRpcEngineV2({ - middleware: [({ context }) => context.assertGet('foo')], + middleware: [middleware], }); const result = await engine.handle(makeRequest(), { @@ -298,6 +303,36 @@ describe('JsonRpcEngineV2', () => { expect(result).toBe('bar'); }); + it('accepts middleware with different context types', async () => { + const middleware1: JsonRpcMiddleware< + JsonRpcCall, + ResultConstraint, + MiddlewareContext<{ foo: string }> + > = ({ context, next }) => { + context.set('foo', 'bar'); + return next(); + }; + + const middleware2: JsonRpcMiddleware< + JsonRpcCall, + ResultConstraint + > = ({ next }) => next(); + + const middleware3: JsonRpcMiddleware< + JsonRpcCall, + string, + MiddlewareContext<{ foo: string; bar: number }> + > = ({ context }) => context.assertGet('foo'); + + const engine = new JsonRpcEngineV2({ + middleware: [middleware1, middleware2, middleware3], + }); + + const result = await engine.handle(makeRequest()); + + expect(result).toBe('bar'); + }); + it('throws if a middleware attempts to modify properties of the context', async () => { const engine = new JsonRpcEngineV2({ middleware: [ @@ -326,24 +361,34 @@ describe('JsonRpcEngineV2', () => { }); it('handles mixed synchronous and asynchronous middleware', async () => { - const middleware1: JsonRpcMiddleware> = - jest.fn(async ({ context, next }) => { - context.set('foo', [1]); - return next(); - }); - const middleware2: JsonRpcMiddleware> = - jest.fn(({ context, next }) => { - const nums = context.assertGet('foo'); - nums.push(2); - return next(); - }); + const middleware1: JsonRpcMiddleware< + JsonRpcRequest, + Json, + MiddlewareContext> + > = jest.fn(async ({ context, next }) => { + context.set('foo', [1]); + return next(); + }); + + const middleware2: JsonRpcMiddleware< + JsonRpcRequest, + Json, + MiddlewareContext> + > = jest.fn(({ context, next }) => { + const nums = context.assertGet('foo'); + nums.push(2); + return next(); + }); + const middleware3: JsonRpcMiddleware< JsonRpcRequest, - number[] + number[], + MiddlewareContext> > = jest.fn(async ({ context }) => { - const nums = context.assertGet('foo'); + const nums = context.assertGet('foo'); return [...nums, 3]; }); + const engine = new JsonRpcEngineV2({ middleware: [middleware1, middleware2, middleware3], }); @@ -625,7 +670,10 @@ describe('JsonRpcEngineV2', () => { let inFlight = 0; let maxInFlight = 0; - const engine = new JsonRpcEngineV2({ + const engine = new JsonRpcEngineV2< + JsonRpcRequest, + MiddlewareContext> + >({ middleware: [ async ({ context, next, request }) => { // eslint-disable-next-line jest/no-conditional-in-test @@ -785,10 +833,13 @@ describe('JsonRpcEngineV2', () => { }); it('propagates context changes', async () => { - const engine1 = new JsonRpcEngineV2({ + const engine1 = new JsonRpcEngineV2< + JsonRpcRequest, + MiddlewareContext> + >({ middleware: [ async ({ context, next }) => { - const nums = context.assertGet('foo'); + const nums = context.assertGet('foo'); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion nums[0]! *= 2; return next(); @@ -805,7 +856,7 @@ describe('JsonRpcEngineV2', () => { engine1.asMiddleware(), async ({ context }) => { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return context.assertGet('foo')[0]! * 2; + return context.assertGet('foo')[0]! * 2; }, ], }); @@ -931,10 +982,13 @@ describe('JsonRpcEngineV2', () => { }); it('propagates context changes', async () => { - const engine1 = new JsonRpcEngineV2({ + const engine1 = new JsonRpcEngineV2< + JsonRpcRequest, + MiddlewareContext> + >({ middleware: [ async ({ context }) => { - const nums = context.assertGet('foo'); + const nums = context.assertGet('foo'); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion nums[0]! *= 2; return null; @@ -942,7 +996,10 @@ describe('JsonRpcEngineV2', () => { ], }); - const engine2 = new JsonRpcEngineV2({ + const engine2 = new JsonRpcEngineV2< + JsonRpcRequest, + MiddlewareContext> + >({ middleware: [ async ({ context, next }) => { context.set('foo', [2]); @@ -954,7 +1011,7 @@ describe('JsonRpcEngineV2', () => { }, async ({ context }) => { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return context.assertGet('foo')[0]! * 2; + return context.assertGet('foo')[0]! * 2; }, ], }); diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts index dd94aaf21ad..4313ca9660f 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts @@ -37,17 +37,21 @@ export type Next = ( request?: Readonly, ) => Promise> | undefined>; -export type MiddlewareParams = { +export type MiddlewareParams< + Request extends JsonRpcCall, + Context extends MiddlewareContext, +> = { request: Readonly; - context: MiddlewareContext; + context: Context; next: Next; }; export type JsonRpcMiddleware< Request extends JsonRpcCall = JsonRpcCall, Result extends ResultConstraint = ResultConstraint, + Context extends MiddlewareContext = MiddlewareContext, > = ( - params: MiddlewareParams, + params: MiddlewareParams, ) => Readonly | undefined | Promise | undefined>; type RequestState = { @@ -55,12 +59,14 @@ type RequestState = { result: Readonly> | undefined; }; -type Options = { - middleware: NonEmptyArray>; +type Options = { + middleware: NonEmptyArray< + JsonRpcMiddleware, Context> + >; }; -type HandleOptions = { - context?: MiddlewareContext; +type HandleOptions = { + context?: Context; }; /** @@ -98,12 +104,19 @@ type HandleOptions = { * } * ``` */ -export class JsonRpcEngineV2 { - #middleware: Readonly>>; +export class JsonRpcEngineV2< + Request extends JsonRpcCall = JsonRpcCall, + Context extends MiddlewareContext = MiddlewareContext, +> { + #middleware: Readonly< + NonEmptyArray< + JsonRpcMiddleware, Context> + > + >; #isDestroyed = false; - constructor({ middleware }: Options) { + constructor({ middleware }: Options) { this.#middleware = [...middleware]; } @@ -119,7 +132,7 @@ export class JsonRpcEngineV2 { request: Extract extends never ? never : Extract, - options?: HandleOptions, + options?: HandleOptions, ): Promise< Extract extends never ? never @@ -137,7 +150,7 @@ export class JsonRpcEngineV2 { notification: Extract extends never ? never : WithoutId>, - options?: HandleOptions, + options?: HandleOptions, ): Promise< Extract extends never ? never @@ -155,12 +168,12 @@ export class JsonRpcEngineV2 { */ async handle( call: MixedParam, - options?: HandleOptions, + options?: HandleOptions, ): Promise | void>; async handle( request: Request, - { context }: HandleOptions = {}, + { context }: HandleOptions = {}, ): Promise> | void> { const isReq = isRequest(request); const { result } = await this.#handle(request, context); @@ -183,7 +196,7 @@ export class JsonRpcEngineV2 { */ async #handle( originalRequest: Request, - context: MiddlewareContext = new MiddlewareContext(), + context: Context = new MiddlewareContext() as Context, ): Promise> { this.#assertIsNotDestroyed(); @@ -220,9 +233,11 @@ export class JsonRpcEngineV2 { * @returns The `next()` function factory. */ #makeNextFactory( - middlewareIterator: Iterator>, + middlewareIterator: Iterator< + JsonRpcMiddleware, Context> + >, state: RequestState, - context: MiddlewareContext, + context: Context, ): () => Next { const makeNext = (): Next => { let wasCalled = false; @@ -264,7 +279,9 @@ export class JsonRpcEngineV2 { return makeNext; } - #makeMiddlewareIterator(): Iterator> { + #makeMiddlewareIterator(): Iterator< + JsonRpcMiddleware, Context> + > { return this.#middleware[Symbol.iterator](); } @@ -325,7 +342,11 @@ export class JsonRpcEngineV2 { * * @returns The JSON-RPC middleware. */ - asMiddleware(): JsonRpcMiddleware { + asMiddleware(): JsonRpcMiddleware< + Request, + ResultConstraint, + Context + > { this.#assertIsNotDestroyed(); return async ({ request, context, next }) => { diff --git a/packages/json-rpc-engine/src/v2/MiddlewareContext.test.ts b/packages/json-rpc-engine/src/v2/MiddlewareContext.test.ts index ccadf604a5a..8d755ddb485 100644 --- a/packages/json-rpc-engine/src/v2/MiddlewareContext.test.ts +++ b/packages/json-rpc-engine/src/v2/MiddlewareContext.test.ts @@ -37,7 +37,7 @@ describe('MiddlewareContext', () => { }); it('assertGet throws if the key is not found', () => { - const context = new MiddlewareContext(); + const context = new MiddlewareContext<{ test: string }>(); expect(() => context.assertGet('test')).toThrow( `Context key "test" not found`, ); @@ -51,13 +51,13 @@ describe('MiddlewareContext', () => { it('assertGet returns the value if the key is found (symbol)', () => { const symbol = Symbol('test'); - const context = new MiddlewareContext(); + const context = new MiddlewareContext<{ [symbol]: string }>(); context.set(symbol, 'value'); expect(context.assertGet(symbol)).toBe('value'); }); it('throws if setting an already set key', () => { - const context = new MiddlewareContext(); + const context = new MiddlewareContext<{ test: string }>(); context.set('test', 'value'); expect(() => context.set('test', 'value')).toThrow( `MiddlewareContext key "test" already exists`, diff --git a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts index 9ef1ce8b91e..3866a89163c 100644 --- a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts +++ b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts @@ -9,7 +9,9 @@ * don't do that. */ export class MiddlewareContext< - KeyValues extends Record, + // The `{}` type is not problematic in this context, it just means "no keys". + // eslint-disable-next-line @typescript-eslint/no-empty-object-type + KeyValues extends Record = {}, > extends Map { constructor( entries?: Iterable, diff --git a/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts b/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts index b83652976cd..40d11285125 100644 --- a/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts +++ b/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts @@ -5,8 +5,9 @@ import type { } from '@metamask/utils'; import { asLegacyMiddleware } from './asLegacyMiddleware'; -import type { JsonRpcMiddleware } from './JsonRpcEngineV2'; +import type { JsonRpcMiddleware, ResultConstraint } from './JsonRpcEngineV2'; import { JsonRpcEngineV2 } from './JsonRpcEngineV2'; +import type { MiddlewareContext } from './MiddlewareContext'; import { getExtraneousKeys, makeRequest } from '../../tests/utils'; import { JsonRpcEngine } from '../JsonRpcEngine'; @@ -147,13 +148,17 @@ describe('asLegacyMiddleware', () => { const observedContextValues: number[] = []; const v2Middleware = jest.fn((({ context, next }) => { - observedContextValues.push(context.assertGet('value')); + observedContextValues.push(context.assertGet('value')); expect(Array.from(context.keys())).toStrictEqual(['value']); context.set('newValue', 2); return next(); - }) satisfies JsonRpcMiddleware); + }) satisfies JsonRpcMiddleware< + JsonRpcRequest, + ResultConstraint, + MiddlewareContext> + >); const v2Engine = new JsonRpcEngineV2({ middleware: [v2Middleware], diff --git a/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts b/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts index d8c477ae601..eb2b8180ab4 100644 --- a/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts +++ b/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts @@ -265,7 +265,7 @@ describe('compatibility-utils', () => { params: [1], id: 42, }; - const context = new MiddlewareContext(); + const context = new MiddlewareContext>(); context.set('extraProp', 'value'); context.set('anotherProp', { nested: true }); @@ -288,7 +288,7 @@ describe('compatibility-utils', () => { params: [1], id: 42, }; - const context = new MiddlewareContext(); + const context = new MiddlewareContext>(); context.set('extraProp', 'value'); context.set(Symbol('anotherProp'), { nested: true }); @@ -310,7 +310,7 @@ describe('compatibility-utils', () => { params: [1], id: 42, }; - const context = new MiddlewareContext(); + const context = new MiddlewareContext>(); context.set('jsonrpc', '3.0'); context.set('method', 'other_method'); context.set('params', [2]); @@ -336,7 +336,7 @@ describe('compatibility-utils', () => { id: 42, existingKey: 'oldValue', }; - const context = new MiddlewareContext(); + const context = new MiddlewareContext>(); context.set('existingKey', 'newValue'); propagateToRequest(request, context); diff --git a/packages/json-rpc-engine/src/v2/compatibility-utils.ts b/packages/json-rpc-engine/src/v2/compatibility-utils.ts index d4eebf9cccc..03257b6a2ef 100644 --- a/packages/json-rpc-engine/src/v2/compatibility-utils.ts +++ b/packages/json-rpc-engine/src/v2/compatibility-utils.ts @@ -87,7 +87,7 @@ export function makeContext>( */ export function propagateToContext( req: Record, - context: MiddlewareContext, + context: MiddlewareContext>, ) { Object.keys(req) .filter( From 4b919a077c175af89235a2a620b4b255a4e24c92 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Sat, 18 Oct 2025 16:15:44 -0700 Subject: [PATCH 03/18] WIP: Working context, broken Request --- .../src/v2/JsonRpcEngineV2.test.ts | 139 +++++++++--------- .../json-rpc-engine/src/v2/JsonRpcEngineV2.ts | 36 ++++- .../src/v2/MiddlewareContext.ts | 79 +++++++++- 3 files changed, 180 insertions(+), 74 deletions(-) diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts index 998d88a8209..6335fde896f 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts @@ -22,7 +22,7 @@ describe('JsonRpcEngineV2', () => { describe('notifications', () => { it('passes the notification through a middleware', async () => { const middleware: JsonRpcMiddleware = jest.fn(); - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware], }); const notification = { jsonrpc, method: 'test_request' }; @@ -38,7 +38,7 @@ describe('JsonRpcEngineV2', () => { }); it('returns no result', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [jest.fn()], }); const notification = { jsonrpc, method: 'test_request' }; @@ -51,7 +51,7 @@ describe('JsonRpcEngineV2', () => { it('returns no result, with multiple middleware', async () => { const middleware1 = jest.fn(({ next }) => next()); const middleware2 = jest.fn(); - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware1, middleware2], }); const notification = { jsonrpc, method: 'test_request' }; @@ -64,7 +64,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware throws', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(() => { throw new Error('test'); @@ -79,7 +79,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware throws, with multiple middleware', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(({ next }) => next()), jest.fn(() => { @@ -95,7 +95,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a result is returned, from the first middleware', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [jest.fn(() => 'foo')], }); const notification = { jsonrpc, method: 'test_request' }; @@ -108,7 +108,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a result is returned, from a later middleware', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(async ({ next }) => { await next(); @@ -127,7 +127,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware calls next() multiple times', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(async ({ next }) => { await next(); @@ -151,7 +151,7 @@ describe('JsonRpcEngineV2', () => { const middleware: JsonRpcMiddleware = jest.fn( () => null, ); - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware], }); const request = makeRequest(); @@ -170,7 +170,7 @@ describe('JsonRpcEngineV2', () => { it('returns a result from the middleware, with multiple middleware', async () => { const middleware1 = jest.fn(({ next }) => next()); const middleware2 = jest.fn(() => null); - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware1, middleware2], }); const request = makeRequest(); @@ -193,7 +193,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware throws', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(() => { throw new Error('test'); @@ -207,7 +207,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware throws, with multiple middleware', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(({ next }) => next()), jest.fn(() => { @@ -222,7 +222,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if no middleware returns a result', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [jest.fn(({ next }) => next()), jest.fn()], }); const request = makeRequest(); @@ -235,7 +235,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware calls next() multiple times', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(async ({ next }) => { await next(); @@ -260,7 +260,7 @@ describe('JsonRpcEngineV2', () => { expect(context).toBeInstanceOf(Map); return null; }); - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware], }); @@ -275,7 +275,7 @@ describe('JsonRpcEngineV2', () => { const middleware2 = jest.fn(({ context }) => { return context.get('foo') as string | undefined; }); - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware1, middleware2], }); @@ -292,7 +292,7 @@ describe('JsonRpcEngineV2', () => { string, MiddlewareContext> > = jest.fn(({ context }) => context.assertGet('foo')); - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware], }); @@ -324,7 +324,7 @@ describe('JsonRpcEngineV2', () => { MiddlewareContext<{ foo: string; bar: number }> > = ({ context }) => context.assertGet('foo'); - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware1, middleware2, middleware3], }); @@ -334,7 +334,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware attempts to modify properties of the context', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(({ context }) => { context.set = () => undefined; @@ -351,7 +351,7 @@ describe('JsonRpcEngineV2', () => { describe('asynchrony', () => { it('handles asynchronous middleware', async () => { const middleware = jest.fn(async () => null); - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware], }); @@ -389,7 +389,7 @@ describe('JsonRpcEngineV2', () => { return [...nums, 3]; }); - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware1, middleware2, middleware3], }); @@ -423,7 +423,7 @@ describe('JsonRpcEngineV2', () => { observedMethod = request.method; return null; }); - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware1, middleware2, middleware3], }); const request = makeRequest({ params: [1] }); @@ -438,7 +438,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if directly modifying the request', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(({ request }) => { // @ts-expect-error Destructive testing. @@ -455,7 +455,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware attempts to modify the request "id" property', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(async ({ request, next }) => { return await next({ @@ -476,7 +476,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware attempts to modify the request "jsonrpc" property', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(async ({ request, next }) => { return await next({ @@ -499,7 +499,7 @@ describe('JsonRpcEngineV2', () => { describe('result handling', () => { it('updates the result after next() is called', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(async ({ next }) => { const result = await next(); @@ -515,7 +515,7 @@ describe('JsonRpcEngineV2', () => { }); it('updates an undefined result with a new value', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(async ({ next }) => { await next(); @@ -531,7 +531,7 @@ describe('JsonRpcEngineV2', () => { }); it('returning undefined propagates previously defined result', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(async ({ next }) => { await next(); @@ -547,7 +547,7 @@ describe('JsonRpcEngineV2', () => { it('catches errors thrown by later middleware', async () => { let observedError: Error | undefined; - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(async ({ next }) => { try { @@ -584,7 +584,7 @@ describe('JsonRpcEngineV2', () => { returnHandlerResults.push(3); }); const middleware4 = jest.fn(() => null); - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware1, middleware2, middleware3, middleware4], }); @@ -600,7 +600,7 @@ describe('JsonRpcEngineV2', () => { return result; }); const middleware2 = jest.fn(() => ({ foo: 'bar' })); - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware1, middleware2], }); @@ -670,10 +670,7 @@ describe('JsonRpcEngineV2', () => { let inFlight = 0; let maxInFlight = 0; - const engine = new JsonRpcEngineV2< - JsonRpcRequest, - MiddlewareContext> - >({ + const engine = JsonRpcEngineV2.create({ middleware: [ async ({ context, next, request }) => { // eslint-disable-next-line jest/no-conditional-in-test @@ -720,7 +717,7 @@ describe('JsonRpcEngineV2', () => { it('eagerly processes requests in parallel, i.e. without queueing them', async () => { const queue = makeArbitraryQueue(3); - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ async ({ request }) => { await queue.enqueue(request.id); @@ -749,10 +746,10 @@ describe('JsonRpcEngineV2', () => { describe('asMiddleware', () => { it('ends a request if it returns a value', async () => { // TODO: We may have to do a lot of these casts? - const engine1 = new JsonRpcEngineV2({ + const engine1 = JsonRpcEngineV2.create({ middleware: [() => null], }); - const engine2 = new JsonRpcEngineV2({ + const engine2 = JsonRpcEngineV2.create({ middleware: [engine1.asMiddleware(), jest.fn(() => 'foo')], }); @@ -762,10 +759,10 @@ describe('JsonRpcEngineV2', () => { }); it('permits returning undefined if a later middleware ends the request', async () => { - const engine1 = new JsonRpcEngineV2({ + const engine1 = JsonRpcEngineV2.create({ middleware: [() => undefined], }); - const engine2 = new JsonRpcEngineV2({ + const engine2 = JsonRpcEngineV2.create({ middleware: [engine1.asMiddleware(), () => null], }); @@ -777,13 +774,13 @@ describe('JsonRpcEngineV2', () => { it('composes nested engines', async () => { const middleware1 = jest.fn(async ({ next }) => next()); const middleware2 = jest.fn(async ({ next }) => next()); - const engine1 = new JsonRpcEngineV2({ + const engine1 = JsonRpcEngineV2.create({ middleware: [middleware1], }); - const engine2 = new JsonRpcEngineV2({ + const engine2 = JsonRpcEngineV2.create({ middleware: [engine1.asMiddleware(), middleware2], }); - const engine3 = new JsonRpcEngineV2({ + const engine3 = JsonRpcEngineV2.create({ middleware: [engine2.asMiddleware(), () => null], }); @@ -795,7 +792,7 @@ describe('JsonRpcEngineV2', () => { }); it('propagates request mutation', async () => { - const engine1 = new JsonRpcEngineV2({ + const engine1 = JsonRpcEngineV2.create({ middleware: [ ({ request, next }) => { return next({ @@ -815,7 +812,7 @@ describe('JsonRpcEngineV2', () => { }); let observedMethod: string | undefined; - const engine2 = new JsonRpcEngineV2({ + const engine2 = JsonRpcEngineV2.create({ middleware: [ engine1.asMiddleware(), ({ request }) => { @@ -833,7 +830,7 @@ describe('JsonRpcEngineV2', () => { }); it('propagates context changes', async () => { - const engine1 = new JsonRpcEngineV2< + const engine1 = JsonRpcEngineV2.create< JsonRpcRequest, MiddlewareContext> >({ @@ -847,7 +844,7 @@ describe('JsonRpcEngineV2', () => { ], }); - const engine2 = new JsonRpcEngineV2({ + const engine2 = JsonRpcEngineV2.create({ middleware: [ async ({ context, next }) => { context.set('foo', [2]); @@ -868,7 +865,7 @@ describe('JsonRpcEngineV2', () => { it('observes results in expected order', async () => { const returnHandlerResults: string[] = []; - const engine1 = new JsonRpcEngineV2({ + const engine1 = JsonRpcEngineV2.create({ middleware: [ async ({ next }) => { await next(); @@ -881,7 +878,7 @@ describe('JsonRpcEngineV2', () => { ], }); - const engine2 = new JsonRpcEngineV2({ + const engine2 = JsonRpcEngineV2.create({ middleware: [ engine1.asMiddleware(), async ({ next }) => { @@ -913,12 +910,12 @@ describe('JsonRpcEngineV2', () => { it('composes nested engines', async () => { const earlierMiddleware = jest.fn(async ({ next }) => next()); - const engine1 = new JsonRpcEngineV2({ + const engine1 = JsonRpcEngineV2.create({ middleware: [() => null], }); const laterMiddleware = jest.fn(() => 'foo'); - const engine2 = new JsonRpcEngineV2({ + const engine2 = JsonRpcEngineV2.create({ middleware: [ earlierMiddleware, async ({ request }) => { @@ -938,7 +935,7 @@ describe('JsonRpcEngineV2', () => { it('does not propagate request mutation', async () => { // Unlike asMiddleware(), although the inner engine mutates request, // those mutations do not propagate when using engine.handle(). - const engine1 = new JsonRpcEngineV2({ + const engine1 = JsonRpcEngineV2.create({ middleware: [ ({ request, next }) => { return next({ @@ -959,7 +956,7 @@ describe('JsonRpcEngineV2', () => { }); let observedMethod: string | undefined; - const engine2 = new JsonRpcEngineV2({ + const engine2 = JsonRpcEngineV2.create({ middleware: [ async ({ request, next, context }) => { await engine1.handle(request as JsonRpcRequest, { context }); @@ -982,7 +979,7 @@ describe('JsonRpcEngineV2', () => { }); it('propagates context changes', async () => { - const engine1 = new JsonRpcEngineV2< + const engine1 = JsonRpcEngineV2.create< JsonRpcRequest, MiddlewareContext> >({ @@ -996,7 +993,7 @@ describe('JsonRpcEngineV2', () => { ], }); - const engine2 = new JsonRpcEngineV2< + const engine2 = JsonRpcEngineV2.create< JsonRpcRequest, MiddlewareContext> >({ @@ -1023,7 +1020,7 @@ describe('JsonRpcEngineV2', () => { it('observes results in expected order', async () => { const returnHandlerResults: string[] = []; - const engine1 = new JsonRpcEngineV2({ + const engine1 = JsonRpcEngineV2.create({ middleware: [ async ({ next }) => { await next(); @@ -1037,7 +1034,7 @@ describe('JsonRpcEngineV2', () => { ], }); - const engine2 = new JsonRpcEngineV2({ + const engine2 = JsonRpcEngineV2.create({ middleware: [ async ({ request, next, context }) => { await engine1.handle(request as JsonRpcRequest, { context }); @@ -1068,7 +1065,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if the inner engine throws', async () => { - const engine1 = new JsonRpcEngineV2({ + const engine1 = JsonRpcEngineV2.create({ middleware: [ () => { throw new Error('test'); @@ -1076,7 +1073,7 @@ describe('JsonRpcEngineV2', () => { ], }); - const engine2 = new JsonRpcEngineV2({ + const engine2 = JsonRpcEngineV2.create({ middleware: [ async ({ request }) => { await engine1.handle(request as JsonRpcRequest); @@ -1093,7 +1090,7 @@ describe('JsonRpcEngineV2', () => { describe('request- and notification-only engines', () => { it('constructs a request-only engine', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [() => null], }); @@ -1109,7 +1106,7 @@ describe('JsonRpcEngineV2', () => { }); it('constructs a notification-only engine', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [() => undefined], }); @@ -1133,7 +1130,7 @@ describe('JsonRpcEngineV2', () => { }); it('constructs a mixed engine', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ // eslint-disable-next-line jest/no-conditional-in-test ({ request }) => (isRequest(request) ? null : undefined), @@ -1146,15 +1143,15 @@ describe('JsonRpcEngineV2', () => { }); it('composes a pipeline of request- and notification-only engines', async () => { - const requestEngine = new JsonRpcEngineV2({ + const requestEngine = JsonRpcEngineV2.create({ middleware: [() => null], }); - const notificationEngine = new JsonRpcEngineV2({ + const notificationEngine = JsonRpcEngineV2.create({ middleware: [() => undefined], }); - const orchestratorEngine = new JsonRpcEngineV2({ + const orchestratorEngine = JsonRpcEngineV2.create({ middleware: [ ({ request, context }) => // eslint-disable-next-line jest/no-conditional-in-test @@ -1180,7 +1177,7 @@ describe('JsonRpcEngineV2', () => { const middleware = { destroy: jest.fn(), }; - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware as unknown as JsonRpcMiddleware], }); @@ -1194,7 +1191,7 @@ describe('JsonRpcEngineV2', () => { destroy: jest.fn(), }; - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware as unknown as JsonRpcMiddleware], }); @@ -1205,7 +1202,7 @@ describe('JsonRpcEngineV2', () => { }); it('causes handle() to throw after destroying the engine', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [() => null], }); @@ -1217,7 +1214,7 @@ describe('JsonRpcEngineV2', () => { }); it('causes asMiddleware() to throw after destroying the engine', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [() => null], }); await engine.destroy(); @@ -1233,7 +1230,7 @@ describe('JsonRpcEngineV2', () => { throw new Error('test'); }), }; - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware as unknown as JsonRpcMiddleware], }); @@ -1249,7 +1246,7 @@ describe('JsonRpcEngineV2', () => { const middleware2 = { destroy: jest.fn(), }; - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ middleware1, middleware2, diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts index 4313ca9660f..a437fd6bf67 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts @@ -7,7 +7,7 @@ import { } from '@metamask/utils'; import deepFreeze from 'deep-freeze-strict'; -import { MiddlewareContext } from './MiddlewareContext'; +import { MergeContexts, MiddlewareContext } from './MiddlewareContext'; import { isNotification, isRequest, @@ -65,10 +65,24 @@ type Options = { >; }; +type CreateOptions< + InputReq extends JsonRpcCall, + Middleware extends NonEmptyArray< + JsonRpcMiddleware, any> + >, +> = { + middleware: Middleware; +}; + type HandleOptions = { context?: Context; }; +type ContextOf = M extends JsonRpcMiddleware ? C : never; + +type MergedContextOf[]> = + MergeContexts>; + /** * A JSON-RPC request and response processor. * @@ -106,7 +120,7 @@ type HandleOptions = { */ export class JsonRpcEngineV2< Request extends JsonRpcCall = JsonRpcCall, - Context extends MiddlewareContext = MiddlewareContext, + Context extends MiddlewareContext = MiddlewareContext, > { #middleware: Readonly< NonEmptyArray< @@ -120,6 +134,24 @@ export class JsonRpcEngineV2< this.#middleware = [...middleware]; } + static create< + InputReq extends JsonRpcCall, + Middleware extends NonEmptyArray< + JsonRpcMiddleware< + InputReq, + ResultConstraint, + MiddlewareContext + > + >, + >({ middleware }: CreateOptions) { + type InputContext = MergedContextOf; + // Cast once so the instance is typed with the merged context + const mw = middleware as unknown as NonEmptyArray< + JsonRpcMiddleware, InputContext> + >; + return new JsonRpcEngineV2({ middleware: mw }); + } + /** * Handle a JSON-RPC request. * diff --git a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts index 3866a89163c..7f3f8734ac3 100644 --- a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts +++ b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts @@ -1,3 +1,5 @@ +export type ContextKeys = string | symbol; + /** * An context object for middleware that attempts to protect against accidental * modifications. Its interface is frozen. @@ -11,7 +13,7 @@ export class MiddlewareContext< // The `{}` type is not problematic in this context, it just means "no keys". // eslint-disable-next-line @typescript-eslint/no-empty-object-type - KeyValues extends Record = {}, + KeyValues extends Record = {}, > extends Map { constructor( entries?: Iterable, @@ -54,3 +56,78 @@ export class MiddlewareContext< return this; } } + +/** + * Extract the {@link MiddlewareContext} union from an array of {@link MiddlewareContext}s. + */ +// Using `any` in this constraint does not pollute other types. +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type ExtractContexts[]> = T[number]; + +/** + * Infer the KeyValues type from a {@link MiddlewareContext}. + */ +// Using `any` in this constraint does not pollute other types. +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type InferKeyValues> = + T extends MiddlewareContext ? U : never; + +/** + * An unholy incantation that converts a union of object types into an + * intersection of object types. + * + * @example + * type A = { a: string } | { b: number }; + * type B = UnionToIntersection; // { a: string } & { b: number } + */ +type UnionToIntersection = ( + U extends unknown ? (k: U) => void : never +) extends (k: infer I) => void + ? I + : never; + +/** + * Simplifies an object type by "merging" its properties. + * + * - Expands intersections into a single object type. + * - Forces mapped/conditional results to resolve into a readable shape. + * - No runtime effect; purely a type-level normalization. + * + * @example + * type A = { a: string } & { b: number }; + * type B = Simplify; // { a: string; b: number } + */ +type Simplify = T extends infer O ? { [K in keyof O]: O[K] } : never; + +/** + * Rejects record types that contain any `never`-valued property. + * + * If any property of `T` resolves to `never`, the result is `never`; otherwise it returns `T` unchanged. + * Useful as a guard to ensure computed/merged record types didn't collapse any fields to `never`. + * + * @example + * type A = ExcludeNever<{ a: string; b: never }>; // never + * type B = ExcludeNever<{ a: string; b: number }>; // { a: string; b: number } + */ +type ExcludeNever> = { + [K in keyof T]-?: [T[K]] extends [never] ? K : never; +}[keyof T] extends never + ? T + : never; + +/** + * Merge a union of {@link MiddlewareContext}s into a single {@link MiddlewareContext} + * supertype. + * + * @param Contexts - The union of {@link MiddlewareContext}s to merge. + * @returns The merged {@link MiddlewareContext} supertype. + * @example + * type A = MiddlewareContext<{ a: string }> | MiddlewareContext<{ b: number }>; + * type B = MergeContexts; // MiddlewareContext<{ a: string, b: number }> + */ +export type MergeContexts[]> = + MiddlewareContext< + ExcludeNever< + Simplify>>> + > + >; From 90b8834057ceb8314bd863b39784550ddaa6cfcd Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Mon, 20 Oct 2025 14:12:03 -0700 Subject: [PATCH 04/18] WIP: Working, but not type safe --- .../src/v2/JsonRpcEngineV2.test.ts | 78 +++++++++---------- .../json-rpc-engine/src/v2/JsonRpcEngineV2.ts | 49 +++++++----- 2 files changed, 66 insertions(+), 61 deletions(-) diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts index 6335fde896f..4335fe2dbdb 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts @@ -336,9 +336,10 @@ describe('JsonRpcEngineV2', () => { it('throws if a middleware attempts to modify properties of the context', async () => { const engine = JsonRpcEngineV2.create({ middleware: [ - jest.fn(({ context }) => { + ({ context }) => { + // @ts-expect-error Destructive testing. context.set = () => undefined; - }), + }, ], }); @@ -478,13 +479,14 @@ describe('JsonRpcEngineV2', () => { it('throws if a middleware attempts to modify the request "jsonrpc" property', async () => { const engine = JsonRpcEngineV2.create({ middleware: [ - jest.fn(async ({ request, next }) => { + async ({ request, next }) => { return await next({ ...request, + // @ts-expect-error Destructive testing. jsonrpc: '3.0', }); - }), - jest.fn(() => null), + }, + () => null, ], }); const request = makeRequest(); @@ -499,13 +501,15 @@ describe('JsonRpcEngineV2', () => { describe('result handling', () => { it('updates the result after next() is called', async () => { + const middleware: JsonRpcMiddleware = async ({ next }) => { + const result = await next() as number; + return result + 1; + }; + const middleware2: JsonRpcMiddleware = () => 1; const engine = JsonRpcEngineV2.create({ middleware: [ - jest.fn(async ({ next }) => { - const result = await next(); - return result + 1; - }), - jest.fn(() => 1), + middleware, + middleware2, ], }); @@ -670,7 +674,9 @@ describe('JsonRpcEngineV2', () => { let inFlight = 0; let maxInFlight = 0; - const engine = JsonRpcEngineV2.create({ + const engine = JsonRpcEngineV2.create< + JsonRpcMiddleware> + >({ middleware: [ async ({ context, next, request }) => { // eslint-disable-next-line jest/no-conditional-in-test @@ -717,13 +723,12 @@ describe('JsonRpcEngineV2', () => { it('eagerly processes requests in parallel, i.e. without queueing them', async () => { const queue = makeArbitraryQueue(3); - const engine = JsonRpcEngineV2.create({ - middleware: [ - async ({ request }) => { - await queue.enqueue(request.id); - return null; - }, - ], + const middleware: JsonRpcMiddleware = async ({ request }) => { + await queue.enqueue(request.id); + return null; + }; + const engine = JsonRpcEngineV2.create({ + middleware: [middleware], }); const p0 = engine.handle(makeRequest({ id: 0 })); @@ -746,7 +751,7 @@ describe('JsonRpcEngineV2', () => { describe('asMiddleware', () => { it('ends a request if it returns a value', async () => { // TODO: We may have to do a lot of these casts? - const engine1 = JsonRpcEngineV2.create({ + const engine1 = JsonRpcEngineV2.create({ middleware: [() => null], }); const engine2 = JsonRpcEngineV2.create({ @@ -830,10 +835,7 @@ describe('JsonRpcEngineV2', () => { }); it('propagates context changes', async () => { - const engine1 = JsonRpcEngineV2.create< - JsonRpcRequest, - MiddlewareContext> - >({ + const engine1 = JsonRpcEngineV2.create({ middleware: [ async ({ context, next }) => { const nums = context.assertGet('foo'); @@ -910,8 +912,9 @@ describe('JsonRpcEngineV2', () => { it('composes nested engines', async () => { const earlierMiddleware = jest.fn(async ({ next }) => next()); - const engine1 = JsonRpcEngineV2.create({ - middleware: [() => null], + const middleware: JsonRpcMiddleware = () => null; + const engine1 = JsonRpcEngineV2.create({ + middleware: [middleware], }); const laterMiddleware = jest.fn(() => 'foo'); @@ -979,10 +982,7 @@ describe('JsonRpcEngineV2', () => { }); it('propagates context changes', async () => { - const engine1 = JsonRpcEngineV2.create< - JsonRpcRequest, - MiddlewareContext> - >({ + const engine1 = JsonRpcEngineV2.create({ middleware: [ async ({ context }) => { const nums = context.assertGet('foo'); @@ -993,10 +993,7 @@ describe('JsonRpcEngineV2', () => { ], }); - const engine2 = JsonRpcEngineV2.create< - JsonRpcRequest, - MiddlewareContext> - >({ + const engine2 = JsonRpcEngineV2.create({ middleware: [ async ({ context, next }) => { context.set('foo', [2]); @@ -1090,8 +1087,9 @@ describe('JsonRpcEngineV2', () => { describe('request- and notification-only engines', () => { it('constructs a request-only engine', async () => { - const engine = JsonRpcEngineV2.create({ - middleware: [() => null], + const middleware: JsonRpcMiddleware = () => null; + const engine = JsonRpcEngineV2.create({ + middleware: [middleware], }); expect(await engine.handle(makeRequest())).toBeNull(); @@ -1106,7 +1104,7 @@ describe('JsonRpcEngineV2', () => { }); it('constructs a notification-only engine', async () => { - const engine = JsonRpcEngineV2.create({ + const engine = JsonRpcEngineV2.create>({ middleware: [() => undefined], }); @@ -1130,7 +1128,7 @@ describe('JsonRpcEngineV2', () => { }); it('constructs a mixed engine', async () => { - const engine = JsonRpcEngineV2.create({ + const engine = JsonRpcEngineV2.create({ middleware: [ // eslint-disable-next-line jest/no-conditional-in-test ({ request }) => (isRequest(request) ? null : undefined), @@ -1143,15 +1141,15 @@ describe('JsonRpcEngineV2', () => { }); it('composes a pipeline of request- and notification-only engines', async () => { - const requestEngine = JsonRpcEngineV2.create({ + const requestEngine = JsonRpcEngineV2.create({ middleware: [() => null], }); - const notificationEngine = JsonRpcEngineV2.create({ + const notificationEngine = JsonRpcEngineV2.create({ middleware: [() => undefined], }); - const orchestratorEngine = JsonRpcEngineV2.create({ + const orchestratorEngine = JsonRpcEngineV2.create({ middleware: [ ({ request, context }) => // eslint-disable-next-line jest/no-conditional-in-test diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts index a437fd6bf67..442a42ea871 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts @@ -65,23 +65,27 @@ type Options = { >; }; -type CreateOptions< - InputReq extends JsonRpcCall, - Middleware extends NonEmptyArray< - JsonRpcMiddleware, any> - >, -> = { - middleware: Middleware; -}; - type HandleOptions = { context?: Context; }; -type ContextOf = M extends JsonRpcMiddleware ? C : never; - -type MergedContextOf[]> = - MergeContexts>; +type RequestOf = + Middleware extends JsonRpcMiddleware, any> + ? R + : never; + +type ContextOf = + Middleware extends JsonRpcMiddleware, infer C> + ? C + : never; + +type MergedContextOf< + Middleware extends JsonRpcMiddleware< + any, + ResultConstraint, + any + >, +> = MergeContexts>; /** * A JSON-RPC request and response processor. @@ -135,21 +139,24 @@ export class JsonRpcEngineV2< } static create< - InputReq extends JsonRpcCall, - Middleware extends NonEmptyArray< + Middleware extends JsonRpcMiddleware< - InputReq, - ResultConstraint, + any, + ResultConstraint, MiddlewareContext > - >, - >({ middleware }: CreateOptions) { + >({ middleware }: { middleware: Middleware[] }) { + type InputRequest = RequestOf; type InputContext = MergedContextOf; // Cast once so the instance is typed with the merged context const mw = middleware as unknown as NonEmptyArray< - JsonRpcMiddleware, InputContext> + JsonRpcMiddleware< + InputRequest, + ResultConstraint, + InputContext + > >; - return new JsonRpcEngineV2({ middleware: mw }); + return new JsonRpcEngineV2({ middleware: mw }); } /** From e4c9f58e6f84a5a40f4d8856b61b34c23dcf7f7e Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Mon, 20 Oct 2025 14:37:33 -0700 Subject: [PATCH 05/18] WIP: Request working, context broken again --- .../src/v2/JsonRpcEngineV2.test.ts | 37 +++++++++--------- .../json-rpc-engine/src/v2/JsonRpcEngineV2.ts | 38 +++++++++---------- .../src/v2/MiddlewareContext.ts | 10 +++-- 3 files changed, 41 insertions(+), 44 deletions(-) diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts index 4335fe2dbdb..c87c5659989 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts @@ -151,7 +151,7 @@ describe('JsonRpcEngineV2', () => { const middleware: JsonRpcMiddleware = jest.fn( () => null, ); - const engine = JsonRpcEngineV2.create({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware], }); const request = makeRequest(); @@ -291,8 +291,8 @@ describe('JsonRpcEngineV2', () => { JsonRpcRequest, string, MiddlewareContext> - > = jest.fn(({ context }) => context.assertGet('foo')); - const engine = JsonRpcEngineV2.create({ + > = ({ context }) => context.assertGet('foo'); + const engine = JsonRpcEngineV2.create({ middleware: [middleware], }); @@ -390,7 +390,7 @@ describe('JsonRpcEngineV2', () => { return [...nums, 3]; }); - const engine = JsonRpcEngineV2.create({ + const engine = JsonRpcEngineV2.create>({ middleware: [middleware1, middleware2, middleware3], }); @@ -501,16 +501,15 @@ describe('JsonRpcEngineV2', () => { describe('result handling', () => { it('updates the result after next() is called', async () => { - const middleware: JsonRpcMiddleware = async ({ next }) => { - const result = await next() as number; + const middleware: JsonRpcMiddleware = async ({ + next, + }) => { + const result = (await next()) as number; return result + 1; }; const middleware2: JsonRpcMiddleware = () => 1; - const engine = JsonRpcEngineV2.create({ - middleware: [ - middleware, - middleware2, - ], + const engine = JsonRpcEngineV2.create({ + middleware: [middleware, middleware2], }); const result = await engine.handle(makeRequest()); @@ -674,9 +673,7 @@ describe('JsonRpcEngineV2', () => { let inFlight = 0; let maxInFlight = 0; - const engine = JsonRpcEngineV2.create< - JsonRpcMiddleware> - >({ + const engine = JsonRpcEngineV2.create({ middleware: [ async ({ context, next, request }) => { // eslint-disable-next-line jest/no-conditional-in-test @@ -723,11 +720,13 @@ describe('JsonRpcEngineV2', () => { it('eagerly processes requests in parallel, i.e. without queueing them', async () => { const queue = makeArbitraryQueue(3); - const middleware: JsonRpcMiddleware = async ({ request }) => { + const middleware: JsonRpcMiddleware< + JsonRpcRequest & { id: number } + > = async ({ request }) => { await queue.enqueue(request.id); return null; }; - const engine = JsonRpcEngineV2.create({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware], }); @@ -912,7 +911,7 @@ describe('JsonRpcEngineV2', () => { it('composes nested engines', async () => { const earlierMiddleware = jest.fn(async ({ next }) => next()); - const middleware: JsonRpcMiddleware = () => null; + const middleware: JsonRpcMiddleware = () => null; const engine1 = JsonRpcEngineV2.create({ middleware: [middleware], }); @@ -1088,7 +1087,7 @@ describe('JsonRpcEngineV2', () => { describe('request- and notification-only engines', () => { it('constructs a request-only engine', async () => { const middleware: JsonRpcMiddleware = () => null; - const engine = JsonRpcEngineV2.create({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware], }); @@ -1104,7 +1103,7 @@ describe('JsonRpcEngineV2', () => { }); it('constructs a notification-only engine', async () => { - const engine = JsonRpcEngineV2.create>({ + const engine = JsonRpcEngineV2.create({ middleware: [() => undefined], }); diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts index 442a42ea871..2476561c7a9 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts @@ -7,7 +7,8 @@ import { } from '@metamask/utils'; import deepFreeze from 'deep-freeze-strict'; -import { MergeContexts, MiddlewareContext } from './MiddlewareContext'; +import type { ContextConstraint, MergeContexts } from './MiddlewareContext'; +import { MiddlewareContext } from './MiddlewareContext'; import { isNotification, isRequest, @@ -49,7 +50,9 @@ export type MiddlewareParams< export type JsonRpcMiddleware< Request extends JsonRpcCall = JsonRpcCall, Result extends ResultConstraint = ResultConstraint, - Context extends MiddlewareContext = MiddlewareContext, + Context extends ContextConstraint = MiddlewareContext< + Record + >, > = ( params: MiddlewareParams, ) => Readonly | undefined | Promise | undefined>; @@ -69,22 +72,17 @@ type HandleOptions = { context?: Context; }; -type RequestOf = - Middleware extends JsonRpcMiddleware, any> - ? R - : never; - type ContextOf = + // Non-polluting `any` constraint. + // eslint-disable-next-line @typescript-eslint/no-explicit-any Middleware extends JsonRpcMiddleware, infer C> ? C : never; type MergedContextOf< - Middleware extends JsonRpcMiddleware< - any, - ResultConstraint, - any - >, + // Non-polluting `any` constraint. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + Middleware extends JsonRpcMiddleware, any>, > = MergeContexts>; /** @@ -124,7 +122,7 @@ type MergedContextOf< */ export class JsonRpcEngineV2< Request extends JsonRpcCall = JsonRpcCall, - Context extends MiddlewareContext = MiddlewareContext, + Context extends ContextConstraint = MiddlewareContext, > { #middleware: Readonly< NonEmptyArray< @@ -134,19 +132,17 @@ export class JsonRpcEngineV2< #isDestroyed = false; - constructor({ middleware }: Options) { + private constructor({ middleware }: Options) { this.#middleware = [...middleware]; } static create< - Middleware extends - JsonRpcMiddleware< - any, - ResultConstraint, - MiddlewareContext - > + InputRequest extends JsonRpcCall = JsonRpcCall, + Middleware extends JsonRpcMiddleware< + InputRequest, + ResultConstraint + > = JsonRpcMiddleware>, >({ middleware }: { middleware: Middleware[] }) { - type InputRequest = RequestOf; type InputContext = MergedContextOf; // Cast once so the instance is typed with the merged context const mw = middleware as unknown as NonEmptyArray< diff --git a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts index 7f3f8734ac3..d4967088efa 100644 --- a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts +++ b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts @@ -1,5 +1,3 @@ -export type ContextKeys = string | symbol; - /** * An context object for middleware that attempts to protect against accidental * modifications. Its interface is frozen. @@ -13,7 +11,7 @@ export type ContextKeys = string | symbol; export class MiddlewareContext< // The `{}` type is not problematic in this context, it just means "no keys". // eslint-disable-next-line @typescript-eslint/no-empty-object-type - KeyValues extends Record = {}, + KeyValues extends Record = {}, > extends Map { constructor( entries?: Iterable, @@ -125,9 +123,13 @@ type ExcludeNever> = { * type A = MiddlewareContext<{ a: string }> | MiddlewareContext<{ b: number }>; * type B = MergeContexts; // MiddlewareContext<{ a: string, b: number }> */ -export type MergeContexts[]> = +export type MergeContexts = MiddlewareContext< ExcludeNever< Simplify>>> > >; + +// Non-polluting `any` constraint. +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export type ContextConstraint = MiddlewareContext; From aafdc9c1241980ef0e4446681963b3a0b7feefd2 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Mon, 20 Oct 2025 14:48:24 -0700 Subject: [PATCH 06/18] feat: Type safe context inference? --- .../src/asV2Middleware.test.ts | 10 +++++----- .../json-rpc-engine/src/v2/JsonRpcEngineV2.ts | 20 +++++++++++-------- .../src/v2/JsonRpcServer.test.ts | 2 +- .../json-rpc-engine/src/v2/JsonRpcServer.ts | 2 +- .../src/v2/asLegacyMiddleware.test.ts | 16 +++++++-------- 5 files changed, 27 insertions(+), 23 deletions(-) diff --git a/packages/json-rpc-engine/src/asV2Middleware.test.ts b/packages/json-rpc-engine/src/asV2Middleware.test.ts index a8d3510f297..dde682820b6 100644 --- a/packages/json-rpc-engine/src/asV2Middleware.test.ts +++ b/packages/json-rpc-engine/src/asV2Middleware.test.ts @@ -21,7 +21,7 @@ describe('asV2Middleware', () => { end(); }); - const v2Engine = new JsonRpcEngineV2({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [asV2Middleware(legacyEngine)], }); @@ -36,7 +36,7 @@ describe('asV2Middleware', () => { end(); }); - const v2Engine = new JsonRpcEngineV2({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [asV2Middleware(legacyEngine)], }); @@ -52,7 +52,7 @@ describe('asV2Middleware', () => { end(); }); - const v2Engine = new JsonRpcEngineV2({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [asV2Middleware(legacyEngine)], }); @@ -68,7 +68,7 @@ describe('asV2Middleware', () => { }); legacyEngine.push(legacyMiddleware); - const v2Engine = new JsonRpcEngineV2({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [asV2Middleware(legacyEngine), () => null], }); @@ -91,7 +91,7 @@ describe('asV2Middleware', () => { }); legacyEngine.push(legacyMiddleware); - const v2Engine = new JsonRpcEngineV2< + const v2Engine = JsonRpcEngineV2.create< JsonRpcRequest, MiddlewareContext> >({ diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts index 2476561c7a9..e29ec9d0ac7 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts @@ -50,9 +50,7 @@ export type MiddlewareParams< export type JsonRpcMiddleware< Request extends JsonRpcCall = JsonRpcCall, Result extends ResultConstraint = ResultConstraint, - Context extends ContextConstraint = MiddlewareContext< - Record - >, + Context extends ContextConstraint = MiddlewareContext, > = ( params: MiddlewareParams, ) => Readonly | undefined | Promise | undefined>; @@ -138,21 +136,27 @@ export class JsonRpcEngineV2< static create< InputRequest extends JsonRpcCall = JsonRpcCall, + InputContext extends ContextConstraint = ContextConstraint, Middleware extends JsonRpcMiddleware< InputRequest, - ResultConstraint - > = JsonRpcMiddleware>, + ResultConstraint, + InputContext + > = JsonRpcMiddleware< + InputRequest, + ResultConstraint, + InputContext + >, >({ middleware }: { middleware: Middleware[] }) { - type InputContext = MergedContextOf; + type MergedContext = MergedContextOf; // Cast once so the instance is typed with the merged context const mw = middleware as unknown as NonEmptyArray< JsonRpcMiddleware< InputRequest, ResultConstraint, - InputContext + MergedContext > >; - return new JsonRpcEngineV2({ middleware: mw }); + return new JsonRpcEngineV2({ middleware: mw }); } /** diff --git a/packages/json-rpc-engine/src/v2/JsonRpcServer.test.ts b/packages/json-rpc-engine/src/v2/JsonRpcServer.test.ts index 5b6ebac47b4..c337b842b1c 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcServer.test.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcServer.test.ts @@ -7,7 +7,7 @@ import { isRequest } from './utils'; const jsonrpc = '2.0' as const; const makeEngine = () => { - return new JsonRpcEngineV2({ + return JsonRpcEngineV2.create({ middleware: [ ({ request }) => { if (request.method !== 'hello') { diff --git a/packages/json-rpc-engine/src/v2/JsonRpcServer.ts b/packages/json-rpc-engine/src/v2/JsonRpcServer.ts index 66a8475d4ac..e30d90b2c16 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcServer.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcServer.ts @@ -74,7 +74,7 @@ export class JsonRpcServer { // @ts-expect-error - hasProperty fails to narrow the type. this.#engine = options.engine; } else { - this.#engine = new JsonRpcEngineV2({ middleware: options.middleware }); + this.#engine = JsonRpcEngineV2.create({ middleware: options.middleware }); } } diff --git a/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts b/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts index 40d11285125..03f047d740b 100644 --- a/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts +++ b/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts @@ -13,7 +13,7 @@ import { JsonRpcEngine } from '../JsonRpcEngine'; describe('asLegacyMiddleware', () => { it('converts a v2 engine to a legacy middleware', () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [() => null], }); const middleware = asLegacyMiddleware(engine); @@ -21,7 +21,7 @@ describe('asLegacyMiddleware', () => { }); it('forwards a result to the legacy engine', async () => { - const v2Engine = new JsonRpcEngineV2({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [() => null], }); @@ -36,7 +36,7 @@ describe('asLegacyMiddleware', () => { }); it('forwarded results are not frozen', async () => { - const v2Engine = new JsonRpcEngineV2({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [() => []], }); @@ -52,7 +52,7 @@ describe('asLegacyMiddleware', () => { }); it('forwards an error to the legacy engine', async () => { - const v2Engine = new JsonRpcEngineV2({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [ () => { throw new Error('test'); @@ -81,7 +81,7 @@ describe('asLegacyMiddleware', () => { it('allows the legacy engine to continue when not ending the request', async () => { const v2Middleware = jest.fn(({ next }) => next()); - const v2Engine = new JsonRpcEngineV2({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [v2Middleware], }); @@ -101,7 +101,7 @@ describe('asLegacyMiddleware', () => { it('allows the legacy engine to continue when not ending the request (passing through the original request)', async () => { const v2Middleware = jest.fn(({ request, next }) => next(request)); - const v2Engine = new JsonRpcEngineV2({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [v2Middleware], }); @@ -120,7 +120,7 @@ describe('asLegacyMiddleware', () => { }); it('propagates request modifications to the legacy engine', async () => { - const v2Engine = new JsonRpcEngineV2({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [ ({ request, next }) => next({ ...request, method: 'test_request_2' }), ], @@ -160,7 +160,7 @@ describe('asLegacyMiddleware', () => { MiddlewareContext> >); - const v2Engine = new JsonRpcEngineV2({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [v2Middleware], }); From 810c34b9c1c9d7b6ff8288bbd68a28a21272c17a Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Mon, 20 Oct 2025 15:17:04 -0700 Subject: [PATCH 07/18] refactor: Propagate context type safety to engine --- .../src/v2/JsonRpcEngineV2.test.ts | 67 +++++++++++++------ .../json-rpc-engine/src/v2/JsonRpcEngineV2.ts | 18 ++++- .../src/v2/MiddlewareContext.ts | 21 +++--- .../src/v2/compatibility-utils.test.ts | 6 +- 4 files changed, 75 insertions(+), 37 deletions(-) diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts index c87c5659989..b65e1baf206 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts @@ -333,11 +333,34 @@ describe('JsonRpcEngineV2', () => { expect(result).toBe('bar'); }); + it('type errors if different context types are mutually exclusive', async () => { + const middleware1: JsonRpcMiddleware< + JsonRpcCall, + ResultConstraint, + MiddlewareContext<{ foo: string }> + > = ({ next, context }) => { + context.set('foo', 'bar'); + return next(); + }; + + const middleware2: JsonRpcMiddleware< + JsonRpcCall, + ResultConstraint, + MiddlewareContext<{ foo: number }> + > = ({ context }) => context.assertGet('foo'); + const engine = JsonRpcEngineV2.create({ + middleware: [middleware1, middleware2], + }); + + // @ts-expect-error - The engine is `never`, which is what we want. + expect(await engine.handle(makeRequest())).toBe('bar'); + }); + it('throws if a middleware attempts to modify properties of the context', async () => { const engine = JsonRpcEngineV2.create({ middleware: [ ({ context }) => { - // @ts-expect-error Destructive testing. + // @ts-expect-error - Destructive testing. context.set = () => undefined; }, ], @@ -442,7 +465,7 @@ describe('JsonRpcEngineV2', () => { const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(({ request }) => { - // @ts-expect-error Destructive testing. + // @ts-expect-error - Destructive testing. request.params = [2]; }) as JsonRpcMiddleware, ], @@ -482,7 +505,7 @@ describe('JsonRpcEngineV2', () => { async ({ request, next }) => { return await next({ ...request, - // @ts-expect-error Destructive testing. + // @ts-expect-error - Destructive testing. jsonrpc: '3.0', }); }, @@ -796,7 +819,7 @@ describe('JsonRpcEngineV2', () => { }); it('propagates request mutation', async () => { - const engine1 = JsonRpcEngineV2.create({ + const engine1 = JsonRpcEngineV2.create>({ middleware: [ ({ request, next }) => { return next({ @@ -808,21 +831,23 @@ describe('JsonRpcEngineV2', () => { return next({ ...request, method: 'test_request_2', - // @ts-expect-error Will obviously work. - params: [request.params[0] * 2], + // Obviously correct. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + params: [request.params![0]! * 2], }); }, ], }); let observedMethod: string | undefined; - const engine2 = JsonRpcEngineV2.create({ + const engine2 = JsonRpcEngineV2.create>({ middleware: [ engine1.asMiddleware(), ({ request }) => { observedMethod = request.method; - // @ts-expect-error Will obviously work. - return request.params[0] * 2; + // Obviously correct. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return request.params![0]! * 2; }, ], }); @@ -937,7 +962,7 @@ describe('JsonRpcEngineV2', () => { it('does not propagate request mutation', async () => { // Unlike asMiddleware(), although the inner engine mutates request, // those mutations do not propagate when using engine.handle(). - const engine1 = JsonRpcEngineV2.create({ + const engine1 = JsonRpcEngineV2.create>({ middleware: [ ({ request, next }) => { return next({ @@ -949,8 +974,9 @@ describe('JsonRpcEngineV2', () => { return next({ ...request, method: 'test_request_2', - // @ts-expect-error Will obviously work at runtime - params: [request.params[0] * 2], + // Obviously correct. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + params: [request.params![0]! * 2], }); }, () => null, @@ -958,16 +984,17 @@ describe('JsonRpcEngineV2', () => { }); let observedMethod: string | undefined; - const engine2 = JsonRpcEngineV2.create({ + const engine2 = JsonRpcEngineV2.create>({ middleware: [ async ({ request, next, context }) => { - await engine1.handle(request as JsonRpcRequest, { context }); + await engine1.handle(request, { context }); return next(); }, ({ request }) => { observedMethod = request.method; - // @ts-expect-error Will obviously work at runtime - return request.params[0] * 2; + // Obviously correct. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return request.params![0]! * 2; }, ], }); @@ -1092,9 +1119,9 @@ describe('JsonRpcEngineV2', () => { }); expect(await engine.handle(makeRequest())).toBeNull(); - // @ts-expect-error Valid at runtime, but should cause a type error + // @ts-expect-error - Valid at runtime, but should cause a type error expect(await engine.handle(makeRequest() as JsonRpcCall)).toBeNull(); - // @ts-expect-error Invalid at runtime and should cause a type error + // @ts-expect-error - Invalid at runtime and should cause a type error await expect(engine.handle(makeNotification())).rejects.toThrow( new JsonRpcEngineError( `Result returned for notification: ${stringify(makeNotification())}`, @@ -1109,7 +1136,7 @@ describe('JsonRpcEngineV2', () => { expect(await engine.handle(makeNotification())).toBeUndefined(); await expect( - // @ts-expect-error Invalid at runtime and should cause a type error + // @ts-expect-error - Invalid at runtime and should cause a type error engine.handle({ id: '1', jsonrpc, method: 'test_request' }), ).rejects.toThrow( new JsonRpcEngineError( @@ -1117,7 +1144,7 @@ describe('JsonRpcEngineV2', () => { ), ); await expect( - // @ts-expect-error Invalid at runtime and should cause a type error + // @ts-expect-error - Invalid at runtime and should cause a type error engine.handle(makeRequest() as JsonRpcRequest), ).rejects.toThrow( new JsonRpcEngineError( diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts index e29ec9d0ac7..ff02b29a915 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts @@ -130,10 +130,21 @@ export class JsonRpcEngineV2< #isDestroyed = false; + // See .create() for why this is private. private constructor({ middleware }: Options) { this.#middleware = [...middleware]; } + // We use a static factory method in order to construct a supertype of all middleware contexts, + // which enables us to instantiate an engine despite different middleware expecting different + // context types. + /** + * Create a new JSON-RPC engine. + * + * @param options - The options for the engine. + * @param options.middleware - The middleware to use. + * @returns The JSON-RPC engine. + */ static create< InputRequest extends JsonRpcCall = JsonRpcCall, InputContext extends ContextConstraint = ContextConstraint, @@ -156,7 +167,12 @@ export class JsonRpcEngineV2< MergedContext > >; - return new JsonRpcEngineV2({ middleware: mw }); + + return new JsonRpcEngineV2({ + middleware: mw, + }) as MergedContext extends never + ? never + : JsonRpcEngineV2; } /** diff --git a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts index d4967088efa..cc787e8addd 100644 --- a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts +++ b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts @@ -55,13 +55,6 @@ export class MiddlewareContext< } } -/** - * Extract the {@link MiddlewareContext} union from an array of {@link MiddlewareContext}s. - */ -// Using `any` in this constraint does not pollute other types. -// eslint-disable-next-line @typescript-eslint/no-explicit-any -type ExtractContexts[]> = T[number]; - /** * Infer the KeyValues type from a {@link MiddlewareContext}. */ @@ -123,12 +116,14 @@ type ExcludeNever> = { * type A = MiddlewareContext<{ a: string }> | MiddlewareContext<{ b: number }>; * type B = MergeContexts; // MiddlewareContext<{ a: string, b: number }> */ -export type MergeContexts = - MiddlewareContext< - ExcludeNever< - Simplify>>> - > - >; +export type MergeContexts = + ExcludeNever< + Simplify>> + > extends never + ? never + : MiddlewareContext< + ExcludeNever>>> + >; // Non-polluting `any` constraint. // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts b/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts index eb2b8180ab4..797d4a59675 100644 --- a/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts +++ b/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts @@ -123,7 +123,7 @@ describe('compatibility-utils', () => { params: undefined, }; - // @ts-expect-error - Intentional abuse + // @ts-expect-error - Destructive testing const request = fromLegacyRequest(legacyRequest); expect(request).toStrictEqual({ @@ -140,7 +140,7 @@ describe('compatibility-utils', () => { id: 42, }; - // @ts-expect-error - Intentional abuse + // @ts-expect-error - Destructive testing const request = fromLegacyRequest(legacyRequest); expect(request).toStrictEqual({ @@ -159,7 +159,7 @@ describe('compatibility-utils', () => { id: 42, }; - // @ts-expect-error - Intentional abuse + // @ts-expect-error - Destructive testing const request = fromLegacyRequest(legacyRequest); expect(request).toStrictEqual({ From 569a598a75f8e3f3e100dcb86c5f72fc3bf34ad3 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Mon, 20 Oct 2025 15:44:50 -0700 Subject: [PATCH 08/18] feat: Make create() throw if the middleware array is empty --- .../src/v2/JsonRpcEngineV2.test.ts | 8 ++++ .../json-rpc-engine/src/v2/JsonRpcEngineV2.ts | 39 +++++++++++-------- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts index b65e1baf206..a8597e19453 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts @@ -18,6 +18,14 @@ import { makeNotification, makeRequest } from '../../tests/utils'; const jsonrpc = '2.0' as const; describe('JsonRpcEngineV2', () => { + describe('create', () => { + it('throws if the middleware array is empty', () => { + expect(() => JsonRpcEngineV2.create({ middleware: [] })).toThrow( + new JsonRpcEngineError('Middleware array cannot be empty'), + ); + }); + }); + describe('handle', () => { describe('notifications', () => { it('passes the notification through a middleware', async () => { diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts index ff02b29a915..b59cac72f0e 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts @@ -60,16 +60,19 @@ type RequestState = { result: Readonly> | undefined; }; -type Options = { +type HandleOptions = { + context?: Context; +}; + +type ConstructorOptions< + Request extends JsonRpcCall, + Context extends MiddlewareContext, +> = { middleware: NonEmptyArray< JsonRpcMiddleware, Context> >; }; -type HandleOptions = { - context?: Context; -}; - type ContextOf = // Non-polluting `any` constraint. // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -80,9 +83,14 @@ type ContextOf = type MergedContextOf< // Non-polluting `any` constraint. // eslint-disable-next-line @typescript-eslint/no-explicit-any - Middleware extends JsonRpcMiddleware, any>, + Middleware extends JsonRpcMiddleware, > = MergeContexts>; +type MiddlewareConstraint< + Request extends JsonRpcCall, + Context extends ContextConstraint, +> = JsonRpcMiddleware, Context>; + /** * A JSON-RPC request and response processor. * @@ -131,7 +139,7 @@ export class JsonRpcEngineV2< #isDestroyed = false; // See .create() for why this is private. - private constructor({ middleware }: Options) { + private constructor({ middleware }: ConstructorOptions) { this.#middleware = [...middleware]; } @@ -141,6 +149,7 @@ export class JsonRpcEngineV2< /** * Create a new JSON-RPC engine. * + * @throws If the middleware array is empty. * @param options - The options for the engine. * @param options.middleware - The middleware to use. * @returns The JSON-RPC engine. @@ -148,18 +157,17 @@ export class JsonRpcEngineV2< static create< InputRequest extends JsonRpcCall = JsonRpcCall, InputContext extends ContextConstraint = ContextConstraint, - Middleware extends JsonRpcMiddleware< - InputRequest, - ResultConstraint, - InputContext - > = JsonRpcMiddleware< + Middleware extends MiddlewareConstraint< InputRequest, - ResultConstraint, InputContext - >, + > = MiddlewareConstraint, >({ middleware }: { middleware: Middleware[] }) { + // We can't use NonEmptyArray for the params because it ruins type inference. + if (middleware.length === 0) { + throw new JsonRpcEngineError('Middleware array cannot be empty'); + } + type MergedContext = MergedContextOf; - // Cast once so the instance is typed with the merged context const mw = middleware as unknown as NonEmptyArray< JsonRpcMiddleware< InputRequest, @@ -167,7 +175,6 @@ export class JsonRpcEngineV2< MergedContext > >; - return new JsonRpcEngineV2({ middleware: mw, }) as MergedContext extends never From 303d5845bae04f1b57ffa74f211095521ab3d241 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Mon, 20 Oct 2025 16:41:11 -0700 Subject: [PATCH 09/18] fix: Type safety and inference (finally?) --- packages/json-rpc-engine/README.md | 35 +-- packages/json-rpc-engine/src/README.md | 2 +- .../src/asV2Middleware.test.ts | 51 ++-- .../src/v2/JsonRpcEngineV2.test.ts | 236 ++++++++++-------- .../json-rpc-engine/src/v2/JsonRpcEngineV2.ts | 34 +-- .../src/v2/JsonRpcServer.test.ts | 16 +- .../src/v2/asLegacyMiddleware.test.ts | 44 ++-- 7 files changed, 233 insertions(+), 185 deletions(-) diff --git a/packages/json-rpc-engine/README.md b/packages/json-rpc-engine/README.md index a3d766dc79a..1938d2fea4c 100644 --- a/packages/json-rpc-engine/README.md +++ b/packages/json-rpc-engine/README.md @@ -18,7 +18,8 @@ or ```ts import { JsonRpcEngineV2 } from '@metamask/json-rpc-engine/v2'; -const engine = new JsonRpcEngineV2({ +// You must use the static factory method `create()` instead of the constructor. +const engine = JsonRpcEngineV2.create({ // Create a stack of middleware and pass it to the engine: middleware: [ ({ request, next, context }) => { @@ -81,7 +82,7 @@ import { JsonRpcEngine } from '@metamask/json-rpc-engine'; const legacyEngine = new JsonRpcEngine(); -const v2Engine = new JsonRpcEngineV2({ +const v2Engine = JsonRpcEngineV2.create({ middleware: [ // ... ], @@ -109,7 +110,7 @@ They receive a `MiddlewareParams` object containing: Here's a basic example: ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ ({ next, context }) => { context.set('foo', 'bar'); @@ -136,7 +137,7 @@ For requests, one of the engine's middleware must "end" the request by returning will throw an error: ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ () => { if (Math.random() > 0.5) { @@ -181,7 +182,7 @@ import { JsonRpcEngineV2, } from '@metamask/json-rpc-engine/v2'; -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ async ({ request, next }) => { if (isRequest(request) && request.method === 'everything') { @@ -208,7 +209,7 @@ Middleware can modify the `method` and `params` properties by passing a new request object to `next()`: ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ ({ request, next }) => { // Modify the request for subsequent middleware @@ -231,7 +232,7 @@ Modifying the `jsonrpc` or `id` properties is not allowed, and will cause an error: ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ ({ request, next }) => { // Modifying either property will cause an error @@ -251,7 +252,7 @@ const engine = new JsonRpcEngineV2({ Middleware can observe the result by awaiting `next()`: ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ async ({ request, next }) => { const startTime = Date.now(); @@ -277,7 +278,7 @@ Like the `request`, the `result` is also immutable. Middleware can update the result by returning a new one. ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ async ({ request, next }) => { const result = await next(); @@ -323,7 +324,7 @@ console.log(result); Use the `context` to share data between middleware: ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ async ({ context, next }) => { context.set('user', { id: '123', name: 'Alice' }); @@ -350,7 +351,7 @@ it is append-only with deletions. If you need to modify a context value over multiple middleware, use an array or object: ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ async ({ context, next }) => { context.set('user', { id: '123', name: 'Alice' }); @@ -371,7 +372,7 @@ const engine = new JsonRpcEngineV2({ Errors in middleware are propagated up the call stack: ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ ({ next }) => { return next(); @@ -395,7 +396,7 @@ try { If your middleware awaits `next()`, it can handle errors using `try`/`catch`: ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ ({ request, next }) => { try { @@ -428,7 +429,7 @@ console.log('Result:', result); Engines can be nested by converting them to middleware using `asMiddleware()`: ```ts -const subEngine = new JsonRpcEngineV2({ +const subEngine = JsonRpcEngineV2.create({ middleware: [ ({ request }) => { return 'Sub-engine result'; @@ -436,7 +437,7 @@ const subEngine = new JsonRpcEngineV2({ ], }); -const mainEngine = new JsonRpcEngineV2({ +const mainEngine = JsonRpcEngineV2.create({ middleware: [ subEngine.asMiddleware(), ({ request, next }) => { @@ -451,7 +452,7 @@ Engines used as middleware may return `undefined` for requests, but only when used as middleware: ```ts -const loggingEngine = new JsonRpcEngineV2({ +const loggingEngine = JsonRpcEngineV2.create({ middleware: [ ({ request, next }) => { console.log('Observed request:', request.method); @@ -459,7 +460,7 @@ const loggingEngine = new JsonRpcEngineV2({ ], }); -const mainEngine = new JsonRpcEngineV2({ +const mainEngine = JsonRpcEngineV2.create({ middleware: [ loggingEngine.asMiddleware(), ({ request }) => { diff --git a/packages/json-rpc-engine/src/README.md b/packages/json-rpc-engine/src/README.md index 61fe01051f2..a83af82fbc2 100644 --- a/packages/json-rpc-engine/src/README.md +++ b/packages/json-rpc-engine/src/README.md @@ -33,7 +33,7 @@ import { asV2Middleware, JsonRpcEngine } from '@metamask/json-rpc-engine'; const legacyEngine = new JsonRpcEngine(); legacyEngine.push(/* ... */); -const v2Engine = new JsonRpcEngineV2({ +const v2Engine = JsonRpcEngineV2.create({ middleware: [asV2Middleware(legacyEngine)], }); ``` diff --git a/packages/json-rpc-engine/src/asV2Middleware.test.ts b/packages/json-rpc-engine/src/asV2Middleware.test.ts index dde682820b6..774e14ad858 100644 --- a/packages/json-rpc-engine/src/asV2Middleware.test.ts +++ b/packages/json-rpc-engine/src/asV2Middleware.test.ts @@ -1,9 +1,10 @@ import { rpcErrors } from '@metamask/rpc-errors'; -import type { JsonRpcRequest } from '@metamask/utils'; +import type { Json, JsonRpcRequest } from '@metamask/utils'; import { JsonRpcEngine } from '.'; import { asV2Middleware } from './asV2Middleware'; import { JsonRpcEngineV2 } from './v2/JsonRpcEngineV2'; +import type { JsonRpcMiddleware as V2Middleware } from './v2/JsonRpcEngineV2'; import type { MiddlewareContext } from './v2/MiddlewareContext'; import { getExtraneousKeys, makeRequest } from '../tests/utils'; @@ -21,7 +22,7 @@ describe('asV2Middleware', () => { end(); }); - const v2Engine = JsonRpcEngineV2.create({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [asV2Middleware(legacyEngine)], }); @@ -36,7 +37,7 @@ describe('asV2Middleware', () => { end(); }); - const v2Engine = JsonRpcEngineV2.create({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [asV2Middleware(legacyEngine)], }); @@ -52,7 +53,7 @@ describe('asV2Middleware', () => { end(); }); - const v2Engine = JsonRpcEngineV2.create({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [asV2Middleware(legacyEngine)], }); @@ -68,8 +69,9 @@ describe('asV2Middleware', () => { }); legacyEngine.push(legacyMiddleware); - const v2Engine = JsonRpcEngineV2.create({ - middleware: [asV2Middleware(legacyEngine), () => null], + const v2Middleware: V2Middleware = () => null; + const v2Engine = JsonRpcEngineV2.create({ + middleware: [asV2Middleware(legacyEngine), v2Middleware], }); const result = await v2Engine.handle(makeRequest()); @@ -91,27 +93,22 @@ describe('asV2Middleware', () => { }); legacyEngine.push(legacyMiddleware); - const v2Engine = JsonRpcEngineV2.create< - JsonRpcRequest, - MiddlewareContext> - >({ - middleware: [ - ({ context, next }) => { - context.set('value', 1); - return next(); - }, - asV2Middleware(legacyEngine), - ({ context }) => { - observedContextValues.push(context.assertGet('newValue')); - - expect(Array.from(context.keys())).toStrictEqual([ - 'value', - 'newValue', - ]); - - return null; - }, - ], + type Context = MiddlewareContext>; + const middleware1: V2Middleware = ({ + context, + next, + }) => { + context.set('value', 1); + return next(); + }; + const middleware2: V2Middleware = ({ + context, + }) => { + observedContextValues.push(context.assertGet('newValue')); + return null; + }; + const v2Engine = JsonRpcEngineV2.create({ + middleware: [middleware1, asV2Middleware(legacyEngine), middleware2], }); await v2Engine.handle(makeRequest()); diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts index a8597e19453..86743a3ed01 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts @@ -17,6 +17,14 @@ import { makeNotification, makeRequest } from '../../tests/utils'; const jsonrpc = '2.0' as const; +const makeNullMiddleware = (): JsonRpcMiddleware => { + return () => null; +}; + +const makeVoidMiddleware = (): JsonRpcMiddleware => { + return () => undefined; +}; + describe('JsonRpcEngineV2', () => { describe('create', () => { it('throws if the middleware array is empty', () => { @@ -103,8 +111,9 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a result is returned, from the first middleware', async () => { + const middleware: JsonRpcMiddleware = () => 'foo'; const engine = JsonRpcEngineV2.create({ - middleware: [jest.fn(() => 'foo')], + middleware: [middleware], }); const notification = { jsonrpc, method: 'test_request' }; @@ -118,11 +127,11 @@ describe('JsonRpcEngineV2', () => { it('throws if a result is returned, from a later middleware', async () => { const engine = JsonRpcEngineV2.create({ middleware: [ - jest.fn(async ({ next }) => { + async ({ next }) => { await next(); return undefined; - }), - jest.fn(() => null), + }, + makeNullMiddleware(), ], }); const notification = { jsonrpc, method: 'test_request' }; @@ -156,10 +165,8 @@ describe('JsonRpcEngineV2', () => { describe('requests', () => { it('returns a result from the middleware', async () => { - const middleware: JsonRpcMiddleware = jest.fn( - () => null, - ); - const engine = JsonRpcEngineV2.create({ + const middleware: JsonRpcMiddleware = jest.fn(() => null); + const engine = JsonRpcEngineV2.create({ middleware: [middleware], }); const request = makeRequest(); @@ -176,8 +183,8 @@ describe('JsonRpcEngineV2', () => { }); it('returns a result from the middleware, with multiple middleware', async () => { - const middleware1 = jest.fn(({ next }) => next()); - const middleware2 = jest.fn(() => null); + const middleware1: JsonRpcMiddleware = jest.fn(({ next }) => next()); + const middleware2: JsonRpcMiddleware = jest.fn(() => null); const engine = JsonRpcEngineV2.create({ middleware: [middleware1, middleware2], }); @@ -264,10 +271,10 @@ describe('JsonRpcEngineV2', () => { describe('context', () => { it('passes the context to the middleware', async () => { - const middleware = jest.fn(({ context }) => { + const middleware: JsonRpcMiddleware = ({ context }) => { expect(context).toBeInstanceOf(Map); return null; - }); + }; const engine = JsonRpcEngineV2.create({ middleware: [middleware], }); @@ -276,13 +283,22 @@ describe('JsonRpcEngineV2', () => { }); it('propagates context changes to subsequent middleware', async () => { - const middleware1 = jest.fn(async ({ context, next }) => { + type Context = MiddlewareContext<{ foo: string }>; + const middleware1: JsonRpcMiddleware< + JsonRpcCall, + Json | void, + Context + > = async ({ context, next }) => { context.set('foo', 'bar'); return next(); - }); - const middleware2 = jest.fn(({ context }) => { + }; + const middleware2: JsonRpcMiddleware< + JsonRpcCall, + string | undefined, + Context + > = ({ context }) => { return context.get('foo') as string | undefined; - }); + }; const engine = JsonRpcEngineV2.create({ middleware: [middleware1, middleware2], }); @@ -300,7 +316,7 @@ describe('JsonRpcEngineV2', () => { string, MiddlewareContext> > = ({ context }) => context.assertGet('foo'); - const engine = JsonRpcEngineV2.create({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware], }); @@ -365,13 +381,12 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware attempts to modify properties of the context', async () => { + const middleware: JsonRpcMiddleware = ({ context }) => { + // @ts-expect-error - Destructive testing. + context.set = () => undefined; + }; const engine = JsonRpcEngineV2.create({ - middleware: [ - ({ context }) => { - // @ts-expect-error - Destructive testing. - context.set = () => undefined; - }, - ], + middleware: [middleware], }); await expect(engine.handle(makeRequest())).rejects.toThrow( @@ -421,7 +436,7 @@ describe('JsonRpcEngineV2', () => { return [...nums, 3]; }); - const engine = JsonRpcEngineV2.create>({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware1, middleware2, middleware3], }); @@ -495,7 +510,7 @@ describe('JsonRpcEngineV2', () => { id: '2', }); }), - jest.fn(() => null), + makeNullMiddleware(), ], }); const request = makeRequest(); @@ -508,17 +523,16 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware attempts to modify the request "jsonrpc" property', async () => { + const middleware1: JsonRpcMiddleware = async ({ request, next }) => { + return await next({ + ...request, + // @ts-expect-error - Destructive testing. + jsonrpc: '3.0', + }); + }; + const middleware2: JsonRpcMiddleware = () => null; const engine = JsonRpcEngineV2.create({ - middleware: [ - async ({ request, next }) => { - return await next({ - ...request, - // @ts-expect-error - Destructive testing. - jsonrpc: '3.0', - }); - }, - () => null, - ], + middleware: [middleware1, middleware2], }); const request = makeRequest(); @@ -539,7 +553,7 @@ describe('JsonRpcEngineV2', () => { return result + 1; }; const middleware2: JsonRpcMiddleware = () => 1; - const engine = JsonRpcEngineV2.create({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware, middleware2], }); @@ -551,11 +565,11 @@ describe('JsonRpcEngineV2', () => { it('updates an undefined result with a new value', async () => { const engine = JsonRpcEngineV2.create({ middleware: [ - jest.fn(async ({ next }) => { + async ({ next }) => { await next(); return null; - }), - jest.fn(() => undefined), + }, + makeVoidMiddleware(), ], }); @@ -570,7 +584,7 @@ describe('JsonRpcEngineV2', () => { jest.fn(async ({ next }) => { await next(); }), - jest.fn(() => null), + makeNullMiddleware(), ], }); @@ -617,9 +631,13 @@ describe('JsonRpcEngineV2', () => { await next(); returnHandlerResults.push(3); }); - const middleware4 = jest.fn(() => null); const engine = JsonRpcEngineV2.create({ - middleware: [middleware1, middleware2, middleware3, middleware4], + middleware: [ + middleware1, + middleware2, + middleware3, + makeNullMiddleware(), + ], }); await engine.handle(makeRequest()); @@ -704,25 +722,33 @@ describe('JsonRpcEngineV2', () => { let inFlight = 0; let maxInFlight = 0; - const engine = JsonRpcEngineV2.create({ - middleware: [ - async ({ context, next, request }) => { - // eslint-disable-next-line jest/no-conditional-in-test - context.set('id', context.get('id') ?? request.id); + type Context = MiddlewareContext<{ id: JsonRpcId }>; + const inflightMiddleware: JsonRpcMiddleware< + JsonRpcRequest, + Json, + Context + > = async ({ context, next, request }) => { + // eslint-disable-next-line jest/no-conditional-in-test + context.set('id', context.get('id') ?? request.id); - inFlight += 1; - maxInFlight = Math.max(maxInFlight, inFlight); - latch.increment(); + inFlight += 1; + maxInFlight = Math.max(maxInFlight, inFlight); + latch.increment(); - await gate; + await gate; - inFlight -= 1; - return next(); - }, - ({ context, request }) => { - return `result:${request.id}:${context.get('id') as JsonRpcId}`; - }, - ], + inFlight -= 1; + return next(); + }; + const resultMiddleware: JsonRpcMiddleware< + JsonRpcRequest, + string, + Context + > = ({ context, request }) => { + return `result:${request.id}:${context.assertGet('id')}`; + }; + const engine = JsonRpcEngineV2.create({ + middleware: [inflightMiddleware, resultMiddleware], }); // eslint-disable-next-line @typescript-eslint/ban-ts-comment @@ -757,7 +783,7 @@ describe('JsonRpcEngineV2', () => { await queue.enqueue(request.id); return null; }; - const engine = JsonRpcEngineV2.create({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware], }); @@ -780,9 +806,8 @@ describe('JsonRpcEngineV2', () => { describe('composition', () => { describe('asMiddleware', () => { it('ends a request if it returns a value', async () => { - // TODO: We may have to do a lot of these casts? const engine1 = JsonRpcEngineV2.create({ - middleware: [() => null], + middleware: [makeNullMiddleware()], }); const engine2 = JsonRpcEngineV2.create({ middleware: [engine1.asMiddleware(), jest.fn(() => 'foo')], @@ -795,10 +820,10 @@ describe('JsonRpcEngineV2', () => { it('permits returning undefined if a later middleware ends the request', async () => { const engine1 = JsonRpcEngineV2.create({ - middleware: [() => undefined], + middleware: [makeVoidMiddleware()], }); const engine2 = JsonRpcEngineV2.create({ - middleware: [engine1.asMiddleware(), () => null], + middleware: [engine1.asMiddleware(), makeNullMiddleware()], }); const result = await engine2.handle(makeRequest()); @@ -827,7 +852,7 @@ describe('JsonRpcEngineV2', () => { }); it('propagates request mutation', async () => { - const engine1 = JsonRpcEngineV2.create>({ + const engine1 = JsonRpcEngineV2.create({ middleware: [ ({ request, next }) => { return next({ @@ -848,7 +873,7 @@ describe('JsonRpcEngineV2', () => { }); let observedMethod: string | undefined; - const engine2 = JsonRpcEngineV2.create>({ + const engine2 = JsonRpcEngineV2.create({ middleware: [ engine1.asMiddleware(), ({ request }) => { @@ -944,18 +969,23 @@ describe('JsonRpcEngineV2', () => { it('composes nested engines', async () => { const earlierMiddleware = jest.fn(async ({ next }) => next()); - const middleware: JsonRpcMiddleware = () => null; + const engine1Middleware: JsonRpcMiddleware = () => null; const engine1 = JsonRpcEngineV2.create({ - middleware: [middleware], + middleware: [engine1Middleware], }); - const laterMiddleware = jest.fn(() => 'foo'); + const engine1ProxyMiddleware: JsonRpcMiddleware< + JsonRpcRequest + > = async ({ request }) => { + return engine1.handle(request); + }; + const laterMiddleware: JsonRpcMiddleware = jest.fn( + () => 'foo', + ); const engine2 = JsonRpcEngineV2.create({ middleware: [ earlierMiddleware, - async ({ request }) => { - return engine1.handle(request as JsonRpcRequest); - }, + engine1ProxyMiddleware, laterMiddleware, ], }); @@ -970,7 +1000,7 @@ describe('JsonRpcEngineV2', () => { it('does not propagate request mutation', async () => { // Unlike asMiddleware(), although the inner engine mutates request, // those mutations do not propagate when using engine.handle(). - const engine1 = JsonRpcEngineV2.create>({ + const engine1 = JsonRpcEngineV2.create({ middleware: [ ({ request, next }) => { return next({ @@ -987,24 +1017,31 @@ describe('JsonRpcEngineV2', () => { params: [request.params![0]! * 2], }); }, - () => null, + makeNullMiddleware(), ], }); + const engine1ProxyMiddleware: JsonRpcMiddleware = async ({ + request, + next, + context, + }) => { + await engine1.handle(request, { context }); + return next(); + }; + let observedMethod: string | undefined; - const engine2 = JsonRpcEngineV2.create>({ - middleware: [ - async ({ request, next, context }) => { - await engine1.handle(request, { context }); - return next(); - }, - ({ request }) => { - observedMethod = request.method; - // Obviously correct. - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return request.params![0]! * 2; - }, - ], + const observedMethodMiddleware: JsonRpcMiddleware< + JsonRpcRequest, + number + > = ({ request }) => { + observedMethod = request.method; + // Obviously correct. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return request.params![0]! * 2; + }; + const engine2 = JsonRpcEngineV2.create({ + middleware: [engine1ProxyMiddleware, observedMethodMiddleware], }); const result = await engine2.handle(makeRequest({ params: [1] })); @@ -1061,7 +1098,7 @@ describe('JsonRpcEngineV2', () => { await next(); returnHandlerResults.push('1:b'); }, - () => null, + makeNullMiddleware(), ], }); @@ -1079,7 +1116,7 @@ describe('JsonRpcEngineV2', () => { await next(); returnHandlerResults.push('2:b'); }, - () => null, + makeNullMiddleware(), ], }); @@ -1122,7 +1159,7 @@ describe('JsonRpcEngineV2', () => { describe('request- and notification-only engines', () => { it('constructs a request-only engine', async () => { const middleware: JsonRpcMiddleware = () => null; - const engine = JsonRpcEngineV2.create({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware], }); @@ -1138,8 +1175,8 @@ describe('JsonRpcEngineV2', () => { }); it('constructs a notification-only engine', async () => { - const engine = JsonRpcEngineV2.create({ - middleware: [() => undefined], + const engine = JsonRpcEngineV2.create({ + middleware: [makeVoidMiddleware()], }); expect(await engine.handle(makeNotification())).toBeUndefined(); @@ -1162,11 +1199,12 @@ describe('JsonRpcEngineV2', () => { }); it('constructs a mixed engine', async () => { + const mixedMiddleware: JsonRpcMiddleware = ({ request }) => { + // eslint-disable-next-line jest/no-conditional-in-test + return isRequest(request) ? null : undefined; + }; const engine = JsonRpcEngineV2.create({ - middleware: [ - // eslint-disable-next-line jest/no-conditional-in-test - ({ request }) => (isRequest(request) ? null : undefined), - ], + middleware: [mixedMiddleware], }); expect(await engine.handle(makeRequest())).toBeNull(); @@ -1176,11 +1214,11 @@ describe('JsonRpcEngineV2', () => { it('composes a pipeline of request- and notification-only engines', async () => { const requestEngine = JsonRpcEngineV2.create({ - middleware: [() => null], + middleware: [makeNullMiddleware()], }); const notificationEngine = JsonRpcEngineV2.create({ - middleware: [() => undefined], + middleware: [makeVoidMiddleware()], }); const orchestratorEngine = JsonRpcEngineV2.create({ @@ -1235,7 +1273,7 @@ describe('JsonRpcEngineV2', () => { it('causes handle() to throw after destroying the engine', async () => { const engine = JsonRpcEngineV2.create({ - middleware: [() => null], + middleware: [makeNullMiddleware()], }); await engine.destroy(); @@ -1247,7 +1285,7 @@ describe('JsonRpcEngineV2', () => { it('causes asMiddleware() to throw after destroying the engine', async () => { const engine = JsonRpcEngineV2.create({ - middleware: [() => null], + middleware: [makeNullMiddleware()], }); await engine.destroy(); diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts index b59cac72f0e..4f29a98efd9 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts @@ -50,7 +50,9 @@ export type MiddlewareParams< export type JsonRpcMiddleware< Request extends JsonRpcCall = JsonRpcCall, Result extends ResultConstraint = ResultConstraint, - Context extends ContextConstraint = MiddlewareContext, + Context extends ContextConstraint = MiddlewareContext< + Record + >, > = ( params: MiddlewareParams, ) => Readonly | undefined | Promise | undefined>; @@ -73,6 +75,13 @@ type ConstructorOptions< >; }; +type RequestOf = + // Non-polluting `any` constraint. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + Middleware extends JsonRpcMiddleware + ? Request + : never; + type ContextOf = // Non-polluting `any` constraint. // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -86,11 +95,6 @@ type MergedContextOf< Middleware extends JsonRpcMiddleware, > = MergeContexts>; -type MiddlewareConstraint< - Request extends JsonRpcCall, - Context extends ContextConstraint, -> = JsonRpcMiddleware, Context>; - /** * A JSON-RPC request and response processor. * @@ -114,7 +118,7 @@ type MiddlewareConstraint< * * @example * ```ts - * const engine = new JsonRpcEngineV2({ + * const engine = JsonRpcEngineV2.create({ * middleware, * }); * @@ -154,20 +158,20 @@ export class JsonRpcEngineV2< * @param options.middleware - The middleware to use. * @returns The JSON-RPC engine. */ - static create< - InputRequest extends JsonRpcCall = JsonRpcCall, - InputContext extends ContextConstraint = ContextConstraint, - Middleware extends MiddlewareConstraint< - InputRequest, - InputContext - > = MiddlewareConstraint, - >({ middleware }: { middleware: Middleware[] }) { + // Non-polluting `any` constraint. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + static create>({ + middleware, + }: { + middleware: Middleware[]; + }) { // We can't use NonEmptyArray for the params because it ruins type inference. if (middleware.length === 0) { throw new JsonRpcEngineError('Middleware array cannot be empty'); } type MergedContext = MergedContextOf; + type InputRequest = RequestOf; const mw = middleware as unknown as NonEmptyArray< JsonRpcMiddleware< InputRequest, diff --git a/packages/json-rpc-engine/src/v2/JsonRpcServer.test.ts b/packages/json-rpc-engine/src/v2/JsonRpcServer.test.ts index c337b842b1c..ae17101cd75 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcServer.test.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcServer.test.ts @@ -1,5 +1,6 @@ import { rpcErrors } from '@metamask/rpc-errors'; +import type { JsonRpcMiddleware } from './JsonRpcEngineV2'; import { JsonRpcEngineV2 } from './JsonRpcEngineV2'; import { JsonRpcServer } from './JsonRpcServer'; import { isRequest } from './utils'; @@ -7,15 +8,14 @@ import { isRequest } from './utils'; const jsonrpc = '2.0' as const; const makeEngine = () => { + const middleware: JsonRpcMiddleware = ({ request }) => { + if (request.method !== 'hello') { + throw new Error('Unknown method'); + } + return isRequest(request) ? (request.params ?? null) : undefined; + }; return JsonRpcEngineV2.create({ - middleware: [ - ({ request }) => { - if (request.method !== 'hello') { - throw new Error('Unknown method'); - } - return isRequest(request) ? (request.params ?? null) : undefined; - }, - ], + middleware: [middleware], }); }; diff --git a/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts b/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts index 03f047d740b..a136d0dd6e8 100644 --- a/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts +++ b/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts @@ -11,18 +11,22 @@ import type { MiddlewareContext } from './MiddlewareContext'; import { getExtraneousKeys, makeRequest } from '../../tests/utils'; import { JsonRpcEngine } from '../JsonRpcEngine'; +const makeNullMiddleware = (): JsonRpcMiddleware => { + return () => null; +}; + describe('asLegacyMiddleware', () => { it('converts a v2 engine to a legacy middleware', () => { - const engine = JsonRpcEngineV2.create({ - middleware: [() => null], + const engine = JsonRpcEngineV2.create({ + middleware: [makeNullMiddleware()], }); const middleware = asLegacyMiddleware(engine); expect(typeof middleware).toBe('function'); }); it('forwards a result to the legacy engine', async () => { - const v2Engine = JsonRpcEngineV2.create({ - middleware: [() => null], + const v2Engine = JsonRpcEngineV2.create({ + middleware: [makeNullMiddleware()], }); const legacyEngine = new JsonRpcEngine(); @@ -36,8 +40,9 @@ describe('asLegacyMiddleware', () => { }); it('forwarded results are not frozen', async () => { - const v2Engine = JsonRpcEngineV2.create({ - middleware: [() => []], + const v2Middleware: JsonRpcMiddleware = () => []; + const v2Engine = JsonRpcEngineV2.create({ + middleware: [v2Middleware], }); const legacyEngine = new JsonRpcEngine(); @@ -52,12 +57,11 @@ describe('asLegacyMiddleware', () => { }); it('forwards an error to the legacy engine', async () => { - const v2Engine = JsonRpcEngineV2.create({ - middleware: [ - () => { - throw new Error('test'); - }, - ], + const v2Middleware: JsonRpcMiddleware = () => { + throw new Error('test'); + }; + const v2Engine = JsonRpcEngineV2.create({ + middleware: [v2Middleware], }); const legacyEngine = new JsonRpcEngine(); @@ -80,8 +84,10 @@ describe('asLegacyMiddleware', () => { }); it('allows the legacy engine to continue when not ending the request', async () => { - const v2Middleware = jest.fn(({ next }) => next()); - const v2Engine = JsonRpcEngineV2.create({ + const v2Middleware: JsonRpcMiddleware = jest.fn( + ({ next }) => next(), + ); + const v2Engine = JsonRpcEngineV2.create({ middleware: [v2Middleware], }); @@ -100,8 +106,10 @@ describe('asLegacyMiddleware', () => { }); it('allows the legacy engine to continue when not ending the request (passing through the original request)', async () => { - const v2Middleware = jest.fn(({ request, next }) => next(request)); - const v2Engine = JsonRpcEngineV2.create({ + const v2Middleware: JsonRpcMiddleware = jest.fn( + ({ request, next }) => next(request), + ); + const v2Engine = JsonRpcEngineV2.create({ middleware: [v2Middleware], }); @@ -120,7 +128,7 @@ describe('asLegacyMiddleware', () => { }); it('propagates request modifications to the legacy engine', async () => { - const v2Engine = JsonRpcEngineV2.create({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [ ({ request, next }) => next({ ...request, method: 'test_request_2' }), ], @@ -160,7 +168,7 @@ describe('asLegacyMiddleware', () => { MiddlewareContext> >); - const v2Engine = JsonRpcEngineV2.create({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [v2Middleware], }); From c81491064c1f2383be421e326b5d337366de598c Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Mon, 20 Oct 2025 20:28:29 -0700 Subject: [PATCH 10/18] refactor: Add default parameter to create() --- .../src/v2/JsonRpcEngineV2.test.ts | 67 ++++++++----------- .../json-rpc-engine/src/v2/JsonRpcEngineV2.ts | 12 ++-- .../src/v2/asLegacyMiddleware.test.ts | 16 ++--- packages/json-rpc-engine/src/v2/index.ts | 1 + packages/json-rpc-engine/tests/utils.ts | 29 ++++++++ 5 files changed, 70 insertions(+), 55 deletions(-) diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts index 86743a3ed01..7be4832f6cf 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts @@ -13,18 +13,16 @@ import { type JsonRpcNotification, type JsonRpcRequest, } from './utils'; -import { makeNotification, makeRequest } from '../../tests/utils'; +import { + makeNotification, + makeNotificationMiddleware, + makeNullMiddleware, + makeRequest, + makeRequestMiddleware, +} from '../../tests/utils'; const jsonrpc = '2.0' as const; -const makeNullMiddleware = (): JsonRpcMiddleware => { - return () => null; -}; - -const makeVoidMiddleware = (): JsonRpcMiddleware => { - return () => undefined; -}; - describe('JsonRpcEngineV2', () => { describe('create', () => { it('throws if the middleware array is empty', () => { @@ -165,8 +163,8 @@ describe('JsonRpcEngineV2', () => { describe('requests', () => { it('returns a result from the middleware', async () => { - const middleware: JsonRpcMiddleware = jest.fn(() => null); - const engine = JsonRpcEngineV2.create({ + const middleware = jest.fn(() => null); + const engine = JsonRpcEngineV2.create({ middleware: [middleware], }); const request = makeRequest(); @@ -569,7 +567,7 @@ describe('JsonRpcEngineV2', () => { await next(); return null; }, - makeVoidMiddleware(), + makeNotificationMiddleware(), ], }); @@ -820,7 +818,7 @@ describe('JsonRpcEngineV2', () => { it('permits returning undefined if a later middleware ends the request', async () => { const engine1 = JsonRpcEngineV2.create({ - middleware: [makeVoidMiddleware()], + middleware: [makeNotificationMiddleware()], }); const engine2 = JsonRpcEngineV2.create({ middleware: [engine1.asMiddleware(), makeNullMiddleware()], @@ -864,9 +862,7 @@ describe('JsonRpcEngineV2', () => { return next({ ...request, method: 'test_request_2', - // Obviously correct. - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - params: [request.params![0]! * 2], + params: [(request.params as [number])[0] * 2], }); }, ], @@ -878,9 +874,7 @@ describe('JsonRpcEngineV2', () => { engine1.asMiddleware(), ({ request }) => { observedMethod = request.method; - // Obviously correct. - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return request.params![0]! * 2; + return (request.params as [number])[0] * 2; }, ], }); @@ -895,9 +889,8 @@ describe('JsonRpcEngineV2', () => { const engine1 = JsonRpcEngineV2.create({ middleware: [ async ({ context, next }) => { - const nums = context.assertGet('foo'); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - nums[0]! *= 2; + const nums = context.assertGet('foo') as [number]; + nums[0] *= 2; return next(); }, ], @@ -911,8 +904,8 @@ describe('JsonRpcEngineV2', () => { }, engine1.asMiddleware(), async ({ context }) => { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return context.assertGet('foo')[0]! * 2; + const nums = context.assertGet('foo') as [number]; + return nums[0] * 2; }, ], }); @@ -1012,9 +1005,7 @@ describe('JsonRpcEngineV2', () => { return next({ ...request, method: 'test_request_2', - // Obviously correct. - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - params: [request.params![0]! * 2], + params: [(request.params as [number])[0] * 2], }); }, makeNullMiddleware(), @@ -1036,9 +1027,7 @@ describe('JsonRpcEngineV2', () => { number > = ({ request }) => { observedMethod = request.method; - // Obviously correct. - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return request.params![0]! * 2; + return (request.params as [number])[0] * 2; }; const engine2 = JsonRpcEngineV2.create({ middleware: [engine1ProxyMiddleware, observedMethodMiddleware], @@ -1056,9 +1045,8 @@ describe('JsonRpcEngineV2', () => { const engine1 = JsonRpcEngineV2.create({ middleware: [ async ({ context }) => { - const nums = context.assertGet('foo'); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - nums[0]! *= 2; + const nums = context.assertGet('foo') as [number]; + nums[0] *= 2; return null; }, ], @@ -1075,8 +1063,8 @@ describe('JsonRpcEngineV2', () => { return next(); }, async ({ context }) => { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return context.assertGet('foo')[0]! * 2; + const nums = context.assertGet('foo') as [number]; + return nums[0] * 2; }, ], }); @@ -1158,9 +1146,8 @@ describe('JsonRpcEngineV2', () => { describe('request- and notification-only engines', () => { it('constructs a request-only engine', async () => { - const middleware: JsonRpcMiddleware = () => null; const engine = JsonRpcEngineV2.create({ - middleware: [middleware], + middleware: [makeRequestMiddleware()], }); expect(await engine.handle(makeRequest())).toBeNull(); @@ -1176,7 +1163,7 @@ describe('JsonRpcEngineV2', () => { it('constructs a notification-only engine', async () => { const engine = JsonRpcEngineV2.create({ - middleware: [makeVoidMiddleware()], + middleware: [makeNotificationMiddleware()], }); expect(await engine.handle(makeNotification())).toBeUndefined(); @@ -1214,11 +1201,11 @@ describe('JsonRpcEngineV2', () => { it('composes a pipeline of request- and notification-only engines', async () => { const requestEngine = JsonRpcEngineV2.create({ - middleware: [makeNullMiddleware()], + middleware: [makeRequestMiddleware()], }); const notificationEngine = JsonRpcEngineV2.create({ - middleware: [makeVoidMiddleware()], + middleware: [makeNotificationMiddleware()], }); const orchestratorEngine = JsonRpcEngineV2.create({ diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts index 4f29a98efd9..7ae6e9b696a 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts @@ -158,13 +158,11 @@ export class JsonRpcEngineV2< * @param options.middleware - The middleware to use. * @returns The JSON-RPC engine. */ - // Non-polluting `any` constraint. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - static create>({ - middleware, - }: { - middleware: Middleware[]; - }) { + static create< + // Non-polluting `any` constraint. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + Middleware extends JsonRpcMiddleware = JsonRpcMiddleware, + >({ middleware }: { middleware: Middleware[] }) { // We can't use NonEmptyArray for the params because it ruins type inference. if (middleware.length === 0) { throw new JsonRpcEngineError('Middleware array cannot be empty'); diff --git a/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts b/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts index a136d0dd6e8..0acf60b6956 100644 --- a/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts +++ b/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts @@ -8,17 +8,17 @@ import { asLegacyMiddleware } from './asLegacyMiddleware'; import type { JsonRpcMiddleware, ResultConstraint } from './JsonRpcEngineV2'; import { JsonRpcEngineV2 } from './JsonRpcEngineV2'; import type { MiddlewareContext } from './MiddlewareContext'; -import { getExtraneousKeys, makeRequest } from '../../tests/utils'; +import { + getExtraneousKeys, + makeRequest, + makeRequestMiddleware, +} from '../../tests/utils'; import { JsonRpcEngine } from '../JsonRpcEngine'; -const makeNullMiddleware = (): JsonRpcMiddleware => { - return () => null; -}; - describe('asLegacyMiddleware', () => { it('converts a v2 engine to a legacy middleware', () => { const engine = JsonRpcEngineV2.create({ - middleware: [makeNullMiddleware()], + middleware: [makeRequestMiddleware()], }); const middleware = asLegacyMiddleware(engine); expect(typeof middleware).toBe('function'); @@ -26,7 +26,7 @@ describe('asLegacyMiddleware', () => { it('forwards a result to the legacy engine', async () => { const v2Engine = JsonRpcEngineV2.create({ - middleware: [makeNullMiddleware()], + middleware: [makeRequestMiddleware()], }); const legacyEngine = new JsonRpcEngine(); @@ -128,7 +128,7 @@ describe('asLegacyMiddleware', () => { }); it('propagates request modifications to the legacy engine', async () => { - const v2Engine = JsonRpcEngineV2.create({ + const v2Engine = JsonRpcEngineV2.create>({ middleware: [ ({ request, next }) => next({ ...request, method: 'test_request_2' }), ], diff --git a/packages/json-rpc-engine/src/v2/index.ts b/packages/json-rpc-engine/src/v2/index.ts index e9298e2ae67..64e6afae888 100644 --- a/packages/json-rpc-engine/src/v2/index.ts +++ b/packages/json-rpc-engine/src/v2/index.ts @@ -1,3 +1,4 @@ +export type { Json } from '@metamask/utils'; export { asLegacyMiddleware } from './asLegacyMiddleware'; export { getUniqueId } from '../getUniqueId'; export * from './JsonRpcEngineV2'; diff --git a/packages/json-rpc-engine/tests/utils.ts b/packages/json-rpc-engine/tests/utils.ts index 03f3d748e0c..2ed886379a9 100644 --- a/packages/json-rpc-engine/tests/utils.ts +++ b/packages/json-rpc-engine/tests/utils.ts @@ -1,4 +1,5 @@ import type { JsonRpcRequest } from '@metamask/utils'; +import type { JsonRpcMiddleware } from 'src/v2/JsonRpcEngineV2'; import { requestProps } from '../src/v2/compatibility-utils'; import type { JsonRpcNotification } from '../src/v2/utils'; @@ -24,6 +25,34 @@ export const makeNotification = >( ...params, }) as JsonRpcNotification; +/** + * Creates a {@link JsonRpcCall} middleware that returns `null`. + * + * @returns The middleware. + */ +export const makeNullMiddleware = (): JsonRpcMiddleware => { + return () => null; +}; + +/** + * Creates a {@link JsonRpcRequest} middleware that returns `null`. + * + * @returns The middleware. + */ +export const makeRequestMiddleware = (): JsonRpcMiddleware => { + return () => null; +}; + +/** + * Creates a {@link JsonRpcNotification} middleware that returns `undefined`. + * + * @returns The middleware. + */ +export const makeNotificationMiddleware = + (): JsonRpcMiddleware => { + return () => undefined; + }; + /** * Get the keys of a request that are not part of the standard JSON-RPC request * properties. From 2cd94497c061b0b72130781bf384a697e24b42bc Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Tue, 21 Oct 2025 10:47:25 -0700 Subject: [PATCH 11/18] refactor: Simplify types in tests, update default context type --- packages/json-rpc-engine/README.md | 39 ++-- .../src/asV2Middleware.test.ts | 9 +- .../src/v2/JsonRpcEngineV2.test.ts | 185 +++++++++--------- .../json-rpc-engine/src/v2/JsonRpcEngineV2.ts | 12 +- .../src/v2/JsonRpcServer.test.ts | 17 +- .../src/v2/MiddlewareContext.ts | 6 +- .../src/v2/asLegacyMiddleware.test.ts | 6 +- 7 files changed, 140 insertions(+), 134 deletions(-) diff --git a/packages/json-rpc-engine/README.md b/packages/json-rpc-engine/README.md index 1938d2fea4c..ed045ac91a3 100644 --- a/packages/json-rpc-engine/README.md +++ b/packages/json-rpc-engine/README.md @@ -17,21 +17,32 @@ or ```ts import { JsonRpcEngineV2 } from '@metamask/json-rpc-engine/v2'; +import type { + Json, + JsonRpcMiddleware, + MiddlewareContext, +} from '@metamask/json-rpc-engine/v2'; -// You must use the static factory method `create()` instead of the constructor. -const engine = JsonRpcEngineV2.create({ - // Create a stack of middleware and pass it to the engine: - middleware: [ - ({ request, next, context }) => { - if (request.method === 'hello') { - context.set('hello', 'world'); - return next(); - } - return null; - }, - ({ context }) => context.get('hello'), - ], -}); +// Create a stack of middleware. You must explicitly type your middleware +// functions. +const middleware: JsonRpcMiddleware< + JsonRpcRequest, + Json, + MiddlewareContext<{ hello: string }> +>[] = [ + ({ request, next, context }) => { + if (request.method === 'hello') { + context.set('hello', 'world'); + return next(); + } + return null; + }, + ({ context }) => context.get('hello'), +]; + +// Pass your middleware to the engine. You must use the static factory method +// `create()` instead of the constructor. +const engine = JsonRpcEngineV2.create({ middleware }); ``` Requests are handled asynchronously, stepping down the middleware stack until complete. diff --git a/packages/json-rpc-engine/src/asV2Middleware.test.ts b/packages/json-rpc-engine/src/asV2Middleware.test.ts index 774e14ad858..81d06190e93 100644 --- a/packages/json-rpc-engine/src/asV2Middleware.test.ts +++ b/packages/json-rpc-engine/src/asV2Middleware.test.ts @@ -6,7 +6,11 @@ import { asV2Middleware } from './asV2Middleware'; import { JsonRpcEngineV2 } from './v2/JsonRpcEngineV2'; import type { JsonRpcMiddleware as V2Middleware } from './v2/JsonRpcEngineV2'; import type { MiddlewareContext } from './v2/MiddlewareContext'; -import { getExtraneousKeys, makeRequest } from '../tests/utils'; +import { + getExtraneousKeys, + makeNullMiddleware, + makeRequest, +} from '../tests/utils'; describe('asV2Middleware', () => { it('converts a legacy engine to a v2 middleware', () => { @@ -69,9 +73,8 @@ describe('asV2Middleware', () => { }); legacyEngine.push(legacyMiddleware); - const v2Middleware: V2Middleware = () => null; const v2Engine = JsonRpcEngineV2.create({ - middleware: [asV2Middleware(legacyEngine), v2Middleware], + middleware: [asV2Middleware(legacyEngine), makeNullMiddleware()], }); const result = await v2Engine.handle(makeRequest()); diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts index 7be4832f6cf..da283573a25 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts @@ -1,5 +1,5 @@ /* eslint-disable n/callback-return */ // next() is not a Node.js callback. -import type { Json, JsonRpcId, NonEmptyArray } from '@metamask/utils'; +import type { Json, JsonRpcId } from '@metamask/utils'; import { createDeferredPromise } from '@metamask/utils'; import type { JsonRpcMiddleware, ResultConstraint } from './JsonRpcEngineV2'; @@ -109,9 +109,8 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a result is returned, from the first middleware', async () => { - const middleware: JsonRpcMiddleware = () => 'foo'; - const engine = JsonRpcEngineV2.create({ - middleware: [middleware], + const engine = JsonRpcEngineV2.create({ + middleware: [() => 'foo'], }); const notification = { jsonrpc, method: 'test_request' }; @@ -254,7 +253,7 @@ describe('JsonRpcEngineV2', () => { await next(); await next(); }), - jest.fn(), + makeNullMiddleware(), ], }); const request = makeRequest(); @@ -269,12 +268,15 @@ describe('JsonRpcEngineV2', () => { describe('context', () => { it('passes the context to the middleware', async () => { - const middleware: JsonRpcMiddleware = ({ context }) => { - expect(context).toBeInstanceOf(Map); - return null; - }; - const engine = JsonRpcEngineV2.create({ - middleware: [middleware], + const engine = JsonRpcEngineV2.create< + JsonRpcMiddleware + >({ + middleware: [ + ({ context }) => { + expect(context).toBeInstanceOf(Map); + return null; + }, + ], }); await engine.handle(makeRequest()); @@ -379,12 +381,13 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware attempts to modify properties of the context', async () => { - const middleware: JsonRpcMiddleware = ({ context }) => { - // @ts-expect-error - Destructive testing. - context.set = () => undefined; - }; - const engine = JsonRpcEngineV2.create({ - middleware: [middleware], + const engine = JsonRpcEngineV2.create({ + middleware: [ + ({ context }) => { + // @ts-expect-error - Destructive testing. + context.set = () => undefined; + }, + ], }); await expect(engine.handle(makeRequest())).rejects.toThrow( @@ -395,9 +398,8 @@ describe('JsonRpcEngineV2', () => { describe('asynchrony', () => { it('handles asynchronous middleware', async () => { - const middleware = jest.fn(async () => null); const engine = JsonRpcEngineV2.create({ - middleware: [middleware], + middleware: [async () => null], }); const result = await engine.handle(makeRequest()); @@ -406,36 +408,28 @@ describe('JsonRpcEngineV2', () => { }); it('handles mixed synchronous and asynchronous middleware', async () => { - const middleware1: JsonRpcMiddleware< + type Middleware = JsonRpcMiddleware< JsonRpcRequest, Json, MiddlewareContext> - > = jest.fn(async ({ context, next }) => { - context.set('foo', [1]); - return next(); - }); - - const middleware2: JsonRpcMiddleware< - JsonRpcRequest, - Json, - MiddlewareContext> - > = jest.fn(({ context, next }) => { - const nums = context.assertGet('foo'); - nums.push(2); - return next(); - }); + >; - const middleware3: JsonRpcMiddleware< - JsonRpcRequest, - number[], - MiddlewareContext> - > = jest.fn(async ({ context }) => { - const nums = context.assertGet('foo'); - return [...nums, 3]; - }); - - const engine = JsonRpcEngineV2.create({ - middleware: [middleware1, middleware2, middleware3], + const engine = JsonRpcEngineV2.create({ + middleware: [ + async ({ context, next }) => { + context.set('foo', [1]); + return next(); + }, + ({ context, next }) => { + const nums = context.assertGet('foo'); + nums.push(2); + return next(); + }, + async ({ context }) => { + const nums = context.assertGet('foo'); + return [...nums, 3]; + }, + ], }); const result = await engine.handle(makeRequest()); @@ -521,16 +515,17 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware attempts to modify the request "jsonrpc" property', async () => { - const middleware1: JsonRpcMiddleware = async ({ request, next }) => { - return await next({ - ...request, - // @ts-expect-error - Destructive testing. - jsonrpc: '3.0', - }); - }; - const middleware2: JsonRpcMiddleware = () => null; const engine = JsonRpcEngineV2.create({ - middleware: [middleware1, middleware2], + middleware: [ + async ({ request, next }) => { + return await next({ + ...request, + // @ts-expect-error - Destructive testing. + jsonrpc: '3.0', + }); + }, + makeNullMiddleware(), + ], }); const request = makeRequest(); @@ -544,15 +539,16 @@ describe('JsonRpcEngineV2', () => { describe('result handling', () => { it('updates the result after next() is called', async () => { - const middleware: JsonRpcMiddleware = async ({ - next, - }) => { - const result = (await next()) as number; - return result + 1; - }; - const middleware2: JsonRpcMiddleware = () => 1; - const engine = JsonRpcEngineV2.create({ - middleware: [middleware, middleware2], + const engine = JsonRpcEngineV2.create< + JsonRpcMiddleware + >({ + middleware: [ + async ({ next }) => { + const result = (await next()) as number; + return result + 1; + }, + () => 1, + ], }); const result = await engine.handle(makeRequest()); @@ -617,23 +613,20 @@ describe('JsonRpcEngineV2', () => { it('handles returned results in reverse middleware order', async () => { const returnHandlerResults: number[] = []; - const middleware1 = jest.fn(async ({ next }) => { - await next(); - returnHandlerResults.push(1); - }); - const middleware2 = jest.fn(async ({ next }) => { - await next(); - returnHandlerResults.push(2); - }); - const middleware3 = jest.fn(async ({ next }) => { - await next(); - returnHandlerResults.push(3); - }); const engine = JsonRpcEngineV2.create({ middleware: [ - middleware1, - middleware2, - middleware3, + async ({ next }) => { + await next(); + returnHandlerResults.push(1); + }, + async ({ next }) => { + await next(); + returnHandlerResults.push(2); + }, + async ({ next }) => { + await next(); + returnHandlerResults.push(3); + }, makeNullMiddleware(), ], }); @@ -644,14 +637,15 @@ describe('JsonRpcEngineV2', () => { }); it('throws if directly modifying the result', async () => { - const middleware1 = jest.fn(async ({ next }) => { - const result = await next(); - result.foo = 'baz'; - return result; - }); - const middleware2 = jest.fn(() => ({ foo: 'bar' })); const engine = JsonRpcEngineV2.create({ - middleware: [middleware1, middleware2], + middleware: [ + async ({ next }) => { + const result = (await next()) as { foo: string }; + result.foo = 'baz'; + return result; + }, + () => ({ foo: 'bar' }), + ], }); await expect(engine.handle(makeRequest())).rejects.toThrow( @@ -1012,15 +1006,6 @@ describe('JsonRpcEngineV2', () => { ], }); - const engine1ProxyMiddleware: JsonRpcMiddleware = async ({ - request, - next, - context, - }) => { - await engine1.handle(request, { context }); - return next(); - }; - let observedMethod: string | undefined; const observedMethodMiddleware: JsonRpcMiddleware< JsonRpcRequest, @@ -1030,7 +1015,13 @@ describe('JsonRpcEngineV2', () => { return (request.params as [number])[0] * 2; }; const engine2 = JsonRpcEngineV2.create({ - middleware: [engine1ProxyMiddleware, observedMethodMiddleware], + middleware: [ + async ({ request, next, context }) => { + await engine1.handle(request, { context }); + return next(); + }, + observedMethodMiddleware, + ], }); const result = await engine2.handle(makeRequest({ params: [1] })); @@ -1059,7 +1050,7 @@ describe('JsonRpcEngineV2', () => { return next(); }, async ({ request, next, context }) => { - await engine1.handle(request as JsonRpcRequest, { context }); + await engine1.handle(request, { context }); return next(); }, async ({ context }) => { @@ -1307,7 +1298,7 @@ describe('JsonRpcEngineV2', () => { middleware: [ middleware1, middleware2, - ] as unknown as NonEmptyArray, + ] as unknown as JsonRpcMiddleware[], }); await expect(engine.destroy()).rejects.toThrow(new Error('test')); diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts index 7ae6e9b696a..23df816b751 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts @@ -7,7 +7,11 @@ import { } from '@metamask/utils'; import deepFreeze from 'deep-freeze-strict'; -import type { ContextConstraint, MergeContexts } from './MiddlewareContext'; +import type { + ContextConstraint, + DefaultContext, + MergeContexts, +} from './MiddlewareContext'; import { MiddlewareContext } from './MiddlewareContext'; import { isNotification, @@ -50,9 +54,7 @@ export type MiddlewareParams< export type JsonRpcMiddleware< Request extends JsonRpcCall = JsonRpcCall, Result extends ResultConstraint = ResultConstraint, - Context extends ContextConstraint = MiddlewareContext< - Record - >, + Context extends ContextConstraint = DefaultContext, > = ( params: MiddlewareParams, ) => Readonly | undefined | Promise | undefined>; @@ -132,7 +134,7 @@ type MergedContextOf< */ export class JsonRpcEngineV2< Request extends JsonRpcCall = JsonRpcCall, - Context extends ContextConstraint = MiddlewareContext, + Context extends ContextConstraint = DefaultContext, > { #middleware: Readonly< NonEmptyArray< diff --git a/packages/json-rpc-engine/src/v2/JsonRpcServer.test.ts b/packages/json-rpc-engine/src/v2/JsonRpcServer.test.ts index ae17101cd75..88d90d2e7d3 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcServer.test.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcServer.test.ts @@ -8,14 +8,15 @@ import { isRequest } from './utils'; const jsonrpc = '2.0' as const; const makeEngine = () => { - const middleware: JsonRpcMiddleware = ({ request }) => { - if (request.method !== 'hello') { - throw new Error('Unknown method'); - } - return isRequest(request) ? (request.params ?? null) : undefined; - }; - return JsonRpcEngineV2.create({ - middleware: [middleware], + return JsonRpcEngineV2.create({ + middleware: [ + ({ request }) => { + if (request.method !== 'hello') { + throw new Error('Unknown method'); + } + return isRequest(request) ? (request.params ?? null) : undefined; + }, + ], }); }; diff --git a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts index cc787e8addd..d2f9dd85bda 100644 --- a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts +++ b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts @@ -9,9 +9,7 @@ * don't do that. */ export class MiddlewareContext< - // The `{}` type is not problematic in this context, it just means "no keys". - // eslint-disable-next-line @typescript-eslint/no-empty-object-type - KeyValues extends Record = {}, + KeyValues extends Record = Record, > extends Map { constructor( entries?: Iterable, @@ -128,3 +126,5 @@ export type MergeContexts = // Non-polluting `any` constraint. // eslint-disable-next-line @typescript-eslint/no-explicit-any export type ContextConstraint = MiddlewareContext; + +export type DefaultContext = MiddlewareContext; diff --git a/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts b/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts index 0acf60b6956..b7968898f62 100644 --- a/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts +++ b/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts @@ -7,7 +7,6 @@ import type { import { asLegacyMiddleware } from './asLegacyMiddleware'; import type { JsonRpcMiddleware, ResultConstraint } from './JsonRpcEngineV2'; import { JsonRpcEngineV2 } from './JsonRpcEngineV2'; -import type { MiddlewareContext } from './MiddlewareContext'; import { getExtraneousKeys, makeRequest, @@ -156,7 +155,7 @@ describe('asLegacyMiddleware', () => { const observedContextValues: number[] = []; const v2Middleware = jest.fn((({ context, next }) => { - observedContextValues.push(context.assertGet('value')); + observedContextValues.push(context.assertGet('value') as number); expect(Array.from(context.keys())).toStrictEqual(['value']); @@ -164,8 +163,7 @@ describe('asLegacyMiddleware', () => { return next(); }) satisfies JsonRpcMiddleware< JsonRpcRequest, - ResultConstraint, - MiddlewareContext> + ResultConstraint >); const v2Engine = JsonRpcEngineV2.create({ From 5bcc6b9c5ce12cc4d22d148d408789130ed3da7f Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Tue, 21 Oct 2025 11:39:52 -0700 Subject: [PATCH 12/18] refactor: Minor tweaks --- packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts | 11 ++++++++--- packages/json-rpc-engine/src/v2/JsonRpcServer.ts | 3 ++- .../json-rpc-engine/src/v2/MiddlewareContext.ts | 14 ++++++++++++++ .../src/v2/compatibility-utils.test.ts | 11 +++++++---- 4 files changed, 31 insertions(+), 8 deletions(-) diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts index 23df816b751..dc738ecb98b 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts @@ -161,9 +161,14 @@ export class JsonRpcEngineV2< * @returns The JSON-RPC engine. */ static create< - // Non-polluting `any` constraint. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - Middleware extends JsonRpcMiddleware = JsonRpcMiddleware, + Middleware extends JsonRpcMiddleware< + // Non-polluting `any` constraint. + /* eslint-disable @typescript-eslint/no-explicit-any */ + any, + ResultConstraint, + any + /* eslint-enable @typescript-eslint/no-explicit-any */ + > = JsonRpcMiddleware, >({ middleware }: { middleware: Middleware[] }) { // We can't use NonEmptyArray for the params because it ruins type inference. if (middleware.length === 0) { diff --git a/packages/json-rpc-engine/src/v2/JsonRpcServer.ts b/packages/json-rpc-engine/src/v2/JsonRpcServer.ts index e30d90b2c16..bd10a67bf4a 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcServer.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcServer.ts @@ -201,7 +201,8 @@ function isMinimalRequest(rawRequest: unknown): rawRequest is MinimalRequest { } /** - * Check if a request has valid params. + * Check if a request has valid params, i.e. an array or object. + * The contents of the params are not inspected. * * @param rawRequest - The request to check. * @returns `true` if the request has valid params, `false` otherwise. diff --git a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts index d2f9dd85bda..eb11baf031c 100644 --- a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts +++ b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts @@ -7,6 +7,20 @@ * * The override protections are circumvented when using e.g. `Reflect.set`, so * don't do that. + * + * @template KeyValues - The type of the keys and values in the context. + * @example + * // By default, the context permits any PropertyKey as a key. + * const context = new MiddlewareContext(); + * context.set('foo', 'bar'); + * context.get('foo'); // 'bar' + * context.get('fizz'); // undefined + * @example + * // By specifying an object type, the context permits only the keys of the object. + * type Context = MiddlewareContext<{ foo: string }>; + * const context = new Context([['foo', 'bar']]); + * context.get('foo'); // 'bar' + * context.get('fizz'); // Type error */ export class MiddlewareContext< KeyValues extends Record = Record, diff --git a/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts b/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts index 797d4a59675..9e75753daa3 100644 --- a/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts +++ b/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts @@ -282,16 +282,18 @@ describe('compatibility-utils', () => { }); it('does not copy non-string properties from context to request', () => { + const symbol = Symbol('anotherProp'); + const context = new MiddlewareContext(); + context.set('extraProp', 'value'); + context.set(symbol, { nested: true }); + context.set(42, 'value'); + const request = { jsonrpc, method: 'test_method', params: [1], id: 42, }; - const context = new MiddlewareContext>(); - context.set('extraProp', 'value'); - context.set(Symbol('anotherProp'), { nested: true }); - propagateToRequest(request, context); expect(request).toStrictEqual({ @@ -301,6 +303,7 @@ describe('compatibility-utils', () => { id: 42, extraProp: 'value', }); + expect(symbol in request).toBe(false); }); it('excludes JSON-RPC properties from propagation', () => { From a45a3c543e982af39b57b9daae7741067efe37ac Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Tue, 21 Oct 2025 12:02:10 -0700 Subject: [PATCH 13/18] feat: Add version-agnostic isInstance to JsonRpcEngineError --- packages/json-rpc-engine/src/v2/utils.test.ts | 14 +++++++++---- packages/json-rpc-engine/src/v2/utils.ts | 20 +++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/packages/json-rpc-engine/src/v2/utils.test.ts b/packages/json-rpc-engine/src/v2/utils.test.ts index 7dcc5185ebe..f7fde4f0e05 100644 --- a/packages/json-rpc-engine/src/v2/utils.test.ts +++ b/packages/json-rpc-engine/src/v2/utils.test.ts @@ -27,7 +27,7 @@ describe('utils', () => { }, false, ], - ])('should return $expected for $request', (request, expected) => { + ])('returns $expected for $request', (request, expected) => { expect(isRequest(request)).toBe(expected); }); }); @@ -39,13 +39,13 @@ describe('utils', () => { { id: 1, jsonrpc, method: 'eth_getBlockByNumber', params: ['latest'] }, false, ], - ])('should return $expected for $request', (request, expected) => { + ])('returns $expected for $request', (request, expected) => { expect(isNotification(request)).toBe(expected); }); }); describe('stringify', () => { - it('should stringify a JSON object', () => { + it('stringifies a JSON object', () => { expect(stringify({ foo: 'bar' })).toMatchInlineSnapshot(` "{ \\"foo\\": \\"bar\\" @@ -55,11 +55,17 @@ describe('utils', () => { }); describe('JsonRpcEngineError', () => { - it('should create an error with the correct name', () => { + it('creates an error with the correct name', () => { const error = new JsonRpcEngineError('test'); expect(error).toBeInstanceOf(Error); expect(error.name).toBe('JsonRpcEngineError'); expect(error.message).toBe('test'); }); + + it('isInstance checks if a value is a JsonRpcEngineError instance', () => { + const error = new JsonRpcEngineError('test'); + expect(JsonRpcEngineError.isInstance(error)).toBe(true); + expect(JsonRpcEngineError.isInstance(new Error('test'))).toBe(false); + }); }); }); diff --git a/packages/json-rpc-engine/src/v2/utils.ts b/packages/json-rpc-engine/src/v2/utils.ts index abe761098cb..25be3d6c1a4 100644 --- a/packages/json-rpc-engine/src/v2/utils.ts +++ b/packages/json-rpc-engine/src/v2/utils.ts @@ -33,9 +33,29 @@ export function stringify(value: unknown): string { return JSON.stringify(value, null, 2); } +const JsonRpcEngineErrorSymbol = Symbol.for('JsonRpcEngineError'); + export class JsonRpcEngineError extends Error { + private readonly [JsonRpcEngineErrorSymbol] = true; + constructor(message: string) { super(message); this.name = 'JsonRpcEngineError'; } + + /** + * Check if a value is a {@link JsonRpcEngineError} instance. + * Works across different package versions in the same realm. + * + * @param value - The value to check. + * @returns Whether the value is a {@link JsonRpcEngineError} instance. + */ + static isInstance( + value: Value, + ): value is Value & JsonRpcEngineError { + return ( + hasProperty(value, JsonRpcEngineErrorSymbol) && + value[JsonRpcEngineErrorSymbol] === true + ); + } } From a3765880c9bb1da8c3352265b59377eb21e88a98 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Tue, 21 Oct 2025 12:21:30 -0700 Subject: [PATCH 14/18] feat: Re-export JSON-RPC types from utils --- packages/json-rpc-engine/src/v2/index.ts | 9 +++++++-- packages/json-rpc-engine/src/v2/utils.ts | 15 ++++++++------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/json-rpc-engine/src/v2/index.ts b/packages/json-rpc-engine/src/v2/index.ts index 64e6afae888..e8cbc1291b0 100644 --- a/packages/json-rpc-engine/src/v2/index.ts +++ b/packages/json-rpc-engine/src/v2/index.ts @@ -1,8 +1,13 @@ -export type { Json } from '@metamask/utils'; export { asLegacyMiddleware } from './asLegacyMiddleware'; export { getUniqueId } from '../getUniqueId'; export * from './JsonRpcEngineV2'; export { JsonRpcServer } from './JsonRpcServer'; export type { MiddlewareContext } from './MiddlewareContext'; export { isNotification, isRequest, JsonRpcEngineError } from './utils'; -export type { JsonRpcCall, JsonRpcNotification, JsonRpcRequest } from './utils'; +export type { + Json, + JsonRpcCall, + JsonRpcNotification, + JsonRpcParams, + JsonRpcRequest, +} from './utils'; diff --git a/packages/json-rpc-engine/src/v2/utils.ts b/packages/json-rpc-engine/src/v2/utils.ts index 25be3d6c1a4..5a0b23bb910 100644 --- a/packages/json-rpc-engine/src/v2/utils.ts +++ b/packages/json-rpc-engine/src/v2/utils.ts @@ -1,15 +1,16 @@ import { hasProperty, - type JsonRpcNotification as BaseJsonRpcNotification, + type JsonRpcNotification, type JsonRpcParams, - type JsonRpcRequest as BaseJsonRpcRequest, + type JsonRpcRequest, } from '@metamask/utils'; -export type JsonRpcNotification = - BaseJsonRpcNotification; - -export type JsonRpcRequest = - BaseJsonRpcRequest; +export type { + Json, + JsonRpcParams, + JsonRpcRequest, + JsonRpcNotification, +} from '@metamask/utils'; export type JsonRpcCall = | JsonRpcNotification From 2eb37827c4a16808213d178d87dfaf047cf04bd4 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Wed, 22 Oct 2025 08:15:45 -0700 Subject: [PATCH 15/18] docs: Update readme --- packages/json-rpc-engine/README.md | 261 ++++++++++++++++++++++++----- 1 file changed, 223 insertions(+), 38 deletions(-) diff --git a/packages/json-rpc-engine/README.md b/packages/json-rpc-engine/README.md index ed045ac91a3..e4da1cc1c79 100644 --- a/packages/json-rpc-engine/README.md +++ b/packages/json-rpc-engine/README.md @@ -23,26 +23,26 @@ import type { MiddlewareContext, } from '@metamask/json-rpc-engine/v2'; -// Create a stack of middleware. You must explicitly type your middleware -// functions. -const middleware: JsonRpcMiddleware< +type Middleware = JsonRpcMiddleware< JsonRpcRequest, Json, MiddlewareContext<{ hello: string }> ->[] = [ - ({ request, next, context }) => { - if (request.method === 'hello') { - context.set('hello', 'world'); - return next(); - } - return null; - }, - ({ context }) => context.get('hello'), -]; - -// Pass your middleware to the engine. You must use the static factory method -// `create()` instead of the constructor. -const engine = JsonRpcEngineV2.create({ middleware }); +>; + +// Engines are instantiated using the `create()` factory method as opposed to +// the constructor, which is private. +const engine = JsonRpcEngineV2.create({ + middleware: [ + ({ request, next, context }) => { + if (request.method === 'hello') { + context.set('hello', 'world'); + return next(); + } + return null; + }, + ({ context }) => context.assertGet('hello'), + ], +}); ``` Requests are handled asynchronously, stepping down the middleware stack until complete. @@ -103,8 +103,8 @@ legacyEngine.push(asLegacyMiddleware(v2Engine)); ``` In keeping with the conventions of the legacy engine, non-JSON-RPC string properties of the `context` will be -copied over to the request once the V2 engine is done with the request. _Note that any symbol keys of the `context` -will **not** be copied over._ +copied over to the request once the V2 engine is done with the request. _Note that **only `string` keys** of +the `context` will be copied over._ ### Middleware @@ -116,7 +116,7 @@ They receive a `MiddlewareParams` object containing: - `context` - An append-only `Map` for passing data between middleware - `next` - - Function to call the next middleware in the stack + - Function that calls the next middleware in the stack and returns its result (if any) Here's a basic example: @@ -125,18 +125,42 @@ const engine = JsonRpcEngineV2.create({ middleware: [ ({ next, context }) => { context.set('foo', 'bar'); - // Proceed to the next middleware + // Proceed to the next middleware and return its result return next(); }, async ({ request, context }) => { await doSomething(request, context.get('foo')); - // Return a result to end the request + // Return a result wihout calling next() to end the request return 42; }, ], }); ``` +In practice, middleware functions are often defined apart from the engine in which +they are used. Middleware defined in this manner must use the `JsonRpcMiddleware` type: + +```ts +export const permissionMiddleware: JsonRpcMiddleware< + JsonRpcRequest, + Json, // The result + MiddlewareContext<{ user: User; permissions: Permissions }> +> = async ({ request, context, next }) => { + const user = context.assertGet('user'); + const permissions = await getUserPermissions(user.id); + context.set('permissions', permissions); + return next(); +}; +``` + +Middleware can specify a return type, however `next()` always returns the widest possible +type based on the type of the `request`. See [Requests vs. notifications](#requests-vs-notifications) +for more details. + +Creating a useful `JsonRpcEngineV2` requires composing differently typed middleware together. +See [Engine composition](#engine-composition) for how to +accomplish this in the same or a set of composed engines. + ### Requests vs. notifications JSON-RPC requests come in two flavors: @@ -144,6 +168,9 @@ JSON-RPC requests come in two flavors: - [Requests](https://www.jsonrpc.org/specification#request_object), i.e. request objects _with_ an `id` - [Notifications](https://www.jsonrpc.org/specification#notification), i.e. request objects _without_ an `id` +`next()` returns `Json` for requests, `void` for notifications, and `Json | void` if the type of the request +object is not known. + For requests, one of the engine's middleware must "end" the request by returning a non-`undefined` result, or `.handle()` will throw an error: @@ -170,7 +197,7 @@ try { ``` For notifications, on the other hand, one of the engine's middleware must return `undefined` to end the request, -and any non-`undefined` return values will cause an error: +and any non-`undefined` return values will cause an error to be thrown: ```ts const notification = { jsonrpc: '2.0', method: 'hello' }; @@ -186,6 +213,12 @@ try { If your middleware may be passed both requests and notifications, use the `isRequest` or `isNotification` utilities to determine what to do: +> [!NOTE] +> Middleware that handle both requests and notifications—i.e. the `JsonRpcCall` type— +> must ensure that their return values are valid for incoming requests at runtime. +> There is no compile time type error if such a middleware returns e.g. a string +> for a notification. + ```ts import { isRequest, @@ -246,9 +279,9 @@ an error: const engine = JsonRpcEngineV2.create({ middleware: [ ({ request, next }) => { - // Modifying either property will cause an error return next({ ...request, + // Modifying either property will cause an error jsonrpc: '3.0', id: 'foo', }); @@ -256,6 +289,9 @@ const engine = JsonRpcEngineV2.create({ () => 42, ], }); + +// Error: Middleware attempted to modify readonly property... +await engine.handle(anyRequest); ``` ### Result handling @@ -275,8 +311,8 @@ const engine = JsonRpcEngineV2.create({ `Request ${request.method} producing ${result} took ${duration}ms`, ); - // By returning undefined, the same result will be forwarded to earlier - // middleware awaiting next() + // By returning `undefined`, the result will be forwarded unmodified to earlier + // middleware. }, ({ request }) => { return 'Hello, World!'; @@ -306,6 +342,7 @@ const engine = JsonRpcEngineV2.create({ }; } + // Returning the unmodified result is equivalent to returning `undefined` return result; }, ({ request }) => { @@ -330,7 +367,7 @@ console.log(result); // } ``` -### Context sharing +### The `MiddlewareContext` Use the `context` to share data between middleware: @@ -343,8 +380,7 @@ const engine = JsonRpcEngineV2.create({ }, async ({ context, next }) => { // context.assertGet() throws if the value does not exist - // Use with caution: it does not otherwise perform any type checks. - const user = context.assertGet<{ id: string; name: string }>('user'); + const user = context.assertGet('user') as { id: string; name: string }; context.set('permissions', await getUserPermissions(user.id)); return next(); }, @@ -357,9 +393,10 @@ const engine = JsonRpcEngineV2.create({ }); ``` -The `context` accepts symbol and string keys. To prevent accidental naming collisions, -it is append-only with deletions. -If you need to modify a context value over multiple middleware, use an array or object: +The `context` supports `PropertyKey` keys, i.e. strings, numbers, and symbols. +To prevent accidental naming collisions, existing keys must be deleted before they can be +overwritten via `set()`. +Context values are not frozen, and objects can be mutated as normal: ```ts const engine = JsonRpcEngineV2.create({ @@ -378,6 +415,31 @@ const engine = JsonRpcEngineV2.create({ }); ``` +#### Constraining context keys and values + +The context exposes a generic parameter `KeyValues`, which determines the keys and values +a context instance supports: + +```ts +const context = new MiddlewareContext(); +context.set('foo', 'bar'); +context.get('foo'); // 'bar' +context.get('fizz'); // undefined +``` + +By default, `KeyValues` is `Record`. However, any object type can be +specified, effectively turning the context into a strongly typed `Map`: + +```ts +const context = new MiddlewareContext<{ foo: string }>([['foo', 'bar']]); +context.get('foo'); // 'bar' +context.get('fizz'); // Type error +``` + +The context is itself exposed as the third generic parameter of the `JsonRpcMiddleware` type. +See [Instrumenting middleware pipelines](#instrumenting-middleware-pipelines) for how to +compose different context types together. + ### Error handling Errors in middleware are propagated up the call stack: @@ -435,8 +497,90 @@ console.log('Result:', result); // Result: 42 ``` +#### Internal errors + +The engine throws `JsonRpcEngineError` values when its invariants are violated, e.g. a middleware returns +a result value for a notification. +If you want to reliably detect these cases, use `JsonRpcEngineError.isInstance(error)`, which works across +versions of this package in the same realm. + ### Engine composition +#### Instrumenting middleware pipelines + +As discussed in the [Middleware](#middleware) section, middleware are often defined apart from the +engine in which they are used. To be used within the same engine, a set of middleware must have +compatible types. Specifically, all middleware must: + +- Handle either `JsonRpcRequest`, `JsonRpcNotification`, or both (i.e. `JsonRpcCall`) +- Return valid results for the overall request type +- Specify mutually inclusive context types + - The context types may be the same, partially intersecting, or completely disjoint + so long as they are not mutually exclusive. + +For example, the following middleware are compatible: + +```ts +const middleware1: JsonRpcMiddleware< + JsonRpcRequest, + Json, + MiddlewareContext<{ foo: string }> +> = /* ... */; + +const middleware2: JsonRpcMiddleware< + JsonRpcRequest, + Json, + MiddlewareContext<{ bar: string }> +> = /* ... */; + +const middleware3: JsonRpcMiddleware< + JsonRpcRequest, + { foo: string; bar: string }, + MiddlewareContext<{ foo: string; bar: string; baz: number }> +> = /* ... */; + +// ✅ OK +const engine = JsonRpcEngineV2.create({ + middleware: [middleware1, middleware2, middleware3], +}); +``` + +The following middleware are incompatible due to mismatched request types: + +```ts +const middleware1: JsonRpcMiddleware = /* ... */; + +const middleware2: JsonRpcMiddleware = /* ... */; + +// ❌ Attempting to call engine.handle() with any value will cause a type error +const engine = JsonRpcEngineV2.create({ + middleware: [middleware1, middleware2], +}); +``` + +Finally, these middleware are incompatible due to mismatched context types: + +```ts +const middleware1: JsonRpcMiddleware< + JsonRpcRequest, + Json, + MiddlewareContext<{ foo: string }> +> = /* ... */; + +const middleware2: JsonRpcMiddleware< + JsonRpcRequest, + Json, + MiddlewareContext<{ foo: number }> +> = /* ... */; + +// ❌ The type of the engine is `never`; accessing any property will cause a type error +const engine = JsonRpcEngineV2.create({ + middleware: [middleware1, middleware2], +}); +``` + +#### `asMiddleware()` + Engines can be nested by converting them to middleware using `asMiddleware()`: ```ts @@ -490,6 +634,40 @@ console.log('Result:', result); const result2 = await loggingEngine.handle(request); ``` +#### Calling `handle()` in a middleware + +You can also compose different engines together by calling `handle(request, context)` +on a different engine in a middleware. Keep in mind that, unlike when using `asMiddleware()`, +these "sub"-engines must return results for requests. + +This method of composition can be useful to instrument request- and notification-only +middleware pipelines: + +```ts +const requestEngine = JsonRpcEngineV2.create({ + middleware: [ + /* Request-only middleware */ + ], +}); + +const notificationEngine = JsonRpcEngineV2.create({ + middleware: [ + /* Notification-only middleware */ + ], +}); + +const orchestratorEngine = JsonRpcEngineV2.create({ + middleware: [ + ({ request, context }) => + isRequest(request) + ? requestEngine.handle(request, { context }) + : notificationEngine.handle(request as JsonRpcNotification, { + context, + }), + ], +}); +``` + ### `JsonRpcServer` The `JsonRpcServer` wraps a `JsonRpcEngineV2` to provide JSON-RPC 2.0 compliance and error handling. It coerces raw request objects into well-formed requests and handles error serialization. @@ -517,17 +695,24 @@ if ('result' in response) { // Handle error response } -// Notifications return undefined +// Notifications always return undefined const notification = { jsonrpc: '2.0', method: 'hello' }; await server.handle(notification); // Returns undefined ``` -The server accepts any object with a `method` property and validates JSON-RPC 2.0 -compliance. -Response objects are returned for requests but not notifications, and contain +The server accepts any object with a `method` property, coercing it into a request or notification +depending on the presence or absence of the `id` property, respectively. +Except for the `id`, all present JSON-RPC 2.0 fields are validated for spec conformance. +The `id` is replaced during request processing with an internal, trusted value, although the +original `id` is attached to the response before it is returned. + +Response objects are returned for requests, and contain the `result` in case of success and `error` in case of failure. -Errors thrown by the underlying engine are passed to `onError` before being serialized -and attached to the response object via the `error` property. +`undefined` is always returned for notifications. + +Errors thrown by the underlying engine are always passed to `onError` unmodified. +If the request is not a notification, the error is subsequently serialized and attached +to the response object via the `error` property. ## Contributing From 10a36c05a288e4eb3a10bc8a5987d2e948ae8688 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Thu, 23 Oct 2025 15:49:09 -0700 Subject: [PATCH 16/18] refactor: Apply suggestions from review --- .../json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts | 2 +- packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts | 11 ++++++++++- packages/json-rpc-engine/src/v2/MiddlewareContext.ts | 7 ++----- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts index da283573a25..6a25ca0436b 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts @@ -376,7 +376,7 @@ describe('JsonRpcEngineV2', () => { middleware: [middleware1, middleware2], }); - // @ts-expect-error - The engine is `never`, which is what we want. + // @ts-expect-error - The engine is `InvalidEngine`, which is what we want. expect(await engine.handle(makeRequest())).toBe('bar'); }); diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts index dc738ecb98b..69c6b0e750d 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts @@ -97,6 +97,15 @@ type MergedContextOf< Middleware extends JsonRpcMiddleware, > = MergeContexts>; +const INVALID_ENGINE = Symbol('Invalid engine'); + +/** + * An internal type for invalid engines that explains why the engine is invalid. + * + * @template Message - The message explaining why the engine is invalid. + */ +type InvalidEngine = { [INVALID_ENGINE]: Message }; + /** * A JSON-RPC request and response processor. * @@ -187,7 +196,7 @@ export class JsonRpcEngineV2< return new JsonRpcEngineV2({ middleware: mw, }) as MergedContext extends never - ? never + ? InvalidEngine<'Some middleware have incompatible context types'> : JsonRpcEngineV2; } diff --git a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts index eb11baf031c..0fa97b86136 100644 --- a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts +++ b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts @@ -70,10 +70,7 @@ export class MiddlewareContext< /** * Infer the KeyValues type from a {@link MiddlewareContext}. */ -// Using `any` in this constraint does not pollute other types. -// eslint-disable-next-line @typescript-eslint/no-explicit-any -type InferKeyValues> = - T extends MiddlewareContext ? U : never; +type InferKeyValues = T extends MiddlewareContext ? U : never; /** * An unholy incantation that converts a union of object types into an @@ -84,7 +81,7 @@ type InferKeyValues> = * type B = UnionToIntersection; // { a: string } & { b: number } */ type UnionToIntersection = ( - U extends unknown ? (k: U) => void : never + U extends never ? never : (k: U) => void ) extends (k: infer I) => void ? I : never; From 547b6a9bbbdf790821e7e3e726b290d02890c258 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Thu, 23 Oct 2025 15:56:02 -0700 Subject: [PATCH 17/18] feat: Add and test EmtpyContext type --- .../json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts | 9 ++++++++- packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts | 10 +++------- packages/json-rpc-engine/src/v2/MiddlewareContext.ts | 7 ++++++- packages/json-rpc-engine/src/v2/index.ts | 2 +- 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts index 6a25ca0436b..93c7769a37b 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts @@ -4,6 +4,7 @@ import { createDeferredPromise } from '@metamask/utils'; import type { JsonRpcMiddleware, ResultConstraint } from './JsonRpcEngineV2'; import { JsonRpcEngineV2 } from './JsonRpcEngineV2'; +import type { EmptyContext } from './MiddlewareContext'; import { MiddlewareContext } from './MiddlewareContext'; import { isRequest, @@ -343,13 +344,19 @@ describe('JsonRpcEngineV2', () => { > = ({ next }) => next(); const middleware3: JsonRpcMiddleware< + JsonRpcCall, + ResultConstraint, + EmptyContext + > = ({ next }) => next(); + + const middleware4: JsonRpcMiddleware< JsonRpcCall, string, MiddlewareContext<{ foo: string; bar: number }> > = ({ context }) => context.assertGet('foo'); const engine = JsonRpcEngineV2.create({ - middleware: [middleware1, middleware2, middleware3], + middleware: [middleware1, middleware2, middleware3, middleware4], }); const result = await engine.handle(makeRequest()); diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts index 69c6b0e750d..4d34ce04243 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts @@ -7,11 +7,7 @@ import { } from '@metamask/utils'; import deepFreeze from 'deep-freeze-strict'; -import type { - ContextConstraint, - DefaultContext, - MergeContexts, -} from './MiddlewareContext'; +import type { ContextConstraint, MergeContexts } from './MiddlewareContext'; import { MiddlewareContext } from './MiddlewareContext'; import { isNotification, @@ -54,7 +50,7 @@ export type MiddlewareParams< export type JsonRpcMiddleware< Request extends JsonRpcCall = JsonRpcCall, Result extends ResultConstraint = ResultConstraint, - Context extends ContextConstraint = DefaultContext, + Context extends ContextConstraint = MiddlewareContext, > = ( params: MiddlewareParams, ) => Readonly | undefined | Promise | undefined>; @@ -143,7 +139,7 @@ type InvalidEngine = { [INVALID_ENGINE]: Message }; */ export class JsonRpcEngineV2< Request extends JsonRpcCall = JsonRpcCall, - Context extends ContextConstraint = DefaultContext, + Context extends ContextConstraint = MiddlewareContext, > { #middleware: Readonly< NonEmptyArray< diff --git a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts index 0fa97b86136..3bdb9b4cce8 100644 --- a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts +++ b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts @@ -138,4 +138,9 @@ export type MergeContexts = // eslint-disable-next-line @typescript-eslint/no-explicit-any export type ContextConstraint = MiddlewareContext; -export type DefaultContext = MiddlewareContext; +/** + * The empty context type, i.e. `MiddlewareContext<{}>`. + */ +// The empty object type is literally an empty object in this context. +// eslint-disable-next-line @typescript-eslint/no-empty-object-type +export type EmptyContext = MiddlewareContext<{}>; diff --git a/packages/json-rpc-engine/src/v2/index.ts b/packages/json-rpc-engine/src/v2/index.ts index e8cbc1291b0..eb63fa05b7b 100644 --- a/packages/json-rpc-engine/src/v2/index.ts +++ b/packages/json-rpc-engine/src/v2/index.ts @@ -2,7 +2,7 @@ export { asLegacyMiddleware } from './asLegacyMiddleware'; export { getUniqueId } from '../getUniqueId'; export * from './JsonRpcEngineV2'; export { JsonRpcServer } from './JsonRpcServer'; -export type { MiddlewareContext } from './MiddlewareContext'; +export type { MiddlewareContext, EmptyContext } from './MiddlewareContext'; export { isNotification, isRequest, JsonRpcEngineError } from './utils'; export type { Json, From 75f61e771138c3cf60063129e658e03039e4e829 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Thu, 23 Oct 2025 23:01:29 -0700 Subject: [PATCH 18/18] refactor: Tweaks and docs --- packages/json-rpc-engine/README.md | 10 ++- .../src/v2/JsonRpcEngineV2.test.ts | 66 ++++++++++++------- .../json-rpc-engine/src/v2/JsonRpcEngineV2.ts | 10 ++- .../src/v2/MiddlewareContext.ts | 16 +---- packages/json-rpc-engine/src/v2/utils.ts | 14 ++++ 5 files changed, 75 insertions(+), 41 deletions(-) diff --git a/packages/json-rpc-engine/README.md b/packages/json-rpc-engine/README.md index e4da1cc1c79..1113ede22df 100644 --- a/packages/json-rpc-engine/README.md +++ b/packages/json-rpc-engine/README.md @@ -513,6 +513,8 @@ engine in which they are used. To be used within the same engine, a set of middl compatible types. Specifically, all middleware must: - Handle either `JsonRpcRequest`, `JsonRpcNotification`, or both (i.e. `JsonRpcCall`) + - It is okay to mix `JsonRpcCall` middleware with either `JsonRpcRequest` or `JsonRpcNotification` + middleware, as long as the latter two are not mixed together. - Return valid results for the overall request type - Specify mutually inclusive context types - The context types may be the same, partially intersecting, or completely disjoint @@ -547,12 +549,18 @@ const engine = JsonRpcEngineV2.create({ The following middleware are incompatible due to mismatched request types: +> [!WARNING] +> Providing `JsonRpcRequest`- and `JsonRpcNotification`-only middleware to the same engine is +> unsound and should be avoided. However, doing so will **not** cause a type error, and it +> is the programmer's responsibility to prevent it from happening. + ```ts const middleware1: JsonRpcMiddleware = /* ... */; const middleware2: JsonRpcMiddleware = /* ... */; -// ❌ Attempting to call engine.handle() with any value will cause a type error +// ⚠️ Attempting to call engine.handle() will NOT cause a type error, but it +// may cause errors at runtime and should be avoided. const engine = JsonRpcEngineV2.create({ middleware: [middleware1, middleware2], }); diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts index 93c7769a37b..45e30a6afb8 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts @@ -31,6 +31,49 @@ describe('JsonRpcEngineV2', () => { new JsonRpcEngineError('Middleware array cannot be empty'), ); }); + + it('type errors if passed middleware with incompatible context types', async () => { + const middleware1: JsonRpcMiddleware< + JsonRpcCall, + ResultConstraint, + MiddlewareContext<{ foo: string }> + > = ({ next, context }) => { + context.set('foo', 'bar'); + return next(); + }; + + const middleware2: JsonRpcMiddleware< + JsonRpcCall, + ResultConstraint, + MiddlewareContext<{ foo: number }> + > = ({ context }) => context.assertGet('foo'); + const engine = JsonRpcEngineV2.create({ + middleware: [middleware1, middleware2], + }); + + // @ts-expect-error - The engine is `InvalidEngine`. + expect(await engine.handle(makeRequest())).toBe('bar'); + }); + + // Keeping this here for documentation purposes. + // eslint-disable-next-line jest/no-disabled-tests + it.skip('type errors if passed middleware with incompatible request types', async () => { + const middleware1: JsonRpcMiddleware = ({ next }) => + next(); + const middleware2: JsonRpcMiddleware = () => { + return 'foo'; + }; + const engine = JsonRpcEngineV2.create({ + middleware: [middleware1, middleware2], + }); + + // TODO: We want this to cause a type error, but it's unclear if it can be + // made to work due to the difficulty (impossibility?) of distinguishing + // between these two cases: + // - JsonRpcMiddleware | JsonRpcMiddleware (invalid) + // - JsonRpcMiddleware | JsonRpcMiddleware (valid) + expect(await engine.handle(makeRequest() as JsonRpcRequest)).toBe('foo'); + }); }); describe('handle', () => { @@ -364,29 +407,6 @@ describe('JsonRpcEngineV2', () => { expect(result).toBe('bar'); }); - it('type errors if different context types are mutually exclusive', async () => { - const middleware1: JsonRpcMiddleware< - JsonRpcCall, - ResultConstraint, - MiddlewareContext<{ foo: string }> - > = ({ next, context }) => { - context.set('foo', 'bar'); - return next(); - }; - - const middleware2: JsonRpcMiddleware< - JsonRpcCall, - ResultConstraint, - MiddlewareContext<{ foo: number }> - > = ({ context }) => context.assertGet('foo'); - const engine = JsonRpcEngineV2.create({ - middleware: [middleware1, middleware2], - }); - - // @ts-expect-error - The engine is `InvalidEngine`, which is what we want. - expect(await engine.handle(makeRequest())).toBe('bar'); - }); - it('throws if a middleware attempts to modify properties of the context', async () => { const engine = JsonRpcEngineV2.create({ middleware: [ diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts index 4d34ce04243..4f55df4a382 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts @@ -74,9 +74,13 @@ type ConstructorOptions< }; type RequestOf = - // Non-polluting `any` constraint. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - Middleware extends JsonRpcMiddleware + Middleware extends JsonRpcMiddleware< + infer Request, + ResultConstraint, + // Non-polluting `any` constraint. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + any + > ? Request : never; diff --git a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts index 3bdb9b4cce8..8eb291dcc7b 100644 --- a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts +++ b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts @@ -1,3 +1,5 @@ +import type { UnionToIntersection } from './utils'; + /** * An context object for middleware that attempts to protect against accidental * modifications. Its interface is frozen. @@ -72,20 +74,6 @@ export class MiddlewareContext< */ type InferKeyValues = T extends MiddlewareContext ? U : never; -/** - * An unholy incantation that converts a union of object types into an - * intersection of object types. - * - * @example - * type A = { a: string } | { b: number }; - * type B = UnionToIntersection; // { a: string } & { b: number } - */ -type UnionToIntersection = ( - U extends never ? never : (k: U) => void -) extends (k: infer I) => void - ? I - : never; - /** * Simplifies an object type by "merging" its properties. * diff --git a/packages/json-rpc-engine/src/v2/utils.ts b/packages/json-rpc-engine/src/v2/utils.ts index 5a0b23bb910..345c9907b18 100644 --- a/packages/json-rpc-engine/src/v2/utils.ts +++ b/packages/json-rpc-engine/src/v2/utils.ts @@ -24,6 +24,20 @@ export const isNotification = ( msg: JsonRpcCall, ): msg is JsonRpcNotification => !isRequest(msg); +/** + * An unholy incantation that converts a union of object types into an + * intersection of object types. + * + * @example + * type A = { a: string } | { b: number }; + * type B = UnionToIntersection; // { a: string } & { b: number } + */ +export type UnionToIntersection = ( + U extends never ? never : (k: U) => void +) extends (k: infer I) => void + ? I + : never; + /** * JSON-stringifies a value. *