sentry-javascript icon indicating copy to clipboard operation
sentry-javascript copied to clipboard

Make sure field initializers don't add bundle size

Open AbhiPrasad opened this issue 3 years ago • 1 comments

Problem Statement

When using field initializers with typescript, our bundling process does something like so:

constructor() {
  At.prototype.__init.call(this), At.prototype.__init2.call(this), At.prototype.__init3.call(this), At.prototype.__init4.call(this), At.prototype.__init5.call(this), At.prototype.__init6.call(this), At.prototype.__init7.call(this), At.prototype.__init8.call(this), At.prototype.__init9.call(this), At.prototype.__init10.call(this)
}
__init() {
  this.m = !1
}
__init2() {
  this.g = []
}
__init3() {
  this._ = []
}
__init4() {
  this.S = []
}
__init5() {
  this.k = {}
}
__init6() {
  this.O = {}
}
__init7() {
  this.j = {}
}
__init8() {
  this.T = {}
}
__init9() {
  this.D = []
}
__init10() {
  this.R = {}
}

Each __initX() call is generated from a field initializer of a class. This is a bunch of extra bytes. See an example here: https://sucrase.io/#selectedTransforms=typescript,imports&code=class%20Scope%20%7B%0A%20%20public%20a%20%3D%203%3B%0A%7D.

This is done to best match TS behaviour. See https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-7.html#the-usedefineforclassfields-flag-and-the-declare-property-modifier for more details.

Solution Brainstorm

Evidence: https://github.com/getsentry/sentry-javascript/pull/5306

In the mean time, I think avoiding usage of field initializers in favour of initializing the field in a constructor is the best course of action to reduce bundle size.

AbhiPrasad avatar Jun 24 '22 19:06 AbhiPrasad

diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts
index 09c40a5aa..68ebd4cac 100644
--- a/packages/core/src/baseclient.ts
+++ b/packages/core/src/baseclient.ts
@@ -75,6 +75,15 @@ const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been ca
  * }
  */
 export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
+  /** Array of set up integrations. */
+  protected _integrations: IntegrationIndex;
+
+  /** Indicates whether this client's integrations have been set up. */
+  protected _integrationsInitialized: boolean;
+
+  /** Number of calls being processed */
+  protected _numProcessing: number;
+
   /** Options passed to the SDK. */
   protected readonly _options: O;
 
@@ -83,17 +92,8 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
 
   protected readonly _transport?: Transport;
 
-  /** Array of set up integrations. */
-  protected _integrations: IntegrationIndex = {};
-
-  /** Indicates whether this client's integrations have been set up. */
-  protected _integrationsInitialized: boolean = false;
-
-  /** Number of calls being processed */
-  protected _numProcessing: number = 0;
-
   /** Holds flushable  */
-  private _outcomes: { [key: string]: number } = {};
+  private _outcomes: { [key: string]: number };
 
   /**
    * Initializes this client instance.
@@ -102,6 +102,10 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
    */
   protected constructor(options: O) {
     this._options = options;
+    this._integrations = {};
+    this._integrationsInitialized = false;
+    this._numProcessing = 0;
+    this._outcomes = {};
     if (options.dsn) {
       this._dsn = makeDsn(options.dsn);
       const url = getEnvelopeEndpointWithUrlEncodedAuth(this._dsn, options.tunnel);
diff --git a/packages/hub/src/hub.ts b/packages/hub/src/hub.ts
index 9a2d30660..ed6fc7747 100644
--- a/packages/hub/src/hub.ts
+++ b/packages/hub/src/hub.ts
@@ -86,7 +86,7 @@ export interface Carrier {
  */
 export class Hub implements HubInterface {
   /** Is a {@link Layer}[] containing the client and scope */
-  private readonly _stack: Layer[] = [{}];
+  private readonly _stack: Layer[];
 
   /** Contains the last event id of a captured event.  */
   private _lastEventId?: string;
@@ -100,6 +100,7 @@ export class Hub implements HubInterface {
    * @param version number, higher number means higher priority.
    */
   public constructor(client?: Client, scope: Scope = new Scope(), private readonly _version: number = API_VERSION) {
+    this._stack = [{}];
     this.getStackTop().scope = scope;
     if (client) {
       this.bindClient(client);
diff --git a/packages/utils/src/syncpromise.ts b/packages/utils/src/syncpromise.ts
index ec5cd9e9f..5a7980b87 100644
--- a/packages/utils/src/syncpromise.ts
+++ b/packages/utils/src/syncpromise.ts
@@ -47,13 +47,15 @@ export function rejectedSyncPromise<T = never>(reason?: any): PromiseLike<T> {
  * but is not async internally
  */
 class SyncPromise<T> implements PromiseLike<T> {
-  private _state: States = States.PENDING;
-  private _handlers: Array<[boolean, (value: T) => void, (reason: any) => any]> = [];
+  private _state: States;
+  private _handlers: Array<[boolean, (value: T) => void, (reason: any) => any]>;
   private _value: any;
 
   public constructor(
     executor: (resolve: (value?: T | PromiseLike<T> | null) => void, reject: (reason?: any) => void) => void,
   ) {
+    this._state = States.PENDING;
+    this._handlers = [];
     try {
       executor(this._resolve, this._reject);
     } catch (e) {

Seems to give some decent wins!

AbhiPrasad avatar Jun 24 '22 20:06 AbhiPrasad

Can we add a linter rule for this, otherwise it won't happen.

HazAT avatar Jan 26 '23 08:01 HazAT

Yeah we would need to add a linter to make sure this doesn't regress, which shouldn't be too bad to add.

AbhiPrasad avatar Jan 30 '23 09:01 AbhiPrasad