ref: Make types explicit

Author: mydea

packages/core/src/baseclient.ts

@@ -9,8 +9,6 @@ import type {
   Event,
   EventDropReason,
   EventHint,
-  Hook,
-  HookStore,
   Integration,
   IntegrationClass,
   Outcome,
@@ -19,6 +17,7 @@ import type {
   SessionAggregates,
   Severity,
   SeverityLevel,
+  Transaction,
   TransactionEvent,
   Transport,
 } from '@sentry/types';
@@ -99,7 +98,8 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
   /** Holds flushable  */
   private _outcomes: { [key: string]: number } = {};
 
-  private _hooks: HookStore = {};
+  // eslint-disable-next-line @typescript-eslint/ban-types
+  private _hooks: Record<string, Function[]> = {};
 
   /**
    * Initializes this client instance.
@@ -355,24 +355,35 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
     }
   }
 
-  /**
-   * @inheritDoc
-   */
-  public on<HookType extends Hook>(hook: HookType['name'], callback: HookType['callback']): void {
+  // Keep on() & emit() signatures in sync with types' client.ts interface
+
+  /** @inheritdoc */
+  public on(hook: 'startTransaction' | 'finishTransaction', callback: (transaction: Transaction) => void): void;
+
+  /** @inheritdoc */
+  public on(hook: 'beforeEnvelope', callback: (envelope: Envelope) => void): void;
+
+  /** @inheritdoc */
+  public on(hook: string, callback: unknown): void {
     if (!this._hooks[hook]) {
       this._hooks[hook] = [];
     }
 
-    (this._hooks[hook] as HookType['callback'][]).push(callback);
+    // @ts-ignore We assue the types are correct
+    this._hooks[hook].push(callback);
   }
 
-  /**
-   * @inheritDoc
-   */
-  public emit<HookType extends Hook>(hook: HookType['name'], ...args: Parameters<HookType['callback']>): void {
+  /** @inheritdoc */
+  public emit(hook: 'startTransaction' | 'finishTransaction', transaction: Transaction): void;
+
+  /** @inheritdoc */
+  public emit(hook: 'beforeEnvelope', envelope: Envelope): void;
+
+  /** @inheritdoc */
+  public emit(hook: string, ...rest: unknown[]): void {
     if (this._hooks[hook]) {
-      // @ts-ignore it does not like ...args, but we know this is correct
-      (this._hooks[hook] as HookType['callback'][]).forEach(callback => callback(...args));
+      // @ts-ignore we cannot enforce the callback to match the hook
+      this._hooks[hook].forEach(callback => callback(...rest));
     }
   }
 

packages/core/test/lib/base.test.ts

@@ -1,4 +1,4 @@
-import { Event, Span, Transaction, TransactionHook, EnvelopeHook, Envelope } from '@sentry/types';
+import type { Envelope, Event, Span, Transaction, Client } from '@sentry/types';
 import { dsnToString, logger, SentryError, SyncPromise } from '@sentry/utils';
 
 import { Hub, makeSession, Scope } from '../../src';
@@ -1732,41 +1732,45 @@ describe('BaseClient', () => {
   });
 
   describe('hooks', () => {
-    it('should call a startTransaction hook', () => {
-      expect.assertions(1);
+    const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN });
 
-      const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN });
-      const client = new TestClient(options);
+    // Make sure types work for both Client & BaseClient
+    const scenarios = [
+      ['BaseClient', new TestClient(options)],
+      ['Client', new TestClient(options) as Client],
+    ] as const;
 
-      const mockTransaction = {
-        traceId: '86f39e84263a4de99c326acab3bfe3bd',
-      } as Transaction;
+    describe.each(scenarios)('with client %s', (_, client) => {
+      it('should call a startTransaction hook', () => {
+        expect.assertions(1);
 
-      client?.on<TransactionHook>('startTransaction', (transaction: Transaction) => {
-        expect(transaction).toEqual(mockTransaction);
-      });
+        const mockTransaction = {
+          traceId: '86f39e84263a4de99c326acab3bfe3bd',
+        } as Transaction;
 
-      client?.emit<TransactionHook>('startTransaction', mockTransaction);
-    });
+        client.on?.('startTransaction', transaction => {
+          expect(transaction).toEqual(mockTransaction);
+        });
 
-    it('should call a beforeEnvelope hook', () => {
-      expect.assertions(1);
+        client.emit?.('startTransaction', mockTransaction);
+      });
 
-      const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN });
-      const client = new TestClient(options);
+      it('should call a beforeEnvelope hook', () => {
+        expect.assertions(1);
 
-      const mockEnvelope = [
-        {
-          event_id: '12345',
-        },
-        {},
-      ] as Envelope;
+        const mockEnvelope = [
+          {
+            event_id: '12345',
+          },
+          {},
+        ] as Envelope;
 
-      client?.on<EnvelopeHook>('beforeEnvelope', (envelope: Envelope) => {
-        expect(envelope).toEqual(mockEnvelope);
-      });
+        client.on?.('beforeEnvelope', envelope => {
+          expect(envelope).toEqual(mockEnvelope);
+        });
 
-      client?.emit<EnvelopeHook>('beforeEnvelope', mockEnvelope);
+        client.emit?.('beforeEnvelope', mockEnvelope);
+      });
     });
   });
 });

packages/types/src/client.ts

@@ -2,14 +2,15 @@ import type { EventDropReason } from './clientreport';
 import type { DataCategory } from './datacategory';
 import type { DsnComponents } from './dsn';
 import type { Event, EventHint } from './event';
-import type { Hook } from './hooks';
 import type { Integration, IntegrationClass } from './integration';
 import type { ClientOptions } from './options';
 import type { Scope } from './scope';
 import type { SdkMetadata } from './sdkmetadata';
 import type { Session, SessionAggregates } from './session';
 import type { Severity, SeverityLevel } from './severity';
 import type { Transport } from './transport';
+import { Transaction } from './transaction';
+import { Envelope } from './envelope';
 
 /**
  * User-Facing Sentry SDK Client.
@@ -155,11 +156,22 @@ export interface Client<O extends ClientOptions = ClientOptions> {
   /**
    * Register a callback for transaction start and finish.
    */
-  on?<HookType extends Hook>(hook: HookType['name'], callback: HookType['callback']): void;
+  on?(hook: 'startTransaction' | 'finishTransaction', callback: (transaction: Transaction) => void): void;
+
+  /**
+   * Register a callback for transaction start and finish.
+   */
+  on?(hook: 'beforeEnvelope', callback: (envelope: Envelope) => void): void;
+
+  /**
+   * Fire a hook event for transaction start and finish. Expects to be given a transaction as the
+   * second argument.
+   */
+  emit?(hook: 'startTransaction' | 'finishTransaction', transaction: Transaction): void;
 
   /*
    * Fire a hook event for envelope creation and sending. Expects to be given an envelope as the
    * second argument.
    */
-  emit?<HookType extends Hook>(hook: HookType['name'], ...args: Parameters<HookType['callback']>): void;
+  emit?(hook: 'beforeEnvelope', envelope: Envelope): void;
 }

packages/types/src/hooks.ts

@@ -96,4 +96,3 @@ export type { WrappedFunction } from './wrappedfunction';
 export type { Instrumenter } from './instrumenter';
 
 export type { BrowserClientReplayOptions } from './browseroptions';
-export type { HookStore, Hook, TransactionHook, EnvelopeHook } from './hooks';