Skip to content

Commit 01a4cc9

Browse files
authored
feat(core): Add type & utility for function-based integrations (#9818)
This PR adds new types for function-based integrations, that eventually (in v8) should fully replace the class-based functions. This also introduces a small helper function to make writing such integrations easier (as we need to set an id/name etc. on the integration). With this, you can write an integration like this: ```ts const inboundFiltersIntegration = makeIntegrationFn( 'InboundFilters', (options: Partial<InboundFiltersOptions>) => { return { processEvent(event, _hint, client) { const clientOptions = client.getOptions(); const mergedOptions = _mergeOptions(options, clientOptions); return _shouldDropEvent(event, mergedOptions) ? null : event; } } }); ``` And you get a fully typed integration ready to go! For backwards compatibility, and so that we can actually start converting integrations in v7 already, this PR also adds a small utility `convertIntegrationFnToClass()` to convert such an integration to the "current" integration class syntax. So we can actually already start porting integrations over like this: ```js /** Inbound filters configurable by the user */ // eslint-disable-next-line deprecation/deprecation export const InboundFilters = convertIntegrationFnToClass(inboundFiltersIntegration); ``` Then, in v8 we only have to remove all the `convertIntegrationFnToClass` calls, export the integration functions directly, and update the overall integration types which can be passed to `init()` etc.
1 parent f2a4caa commit 01a4cc9

File tree

6 files changed

+145
-41
lines changed

6 files changed

+145
-41
lines changed

packages/core/src/index.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,12 @@ export { createTransport } from './transports/base';
5454
export { makeOfflineTransport } from './transports/offline';
5555
export { makeMultiplexedTransport } from './transports/multiplexed';
5656
export { SDK_VERSION } from './version';
57-
export { getIntegrationsToSetup, addIntegration } from './integration';
57+
export {
58+
getIntegrationsToSetup,
59+
addIntegration,
60+
// eslint-disable-next-line deprecation/deprecation
61+
convertIntegrationFnToClass,
62+
} from './integration';
5863
export { FunctionToString, InboundFilters, LinkedErrors } from './integrations';
5964
export { prepareEvent } from './utils/prepareEvent';
6065
export { createCheckInEnvelope } from './checkin';

packages/core/src/integration.ts

+24-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { Client, Event, EventHint, Integration, Options } from '@sentry/types';
1+
import type { Client, Event, EventHint, Integration, IntegrationClass, IntegrationFn, Options } from '@sentry/types';
22
import { arrayify, logger } from '@sentry/utils';
33

44
import { DEBUG_BUILD } from './debug-build';
@@ -155,3 +155,26 @@ function findIndex<T>(arr: T[], callback: (item: T) => boolean): number {
155155

156156
return -1;
157157
}
158+
159+
/**
160+
* Convert a new integration function to the legacy class syntax.
161+
* In v8, we can remove this and instead export the integration functions directly.
162+
*
163+
* @deprecated This will be removed in v8!
164+
*/
165+
export function convertIntegrationFnToClass<Fn extends IntegrationFn>(
166+
name: string,
167+
fn: Fn,
168+
): IntegrationClass<Integration> {
169+
return Object.assign(
170+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
171+
function ConvertedIntegration(...rest: any[]) {
172+
return {
173+
// eslint-disable-next-line @typescript-eslint/no-empty-function
174+
setupOnce: () => {},
175+
...fn(...rest),
176+
};
177+
},
178+
{ id: name },
179+
) as unknown as IntegrationClass<Integration>;
180+
}

packages/core/src/integrations/inboundfilters.ts

+18-37
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
import type { Client, Event, EventHint, Integration, StackFrame } from '@sentry/types';
1+
import type { Event, IntegrationFn, StackFrame } from '@sentry/types';
22
import { getEventDescription, logger, stringMatchesSomePattern } from '@sentry/utils';
33

44
import { DEBUG_BUILD } from '../debug-build';
5+
import { convertIntegrationFnToClass } from '../integration';
56

67
// "Script error." is hard coded into browsers for errors that it can't read.
78
// this is the result of a script being pulled in from an external domain and CORS.
@@ -28,42 +29,23 @@ export interface InboundFiltersOptions {
2829
disableTransactionDefaults: boolean;
2930
}
3031

31-
/** Inbound filters configurable by the user */
32-
export class InboundFilters implements Integration {
33-
/**
34-
* @inheritDoc
35-
*/
36-
public static id: string = 'InboundFilters';
37-
38-
/**
39-
* @inheritDoc
40-
*/
41-
public name: string;
42-
43-
private readonly _options: Partial<InboundFiltersOptions>;
44-
45-
public constructor(options: Partial<InboundFiltersOptions> = {}) {
46-
this.name = InboundFilters.id;
47-
this._options = options;
48-
}
49-
50-
/**
51-
* @inheritDoc
52-
*/
53-
public setupOnce(_addGlobalEventProcessor: unknown, _getCurrentHub: unknown): void {
54-
// noop
55-
}
32+
const INTEGRATION_NAME = 'InboundFilters';
33+
const inboundFiltersIntegration: IntegrationFn = (options: Partial<InboundFiltersOptions>) => {
34+
return {
35+
name: INTEGRATION_NAME,
36+
processEvent(event, _hint, client) {
37+
const clientOptions = client.getOptions();
38+
const mergedOptions = _mergeOptions(options, clientOptions);
39+
return _shouldDropEvent(event, mergedOptions) ? null : event;
40+
},
41+
};
42+
};
5643

57-
/** @inheritDoc */
58-
public processEvent(event: Event, _eventHint: EventHint, client: Client): Event | null {
59-
const clientOptions = client.getOptions();
60-
const options = _mergeOptions(this._options, clientOptions);
61-
return _shouldDropEvent(event, options) ? null : event;
62-
}
63-
}
44+
/** Inbound filters configurable by the user */
45+
// eslint-disable-next-line deprecation/deprecation
46+
export const InboundFilters = convertIntegrationFnToClass(INTEGRATION_NAME, inboundFiltersIntegration);
6447

65-
/** JSDoc */
66-
export function _mergeOptions(
48+
function _mergeOptions(
6749
internalOptions: Partial<InboundFiltersOptions> = {},
6850
clientOptions: Partial<InboundFiltersOptions> = {},
6951
): Partial<InboundFiltersOptions> {
@@ -84,8 +66,7 @@ export function _mergeOptions(
8466
};
8567
}
8668

87-
/** JSDoc */
88-
export function _shouldDropEvent(event: Event, options: Partial<InboundFiltersOptions>): boolean {
69+
function _shouldDropEvent(event: Event, options: Partial<InboundFiltersOptions>): boolean {
8970
if (options.ignoreInternal && _isSentryError(event)) {
9071
DEBUG_BUILD &&
9172
logger.warn(`Event dropped due to being internal Sentry Error.\nEvent: ${getEventDescription(event)}`);

packages/core/test/lib/integration.test.ts

+55-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,13 @@ import type { Integration, Options } from '@sentry/types';
22
import { logger } from '@sentry/utils';
33

44
import { Hub, makeMain } from '../../src/hub';
5-
import { addIntegration, getIntegrationsToSetup, installedIntegrations, setupIntegration } from '../../src/integration';
5+
import {
6+
addIntegration,
7+
convertIntegrationFnToClass,
8+
getIntegrationsToSetup,
9+
installedIntegrations,
10+
setupIntegration,
11+
} from '../../src/integration';
612
import { TestClient, getDefaultTestClientOptions } from '../mocks/client';
713

814
function getTestClient(): TestClient {
@@ -647,3 +653,51 @@ describe('addIntegration', () => {
647653
expect(warnings).toHaveBeenCalledWith('Cannot add integration "test" because no SDK Client is available.');
648654
});
649655
});
656+
657+
describe('convertIntegrationFnToClass', () => {
658+
/* eslint-disable deprecation/deprecation */
659+
it('works with a minimal integration', () => {
660+
const integrationFn = () => ({ name: 'testName' });
661+
662+
const IntegrationClass = convertIntegrationFnToClass('testName', integrationFn);
663+
664+
expect(IntegrationClass.id).toBe('testName');
665+
666+
const integration = new IntegrationClass();
667+
expect(integration).toEqual({
668+
name: 'testName',
669+
setupOnce: expect.any(Function),
670+
});
671+
});
672+
673+
it('works with integration hooks', () => {
674+
const setup = jest.fn();
675+
const setupOnce = jest.fn();
676+
const processEvent = jest.fn();
677+
const preprocessEvent = jest.fn();
678+
679+
const integrationFn = () => {
680+
return {
681+
name: 'testName',
682+
setup,
683+
setupOnce,
684+
processEvent,
685+
preprocessEvent,
686+
};
687+
};
688+
689+
const IntegrationClass = convertIntegrationFnToClass('testName', integrationFn);
690+
691+
expect(IntegrationClass.id).toBe('testName');
692+
693+
const integration = new IntegrationClass();
694+
expect(integration).toEqual({
695+
name: 'testName',
696+
setupOnce,
697+
setup,
698+
processEvent,
699+
preprocessEvent,
700+
});
701+
});
702+
/* eslint-enable deprecation/deprecation */
703+
});

packages/types/src/index.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ export type { EventProcessor } from './eventprocessor';
5252
export type { Exception } from './exception';
5353
export type { Extra, Extras } from './extra';
5454
export type { Hub } from './hub';
55-
export type { Integration, IntegrationClass } from './integration';
55+
export type { Integration, IntegrationClass, IntegrationFn, IntegrationFnResult } from './integration';
5656
export type { Mechanism } from './mechanism';
5757
export type { ExtractedNodeRequestData, HttpHeaderValue, Primitive, WorkerLocation } from './misc';
5858
export type { ClientOptions, Options } from './options';

packages/types/src/integration.ts

+41
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,47 @@ export interface IntegrationClass<T> {
1313
new (...args: any[]): T;
1414
}
1515

16+
/**
17+
* An integration in function form.
18+
* This is expected to return an integration result,
19+
*/
20+
export type IntegrationFn = (...rest: any[]) => IntegrationFnResult;
21+
22+
export interface IntegrationFnResult {
23+
/**
24+
* The name of the integration.
25+
*/
26+
name: string;
27+
28+
/**
29+
* This hook is only called once, even if multiple clients are created.
30+
* It does not receives any arguments, and should only use for e.g. global monkey patching and similar things.
31+
*/
32+
setupOnce?(): void;
33+
34+
/**
35+
* Set up an integration for the given client.
36+
* Receives the client as argument.
37+
*
38+
* Whenever possible, prefer this over `setupOnce`, as that is only run for the first client,
39+
* whereas `setup` runs for each client. Only truly global things (e.g. registering global handlers)
40+
* should be done in `setupOnce`.
41+
*/
42+
setup?(client: Client): void;
43+
44+
/**
45+
* An optional hook that allows to preprocess an event _before_ it is passed to all other event processors.
46+
*/
47+
preprocessEvent?(event: Event, hint: EventHint | undefined, client: Client): void;
48+
49+
/**
50+
* An optional hook that allows to process an event.
51+
* Return `null` to drop the event, or mutate the event & return it.
52+
* This receives the client that the integration was installed for as third argument.
53+
*/
54+
processEvent?(event: Event, hint: EventHint, client: Client): Event | null | PromiseLike<Event | null>;
55+
}
56+
1657
/** Integration interface */
1758
export interface Integration {
1859
/**

0 commit comments

Comments
 (0)