Skip to content

Commit 797a4dd

Browse files
authored
feat: Avoid class fields all-together (#14887)
We already have an eslint rule to avoid class fields, but had exceptions for static fields as well as for arrow functions. This also leads to bundle size increases, so removing the exceptions and handling the (few) exceptions we have there should save some bytes. Additionally, this has additional challenges if we want to avoid/reduce polyfills, as class fields need to be polyfilled for ES2020, sadly. Found as part of #14882
1 parent 3aa9078 commit 797a4dd

File tree

15 files changed

+140
-159
lines changed

15 files changed

+140
-159
lines changed

docs/migration/v8-to-v9.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@ Sentry.init({
7878

7979
In v9, an `undefined` value will be treated the same as if the value is not defined at all. You'll need to set `tracesSampleRate: 0` if you want to enable tracing without performance.
8080

81+
- The `getCurrentHub().getIntegration(IntegrationClass)` method will always return `null` in v9. This has already stopped working mostly in v8, because we stopped exposing integration classes. In v9, the fallback behavior has been removed. Note that this does not change the type signature and is thus not technically breaking, but still worth pointing out.
82+
8183
### `@sentry/node`
8284

8385
- When `skipOpenTelemetrySetup: true` is configured, `httpIntegration({ spans: false })` will be configured by default. This means that you no longer have to specify this yourself in this scenario. With this change, no spans are emitted once `skipOpenTelemetrySetup: true` is configured, without any further configuration being needed.
@@ -208,6 +210,7 @@ This led to some duplication, where we had to keep an interface in `@sentry/type
208210
Since v9, the types have been merged into `@sentry/core`, which removed some of this duplication. This means that certain things that used to be a separate interface, will not expect an actual instance of the class/concrete implementation. This should not affect most users, unless you relied on passing things with a similar shape to internal methods. The following types are affected:
209211

210212
- `Scope` now always expects the `Scope` class
213+
- The `IntegrationClass` type is no longer exported - it was not used anymore. Instead, use `Integration` or `IntegrationFn`.
211214

212215
# No Version Support Timeline
213216

packages/angular/.eslintrc.cjs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,8 @@ module.exports = {
44
},
55
extends: ['../../.eslintrc.js'],
66
ignorePatterns: ['setup-test.ts', 'patch-vitest.ts'],
7+
rules: {
8+
// Angular transpiles this correctly/relies on this
9+
'@sentry-internal/sdk/no-class-field-initializers': 'off',
10+
},
711
};

packages/core/src/getCurrentHubShim.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
setUser,
1212
startSession,
1313
} from './exports';
14-
import type { Client, EventHint, Hub, Integration, IntegrationClass, SeverityLevel } from './types-hoist';
14+
import type { Client, EventHint, Hub, Integration, SeverityLevel } from './types-hoist';
1515

1616
/**
1717
* This is for legacy reasons, and returns a proxy object instead of a hub to be used.
@@ -48,9 +48,8 @@ export function getCurrentHubShim(): Hub {
4848
setExtras,
4949
setContext,
5050

51-
getIntegration<T extends Integration>(integration: IntegrationClass<T>): T | null {
52-
const client = getClient();
53-
return (client && client.getIntegrationByName<T>(integration.id)) || null;
51+
getIntegration<T extends Integration>(_integration: unknown): T | null {
52+
return null;
5453
},
5554

5655
startSession,

packages/core/src/types-hoist/hub.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { Breadcrumb, BreadcrumbHint } from './breadcrumb';
33
import type { Client } from './client';
44
import type { Event, EventHint } from './event';
55
import type { Extra, Extras } from './extra';
6-
import type { Integration, IntegrationClass } from './integration';
6+
import type { Integration } from './integration';
77
import type { Primitive } from './misc';
88
import type { Session } from './session';
99
import type { SeverityLevel } from './severity';
@@ -171,9 +171,9 @@ export interface Hub {
171171
/**
172172
* Returns the integration if installed on the current client.
173173
*
174-
* @deprecated Use `Sentry.getClient().getIntegration()` instead.
174+
* @deprecated Use `Sentry.getClient().getIntegrationByName()` instead.
175175
*/
176-
getIntegration<T extends Integration>(integration: IntegrationClass<T>): T | null;
176+
getIntegration<T extends Integration>(integration: unknown): T | null;
177177

178178
/**
179179
* Starts a new `Session`, sets on the current scope and returns it.

packages/core/src/types-hoist/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export type { Exception } from './exception';
5858
export type { Extra, Extras } from './extra';
5959
// eslint-disable-next-line deprecation/deprecation
6060
export type { Hub } from './hub';
61-
export type { Integration, IntegrationClass, IntegrationFn } from './integration';
61+
export type { Integration, IntegrationFn } from './integration';
6262
export type { Mechanism } from './mechanism';
6363
export type { ExtractedNodeRequestData, HttpHeaderValue, Primitive, WorkerLocation } from './misc';
6464
export type { ClientOptions, Options } from './options';

packages/core/src/types-hoist/integration.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,6 @@
11
import type { Client } from './client';
22
import type { Event, EventHint } from './event';
33

4-
/** Integration Class Interface */
5-
export interface IntegrationClass<T> {
6-
/**
7-
* Property that holds the integration name
8-
*/
9-
id: string;
10-
11-
new (...args: any[]): T;
12-
}
13-
144
/** Integration interface */
155
export interface Integration {
166
/**

packages/core/src/utils-hoist/syncpromise.ts

Lines changed: 44 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
/* eslint-disable @typescript-eslint/explicit-function-return-type */
21
/* eslint-disable @typescript-eslint/no-explicit-any */
32
import { isThenable } from './is';
43

@@ -40,29 +39,25 @@ export function rejectedSyncPromise<T = never>(reason?: any): PromiseLike<T> {
4039
});
4140
}
4241

42+
type Executor<T> = (resolve: (value?: T | PromiseLike<T> | null) => void, reject: (reason?: any) => void) => void;
43+
4344
/**
4445
* Thenable class that behaves like a Promise and follows it's interface
4546
* but is not async internally
4647
*/
47-
class SyncPromise<T> implements PromiseLike<T> {
48+
export class SyncPromise<T> implements PromiseLike<T> {
4849
private _state: States;
4950
private _handlers: Array<[boolean, (value: T) => void, (reason: any) => any]>;
5051
private _value: any;
5152

52-
public constructor(
53-
executor: (resolve: (value?: T | PromiseLike<T> | null) => void, reject: (reason?: any) => void) => void,
54-
) {
53+
public constructor(executor: Executor<T>) {
5554
this._state = States.PENDING;
5655
this._handlers = [];
5756

58-
try {
59-
executor(this._resolve, this._reject);
60-
} catch (e) {
61-
this._reject(e);
62-
}
57+
this._runExecutor(executor);
6358
}
6459

65-
/** JSDoc */
60+
/** @inheritdoc */
6661
public then<TResult1 = T, TResult2 = never>(
6762
onfulfilled?: ((value: T) => TResult1 | PromiseLike<TResult1>) | null,
6863
onrejected?: ((reason: any) => TResult2 | PromiseLike<TResult2>) | null,
@@ -99,14 +94,14 @@ class SyncPromise<T> implements PromiseLike<T> {
9994
});
10095
}
10196

102-
/** JSDoc */
97+
/** @inheritdoc */
10398
public catch<TResult = never>(
10499
onrejected?: ((reason: any) => TResult | PromiseLike<TResult>) | null,
105100
): PromiseLike<T | TResult> {
106101
return this.then(val => val, onrejected);
107102
}
108103

109-
/** JSDoc */
104+
/** @inheritdoc */
110105
public finally<TResult>(onfinally?: (() => void) | null): PromiseLike<TResult> {
111106
return new SyncPromise<TResult>((resolve, reject) => {
112107
let val: TResult | any;
@@ -138,35 +133,8 @@ class SyncPromise<T> implements PromiseLike<T> {
138133
});
139134
}
140135

141-
/** JSDoc */
142-
private readonly _resolve = (value?: T | PromiseLike<T> | null) => {
143-
this._setResult(States.RESOLVED, value);
144-
};
145-
146-
/** JSDoc */
147-
private readonly _reject = (reason?: any) => {
148-
this._setResult(States.REJECTED, reason);
149-
};
150-
151-
/** JSDoc */
152-
private readonly _setResult = (state: States, value?: T | PromiseLike<T> | any) => {
153-
if (this._state !== States.PENDING) {
154-
return;
155-
}
156-
157-
if (isThenable(value)) {
158-
void (value as PromiseLike<T>).then(this._resolve, this._reject);
159-
return;
160-
}
161-
162-
this._state = state;
163-
this._value = value;
164-
165-
this._executeHandlers();
166-
};
167-
168-
/** JSDoc */
169-
private readonly _executeHandlers = () => {
136+
/** Excute the resolve/reject handlers. */
137+
private _executeHandlers(): void {
170138
if (this._state === States.PENDING) {
171139
return;
172140
}
@@ -189,7 +157,38 @@ class SyncPromise<T> implements PromiseLike<T> {
189157

190158
handler[0] = true;
191159
});
192-
};
193-
}
160+
}
194161

195-
export { SyncPromise };
162+
/** Run the executor for the SyncPromise. */
163+
private _runExecutor(executor: Executor<T>): void {
164+
const setResult = (state: States, value?: T | PromiseLike<T> | any): void => {
165+
if (this._state !== States.PENDING) {
166+
return;
167+
}
168+
169+
if (isThenable(value)) {
170+
void (value as PromiseLike<T>).then(resolve, reject);
171+
return;
172+
}
173+
174+
this._state = state;
175+
this._value = value;
176+
177+
this._executeHandlers();
178+
};
179+
180+
const resolve = (value: unknown): void => {
181+
setResult(States.RESOLVED, value);
182+
};
183+
184+
const reject = (reason: unknown): void => {
185+
setResult(States.REJECTED, reason);
186+
};
187+
188+
try {
189+
executor(resolve, reject);
190+
} catch (e) {
191+
reject(e);
192+
}
193+
}
194+
}

packages/eslint-plugin-sdk/src/rules/no-class-field-initializers.js

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,7 @@ module.exports = {
2929
create(context) {
3030
return {
3131
'ClassProperty, PropertyDefinition'(node) {
32-
// We do allow arrow functions being initialized directly
33-
if (
34-
!node.static &&
35-
node.value !== null &&
36-
node.value.type !== 'ArrowFunctionExpression' &&
37-
node.value.type !== 'FunctionExpression' &&
38-
node.value.type !== 'CallExpression'
39-
) {
32+
if (node.value !== null) {
4033
context.report({
4134
node,
4235
message: `Avoid class field initializers. Property "${node.key.name}" should be initialized in the constructor.`,

packages/nestjs/src/integrations/sentry-nest-event-instrumentation.ts

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,14 @@ import { getEventSpanOptions } from './helpers';
1010
import type { OnEventTarget } from './types';
1111

1212
const supportedVersions = ['>=2.0.0'];
13+
const COMPONENT = '@nestjs/event-emitter';
1314

1415
/**
1516
* Custom instrumentation for nestjs event-emitter
1617
*
1718
* This hooks into the `OnEvent` decorator, which is applied on event handlers.
1819
*/
1920
export class SentryNestEventInstrumentation extends InstrumentationBase {
20-
public static readonly COMPONENT = '@nestjs/event-emitter';
21-
public static readonly COMMON_ATTRIBUTES = {
22-
component: SentryNestEventInstrumentation.COMPONENT,
23-
};
24-
2521
public constructor(config: InstrumentationConfig = {}) {
2622
super('sentry-nestjs-event', SDK_VERSION, config);
2723
}
@@ -30,10 +26,7 @@ export class SentryNestEventInstrumentation extends InstrumentationBase {
3026
* Initializes the instrumentation by defining the modules to be patched.
3127
*/
3228
public init(): InstrumentationNodeModuleDefinition {
33-
const moduleDef = new InstrumentationNodeModuleDefinition(
34-
SentryNestEventInstrumentation.COMPONENT,
35-
supportedVersions,
36-
);
29+
const moduleDef = new InstrumentationNodeModuleDefinition(COMPONENT, supportedVersions);
3730

3831
moduleDef.files.push(this._getOnEventFileInstrumentation(supportedVersions));
3932
return moduleDef;

packages/nestjs/src/integrations/sentry-nest-instrumentation.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { getMiddlewareSpanOptions, getNextProxy, instrumentObservable, isPatched
2020
import type { CallHandler, CatchTarget, InjectableTarget, MinimalNestJsExecutionContext, Observable } from './types';
2121

2222
const supportedVersions = ['>=8.0.0 <11'];
23+
const COMPONENT = '@nestjs/common';
2324

2425
/**
2526
* Custom instrumentation for nestjs.
@@ -29,11 +30,6 @@ const supportedVersions = ['>=8.0.0 <11'];
2930
* 2. @Catch decorator, which is applied on exception filters.
3031
*/
3132
export class SentryNestInstrumentation extends InstrumentationBase {
32-
public static readonly COMPONENT = '@nestjs/common';
33-
public static readonly COMMON_ATTRIBUTES = {
34-
component: SentryNestInstrumentation.COMPONENT,
35-
};
36-
3733
public constructor(config: InstrumentationConfig = {}) {
3834
super('sentry-nestjs', SDK_VERSION, config);
3935
}
@@ -42,7 +38,7 @@ export class SentryNestInstrumentation extends InstrumentationBase {
4238
* Initializes the instrumentation by defining the modules to be patched.
4339
*/
4440
public init(): InstrumentationNodeModuleDefinition {
45-
const moduleDef = new InstrumentationNodeModuleDefinition(SentryNestInstrumentation.COMPONENT, supportedVersions);
41+
const moduleDef = new InstrumentationNodeModuleDefinition(COMPONENT, supportedVersions);
4642

4743
moduleDef.files.push(
4844
this._getInjectableFileInstrumentation(supportedVersions),

packages/react/src/errorboundary.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,14 +145,14 @@ class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundarySta
145145
}
146146
}
147147

148-
public resetErrorBoundary: () => void = () => {
148+
public resetErrorBoundary(): void {
149149
const { onReset } = this.props;
150150
const { error, componentStack, eventId } = this.state;
151151
if (onReset) {
152152
onReset(error, componentStack, eventId);
153153
}
154154
this.setState(INITIAL_STATE);
155-
};
155+
}
156156

157157
public render(): React.ReactNode {
158158
const { fallback, children } = this.props;
@@ -164,7 +164,7 @@ class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundarySta
164164
element = React.createElement(fallback, {
165165
error: state.error,
166166
componentStack: state.componentStack as string,
167-
resetError: this.resetErrorBoundary,
167+
resetError: this.resetErrorBoundary.bind(this),
168168
eventId: state.eventId as string,
169169
});
170170
} else {

packages/react/src/profiler.tsx

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,6 @@ class Profiler extends React.Component<ProfilerProps> {
3939
*/
4040
protected _updateSpan: Span | undefined;
4141

42-
// eslint-disable-next-line @typescript-eslint/member-ordering
43-
public static defaultProps: Partial<ProfilerProps> = {
44-
disabled: false,
45-
includeRender: true,
46-
includeUpdates: true,
47-
};
48-
4942
public constructor(props: ProfilerProps) {
5043
super(props);
5144
const { name, disabled = false } = this.props;
@@ -141,6 +134,15 @@ class Profiler extends React.Component<ProfilerProps> {
141134
}
142135
}
143136

137+
// React.Component default props are defined as static property on the class
138+
Object.assign(Profiler, {
139+
defaultProps: {
140+
disabled: false,
141+
includeRender: true,
142+
includeUpdates: true,
143+
},
144+
});
145+
144146
/**
145147
* withProfiler is a higher order component that wraps a
146148
* component in a {@link Profiler} component. It is recommended that

packages/replay-internal/src/integration.ts

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,16 +46,8 @@ export const replayIntegration = ((options?: ReplayConfiguration) => {
4646

4747
/**
4848
* Replay integration
49-
*
50-
* TODO: Rewrite this to be functional integration
51-
* Exported for tests.
5249
*/
5350
export class Replay implements Integration {
54-
/**
55-
* @inheritDoc
56-
*/
57-
public static id: string = 'Replay';
58-
5951
/**
6052
* @inheritDoc
6153
*/
@@ -114,7 +106,7 @@ export class Replay implements Integration {
114106
beforeErrorSampling,
115107
onError,
116108
}: ReplayConfiguration = {}) {
117-
this.name = Replay.id;
109+
this.name = 'Replay';
118110

119111
const privacyOptions = getPrivacyOptions({
120112
mask,

0 commit comments

Comments
 (0)