From f17f0f221c18b31cc86e26e9bdfb14f086f9e76a Mon Sep 17 00:00:00 2001 From: uki00a Date: Sun, 8 Dec 2024 22:44:08 +0900 Subject: [PATCH] feat: improve type-safety for events --- connection.ts | 153 +++++++++++++---------------------- events.ts | 49 +++++++++++ mod.ts | 13 ++- redis.ts | 60 ++++++-------- tests/commands/connection.ts | 42 +++++----- 5 files changed, 156 insertions(+), 161 deletions(-) create mode 100644 events.ts diff --git a/connection.ts b/connection.ts index 5b2d0f5f..1754da81 100644 --- a/connection.ts +++ b/connection.ts @@ -1,6 +1,11 @@ import type { Backoff } from "./backoff.ts"; import { exponentialBackoff } from "./backoff.ts"; import { ErrorReplyError, isRetriableError } from "./errors.ts"; +import type { + ConnectionEventMap, + ConnectionEventType, + TypedEventTarget, +} from "./events.ts"; import { kUnstableCreateProtocol, kUnstablePipeline, @@ -12,74 +17,6 @@ import type { Command, Protocol } from "./protocol/shared/protocol.ts"; import type { RedisReply, RedisValue } from "./protocol/shared/types.ts"; import { delay } from "./deps/std/async.ts"; -type TypedEventTarget = { - new (): IntermediateEventTarget; -}; - -interface IntermediateEventTarget extends EventTarget { - addEventListener( - type: K, - callback: ( - event: EventMap[K] extends Event ? EventMap[K] : never, - ) => EventMap[K] extends Event ? void : never, - options?: AddEventListenerOptions | boolean, - ): void; - - addEventListener( - type: string, - callback: EventListenerOrEventListenerObject | null, - options?: AddEventListenerOptions | boolean, - ): void; - - removeEventListener( - type: K, - callback: ( - event: EventMap[K] extends Event ? EventMap[K] : never, - ) => EventMap[K] extends Event ? void : never, - options?: EventListenerOptions | boolean, - ): void; - - removeEventListener( - type: string, - callback: EventListenerOrEventListenerObject | null, - options?: EventListenerOptions | boolean, - ): void; -} - -export type ConnectionEvent = Record; - -export type ConnectionErrorEvent = { - error: Error; -}; - -export type ConnectionReconnectingEvent = { - delay: number; -}; - -export type ConnectionEventMap = { - error: CustomEvent; - connect: CustomEvent; - reconnecting: CustomEvent; - ready: CustomEvent; - close: CustomEvent; - end: CustomEvent; -}; - -export type ConnectionEventTarget = TypedEventTarget; - -export type ConnectionEventType = - | "error" - | "connect" - | "reconnecting" - | "ready" - | "close" - | "end"; - -export type ConnectionEventArg = T extends - "error" ? Error - : T extends "reconnecting" ? number - : undefined; - export interface SendCommandOptions { /** * When this option is set, simple or bulk string replies are returned as `Uint8Array` type. @@ -96,7 +33,7 @@ export interface SendCommandOptions { inline?: boolean; } -export interface Connection extends EventTarget { +export interface Connection extends TypedEventTarget { name: string | null; isClosed: boolean; isConnected: boolean; @@ -158,8 +95,8 @@ interface PendingCommand { reject: (error: unknown) => void; } -export class RedisConnection extends (EventTarget as ConnectionEventTarget) - implements Connection { +export class RedisConnection + implements Connection, TypedEventTarget { name: string | null = null; private maxRetryCount = 10; @@ -172,6 +109,7 @@ export class RedisConnection extends (EventTarget as ConnectionEventTarget) private commandQueue: PendingCommand[] = []; #conn!: Deno.Conn; #protocol!: Protocol; + #eventTarget = new EventTarget(); get isClosed(): boolean { return this._isClosed; @@ -190,8 +128,6 @@ export class RedisConnection extends (EventTarget as ConnectionEventTarget) port: number | string, private options: RedisConnectionOptions, ) { - super(); - this.hostname = hostname; this.port = port; if (options.name) { @@ -216,10 +152,10 @@ export class RedisConnection extends (EventTarget as ConnectionEventTarget) const authError = new AuthenticationError("Authentication failed", { cause: error, }); - this.fireEvent("error", authError); + this.#dispatchEvent("error", { error: authError }); throw authError; } else { - this.fireEvent("error", error as Error); + this.#dispatchEvent("error", { error }); throw error; } } @@ -261,6 +197,37 @@ export class RedisConnection extends (EventTarget as ConnectionEventTarget) return promise; } + addEventListener( + type: K, + callback: (event: CustomEvent) => void, + options?: AddEventListenerOptions | boolean, + ): void { + return this.#eventTarget.addEventListener( + type, + callback as (event: Event) => void, + options, + ); + } + + removeEventListener( + type: K, + callback: (event: CustomEvent) => void, + options?: EventListenerOptions | boolean, + ): void { + return this.#eventTarget.removeEventListener( + type, + callback as (event: Event) => void, + options, + ); + } + + #dispatchEvent( + type: K, + detail: ConnectionEventMap[K], + ): boolean { + return this.#eventTarget.dispatchEvent(new CustomEvent(type, { detail })); + } + [kUnstableReadReply](returnsUint8Arrays?: boolean): Promise { return this.#protocol.readReply(returnsUint8Arrays); } @@ -301,7 +268,7 @@ export class RedisConnection extends (EventTarget as ConnectionEventTarget) this._isClosed = false; this._isConnected = true; - this.fireEvent("connect", undefined); + this.#dispatchEvent("connect", undefined); try { if (this.options.password != null) { @@ -315,24 +282,24 @@ export class RedisConnection extends (EventTarget as ConnectionEventTarget) throw error; } - this.fireEvent("ready", undefined); + this.#dispatchEvent("ready", undefined); this.#enableHealthCheckIfNeeded(); } catch (error) { if (error instanceof AuthenticationError) { - this.fireEvent("error", error); - this.fireEvent("end", undefined); + this.#dispatchEvent("error", { error }); + this.#dispatchEvent("end", undefined); throw (error.cause ?? error); } const backoff = this.backoff(retryCount); retryCount++; if (retryCount >= this.maxRetryCount) { - this.fireEvent("error", error as Error); - this.fireEvent("end", undefined); + this.#dispatchEvent("error", { error: error as Error }); + this.#dispatchEvent("end", undefined); throw error; } - this.fireEvent("reconnecting", backoff); + this.#dispatchEvent("reconnecting", { delay: backoff }); await delay(backoff); await this.#connect(retryCount); } @@ -351,15 +318,15 @@ export class RedisConnection extends (EventTarget as ConnectionEventTarget) this.#conn!.close(); } catch (error) { if (!(error instanceof Deno.errors.BadResource)) { - this.fireEvent("error", error as Error); + this.#dispatchEvent("error", { error: error as Error }); throw error; } } finally { if (!isClosedAlready) { - this.fireEvent("close", undefined); + this.#dispatchEvent("close", undefined); if (!canReconnect) { - this.fireEvent("end", undefined); + this.#dispatchEvent("end", undefined); } } } @@ -369,8 +336,8 @@ export class RedisConnection extends (EventTarget as ConnectionEventTarget) try { await this.sendCommand("PING"); this._isConnected = true; - } catch (error) { // TODO: Maybe we should log this error. - this.fireEvent("error", error as Error); + } catch (error) { + this.#dispatchEvent("error", { error }); this.#close(true); await this.connect(); await this.sendCommand("PING"); @@ -389,7 +356,7 @@ export class RedisConnection extends (EventTarget as ConnectionEventTarget) !isRetriableError(error) || this.isManuallyClosedByUser() ) { - this.fireEvent("error", error as Error); + this.#dispatchEvent("error", { error }); return command.reject(error); } @@ -406,7 +373,7 @@ export class RedisConnection extends (EventTarget as ConnectionEventTarget) } } - this.fireEvent("error", error as Error); + this.#dispatchEvent("error", { error }); command.reject(error); } finally { this.commandQueue.shift(); @@ -442,14 +409,6 @@ export class RedisConnection extends (EventTarget as ConnectionEventTarget) setTimeout(ping, healthCheckInterval); } - - private fireEvent( - eventType: T, - eventArg: ConnectionEventArg, - ): boolean { - const event = new CustomEvent(eventType, { detail: eventArg }); - return this.dispatchEvent(event); - } } class AuthenticationError extends Error {} diff --git a/events.ts b/events.ts new file mode 100644 index 00000000..fa17add0 --- /dev/null +++ b/events.ts @@ -0,0 +1,49 @@ +export interface TypedEventTarget> + extends + Omit< + EventTarget, + "addEventListener" | "removeEventListener" | "dispatchEvent" + > { + addEventListener( + type: K, + callback: ( + event: CustomEvent, + ) => void, + options?: AddEventListenerOptions | boolean, + ): void; + + removeEventListener( + type: K, + callback: ( + event: CustomEvent, + ) => void, + options?: EventListenerOptions | boolean, + ): void; +} + +export type ConnectionEvent = Record; + +export type ConnectionErrorEventDetails = { + error: unknown; +}; + +export type ConnectionReconnectingEventDetails = { + delay: number; +}; + +export type ConnectionEventMap = { + error: ConnectionErrorEventDetails; + connect: unknown; + reconnecting: ConnectionReconnectingEventDetails; + ready: unknown; + close: unknown; + end: unknown; +}; + +export type ConnectionEventType = + | "error" + | "connect" + | "reconnecting" + | "ready" + | "close" + | "end"; diff --git a/mod.ts b/mod.ts index 51e6f205..602c9697 100644 --- a/mod.ts +++ b/mod.ts @@ -58,16 +58,15 @@ export type { } from "./command.ts"; export type { Connection, - ConnectionErrorEvent, - ConnectionEvent, - ConnectionEventArg, - ConnectionEventMap, - ConnectionEventTarget, - ConnectionEventType, - ConnectionReconnectingEvent, RedisConnectionOptions, SendCommandOptions, } from "./connection.ts"; +export type { + ConnectionErrorEventDetails, + ConnectionEvent, + ConnectionEventType, + ConnectionReconnectingEventDetails, +} from "./events.ts"; export type { CommandExecutor } from "./executor.ts"; export type { RedisPipeline } from "./pipeline.ts"; export type { diff --git a/redis.ts b/redis.ts index a76d73cc..64dba1d5 100644 --- a/redis.ts +++ b/redis.ts @@ -44,16 +44,15 @@ import type { ZUnionstoreOpts, } from "./command.ts"; import { RedisConnection } from "./connection.ts"; -import type { - Connection, - ConnectionEventMap, - ConnectionEventTarget, - ConnectionEventType, - SendCommandOptions, -} from "./connection.ts"; +import type { Connection, SendCommandOptions } from "./connection.ts"; import type { RedisConnectionOptions } from "./connection.ts"; import type { CommandExecutor } from "./executor.ts"; import { DefaultExecutor } from "./executor.ts"; +import type { + ConnectionEventMap, + ConnectionEventType, + TypedEventTarget, +} from "./events.ts"; import type { Binary, Bulk, @@ -110,7 +109,8 @@ const binaryCommandOptions = { returnUint8Arrays: true, }; -export interface Redis extends RedisCommands, EventTarget { +export interface Redis + extends RedisCommands, TypedEventTarget { readonly isClosed: boolean; readonly isConnected: boolean; @@ -128,20 +128,7 @@ export interface Redis extends RedisCommands, EventTarget { [Symbol.dispose](): void; } -interface TypedEventListener { - (evt: E): void; -} - -interface TypedEventListenerObject { - handleEvent(evt: E): void; -} - -type TypedEventListenerOrEventListenerObject = - | TypedEventListener - | TypedEventListenerObject; - -class RedisImpl extends (EventTarget as ConnectionEventTarget) - implements Redis { +class RedisImpl implements Redis { private readonly executor: CommandExecutor; get isClosed() { @@ -153,32 +140,31 @@ class RedisImpl extends (EventTarget as ConnectionEventTarget) } constructor(executor: CommandExecutor) { - super(); this.executor = executor; } - override addEventListener( + addEventListener( type: K, - callback: - | TypedEventListenerOrEventListenerObject - | null, + callback: (event: CustomEvent) => void, options?: boolean | AddEventListenerOptions, ): void { - const listener = callback as EventListenerOrEventListenerObject | null; - this.executor.connection.addEventListener(type, listener, options); - super.addEventListener(type, listener, options); + return this.executor.connection.addEventListener( + type, + callback, + options, + ); } - override removeEventListener( + removeEventListener( type: K, - callback: - | TypedEventListenerOrEventListenerObject - | null, + callback: (event: CustomEvent) => void, options?: boolean | EventListenerOptions, ): void { - const listener = callback as EventListenerOrEventListenerObject | null; - this.executor.connection.removeEventListener(type, listener, options); - super.removeEventListener(type, listener, options); + return this.executor.connection.removeEventListener( + type, + callback, + options, + ); } sendCommand( diff --git a/tests/commands/connection.ts b/tests/commands/connection.ts index 87749fba..1522b473 100644 --- a/tests/commands/connection.ts +++ b/tests/commands/connection.ts @@ -136,43 +136,43 @@ export function connectionTests( let closeEventFired = false, endEventFired = false; + const firedEvents: Array = []; client.addEventListener("close", () => { closeEventFired = true; + firedEvents.push("close"); }); client.addEventListener("end", () => { endEventFired = true; + firedEvents.push("end"); + }); + // @ts-expect-error unkwnon events should be denied + client.addEventListener("no-such-event", () => { + firedEvents.push("no-such-event"); }); client.close(); assertEquals(closeEventFired, true); assertEquals(endEventFired, true); + assertEquals(firedEvents, ["close", "end"]); }); it("fires events with a lazy client", async () => { const client = createLazyClient(getOpts()); - - let connectEventFired = false, - connectEventFiredTimes = 0, - readyEventFired = false, - readyEventFiredTimes = 0, - closeEventFired = false, - endEventFired = false; + const firedEvents: Array = []; client.addEventListener("connect", () => { - connectEventFired = true; - connectEventFiredTimes++; + firedEvents.push("connect"); }); client.addEventListener("ready", () => { - readyEventFired = true; - readyEventFiredTimes++; + firedEvents.push("ready"); }, { once: true }); client.addEventListener("close", () => { - closeEventFired = true; + firedEvents.push("close"); }); client.addEventListener("end", () => { - endEventFired = true; + firedEvents.push("end"); }); await client.exists("foo"); @@ -182,13 +182,15 @@ export function connectionTests( await client.exists("foo"); client.close(); - assertEquals(connectEventFired, true); - assertEquals(readyEventFired, true); - assertEquals(closeEventFired, true); - assertEquals(endEventFired, true); - - assertEquals(connectEventFiredTimes, 2); - assertEquals(readyEventFiredTimes, 1); + assertEquals(firedEvents, [ + "connect", + "ready", + "close", + "end", + "connect", + "close", + "end", + ]); }); });