From 52932057109921b905c35b494253ad2d33325033 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 26 Jan 2024 11:49:47 -0500 Subject: [PATCH 1/2] fix: Ensure `afterAllSetup` is called when using `addIntegration()` --- packages/core/src/baseclient.ts | 1 + packages/core/test/lib/integration.test.ts | 25 ++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index a4d43fc58a8a..7dd4bdf03ff4 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -369,6 +369,7 @@ export abstract class BaseClient implements Client { */ public addIntegration(integration: Integration): void { setupIntegration(this, integration, this._integrations); + afterSetupIntegrations(this, [integration]); } /** diff --git a/packages/core/test/lib/integration.test.ts b/packages/core/test/lib/integration.test.ts index ccc6476c14d8..02a717b36090 100644 --- a/packages/core/test/lib/integration.test.ts +++ b/packages/core/test/lib/integration.test.ts @@ -646,6 +646,31 @@ describe('addIntegration', () => { expect(warnings).toHaveBeenCalledTimes(1); expect(warnings).toHaveBeenCalledWith('Cannot add integration "test" because no SDK Client is available.'); }); + + it('triggers all hooks', () => { + const setup = jest.fn(); + const setupOnce = jest.fn(); + const setupAfterAll = jest.fn(); + + class CustomIntegration implements Integration { + name = 'test'; + setupOnce = setupOnce; + setup = setup; + afterAllSetup = setupAfterAll; + } + + const client = getTestClient(); + const hub = new Hub(client); + // eslint-disable-next-line deprecation/deprecation + makeMain(hub); + + const integration = new CustomIntegration(); + addIntegration(integration); + + expect(setupOnce).toHaveBeenCalledTimes(1); + expect(setup).toHaveBeenCalledTimes(1); + expect(setupAfterAll).toHaveBeenCalledTimes(1); + }); }); describe('convertIntegrationFnToClass', () => { From 9fcabc28f4ce1e6a5a00f6d6781945d41823fe76 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 26 Jan 2024 12:39:58 -0500 Subject: [PATCH 2/2] better behavior & test --- packages/core/src/baseclient.ts | 8 +++++- packages/core/test/lib/integration.test.ts | 32 ++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 7dd4bdf03ff4..c64c5cf92c0d 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -368,8 +368,14 @@ export abstract class BaseClient implements Client { * @inheritDoc */ public addIntegration(integration: Integration): void { + const isAlreadyInstalled = this._integrations[integration.name]; + + // This hook takes care of only installing if not already installed setupIntegration(this, integration, this._integrations); - afterSetupIntegrations(this, [integration]); + // Here we need to check manually to make sure to not run this multiple times + if (!isAlreadyInstalled) { + afterSetupIntegrations(this, [integration]); + } } /** diff --git a/packages/core/test/lib/integration.test.ts b/packages/core/test/lib/integration.test.ts index 02a717b36090..e819f9413aec 100644 --- a/packages/core/test/lib/integration.test.ts +++ b/packages/core/test/lib/integration.test.ts @@ -671,6 +671,38 @@ describe('addIntegration', () => { expect(setup).toHaveBeenCalledTimes(1); expect(setupAfterAll).toHaveBeenCalledTimes(1); }); + + it('does not trigger hooks if already installed', () => { + const logs = jest.spyOn(logger, 'log'); + + class CustomIntegration implements Integration { + name = 'test'; + setupOnce = jest.fn(); + setup = jest.fn(); + afterAllSetup = jest.fn(); + } + + const client = getTestClient(); + const hub = new Hub(client); + // eslint-disable-next-line deprecation/deprecation + makeMain(hub); + + const integration1 = new CustomIntegration(); + const integration2 = new CustomIntegration(); + addIntegration(integration1); + + expect(integration1.setupOnce).toHaveBeenCalledTimes(1); + expect(integration1.setup).toHaveBeenCalledTimes(1); + expect(integration1.afterAllSetup).toHaveBeenCalledTimes(1); + + addIntegration(integration2); + + expect(integration2.setupOnce).toHaveBeenCalledTimes(0); + expect(integration2.setup).toHaveBeenCalledTimes(0); + expect(integration2.afterAllSetup).toHaveBeenCalledTimes(0); + + expect(logs).toHaveBeenCalledWith('Integration skipped because it was already installed: test'); + }); }); describe('convertIntegrationFnToClass', () => {