From ef5cd39ce3a83321f7bf7792c2e68d40ce2be1c6 Mon Sep 17 00:00:00 2001 From: Christopher Radek <14189820+chrisradek@users.noreply.github.com> Date: Fri, 2 Dec 2022 15:34:18 -0800 Subject: [PATCH] Fix actions loading (#710) --- .changeset/breezy-kids-clean.md | 5 + .../remote-loader/__tests__/index.test.ts | 161 ++++++++++++++++++ .../src/plugins/remote-loader/index.ts | 31 +++- 3 files changed, 191 insertions(+), 6 deletions(-) create mode 100644 .changeset/breezy-kids-clean.md diff --git a/.changeset/breezy-kids-clean.md b/.changeset/breezy-kids-clean.md new file mode 100644 index 000000000..744d29372 --- /dev/null +++ b/.changeset/breezy-kids-clean.md @@ -0,0 +1,5 @@ +--- +'@segment/analytics-next': patch +--- + +Fixes an issue impacting a small number of destinations where explicitly enabling or disabling an integration on load would not work as expected. diff --git a/packages/browser/src/plugins/remote-loader/__tests__/index.test.ts b/packages/browser/src/plugins/remote-loader/__tests__/index.test.ts index 4014300af..1044511ca 100644 --- a/packages/browser/src/plugins/remote-loader/__tests__/index.test.ts +++ b/packages/browser/src/plugins/remote-loader/__tests__/index.test.ts @@ -533,6 +533,167 @@ describe('Remote Loader', () => { ) }) + // Action destinations should be toggled using the `integration` name which matches the `creationName` + // Most action destinations have the same value for creationName and name, but a few (Amplitude) do not. + // Continue to support toggling with plugin.name for backwards compatibility. + it('loads destinations when `All: false` but is enabled (pluginName)', async () => { + const cdnSettings = { + integrations: { + oldValidName: { + versionSettings: { + componentTypes: [], + }, + }, + }, + remotePlugins: [ + { + name: 'valid', + creationName: 'oldValidName', + url: 'cdn/path/to/file.js', + libraryName: 'testPlugin', + settings: { + subscriptions: [], + versionSettings: { + componentTypes: [], + }, + }, + }, + ], + } + + await AnalyticsBrowser.load( + { writeKey: '', cdnSettings }, + { + integrations: { + All: false, + valid: true, + }, + } + ) + + expect(pluginFactory).toHaveBeenCalledTimes(1) + expect(pluginFactory).toHaveBeenCalledWith( + expect.objectContaining(cdnSettings.remotePlugins[0].settings) + ) + }) + + it('loads destinations when `All: false` but is enabled (creationName)', async () => { + const cdnSettings = { + integrations: { + oldValidName: { + versionSettings: { + componentTypes: [], + }, + }, + }, + remotePlugins: [ + { + name: 'valid', + creationName: 'oldValidName', + url: 'cdn/path/to/file.js', + libraryName: 'testPlugin', + settings: { + subscriptions: [], + versionSettings: { + componentTypes: [], + }, + }, + }, + ], + } + + await AnalyticsBrowser.load( + { writeKey: '', cdnSettings }, + { + integrations: { + All: false, + oldValidName: true, + }, + } + ) + + expect(pluginFactory).toHaveBeenCalledTimes(1) + expect(pluginFactory).toHaveBeenCalledWith( + expect.objectContaining(cdnSettings.remotePlugins[0].settings) + ) + }) + + it('does not load destinations when disabled via pluginName', async () => { + const cdnSettings = { + integrations: { + oldValidName: { + versionSettings: { + componentTypes: [], + }, + }, + }, + remotePlugins: [ + { + name: 'valid', + creationName: 'oldValidName', + url: 'cdn/path/to/file.js', + libraryName: 'testPlugin', + settings: { + subscriptions: [], + versionSettings: { + componentTypes: [], + }, + }, + }, + ], + } + + await AnalyticsBrowser.load( + { writeKey: '', cdnSettings }, + { + integrations: { + All: true, + valid: false, + }, + } + ) + + expect(pluginFactory).toHaveBeenCalledTimes(0) + }) + + it('does not load destinations when disabled via creationName', async () => { + const cdnSettings = { + integrations: { + oldValidName: { + versionSettings: { + componentTypes: [], + }, + }, + }, + remotePlugins: [ + { + name: 'valid', + creationName: 'oldValidName', + url: 'cdn/path/to/file.js', + libraryName: 'testPlugin', + settings: { + subscriptions: [], + versionSettings: { + componentTypes: [], + }, + }, + }, + ], + } + + await AnalyticsBrowser.load( + { writeKey: '', cdnSettings }, + { + integrations: { + All: true, + oldValidName: false, + }, + } + ) + + expect(pluginFactory).toHaveBeenCalledTimes(0) + }) + it('applies remote routing rules based on creation name', async () => { const validPlugin = { name: 'valid', diff --git a/packages/browser/src/plugins/remote-loader/index.ts b/packages/browser/src/plugins/remote-loader/index.ts index d66811e55..b45e622b7 100644 --- a/packages/browser/src/plugins/remote-loader/index.ts +++ b/packages/browser/src/plugins/remote-loader/index.ts @@ -135,6 +135,30 @@ function validate(pluginLike: unknown): pluginLike is Plugin[] { return true } +function isPluginDisabled( + userIntegrations: Integrations, + remotePlugin: RemotePlugin +) { + const creationNameEnabled = userIntegrations[remotePlugin.creationName] + const currentNameEnabled = userIntegrations[remotePlugin.name] + + // Check that the plugin isn't explicitly enabled when All: false + if ( + userIntegrations.All === false && + !creationNameEnabled && + !currentNameEnabled + ) { + return true + } + + // Check that the plugin isn't explicitly disabled + if (creationNameEnabled === false || currentNameEnabled === false) { + return true + } + + return false +} + export async function remoteLoader( settings: LegacySettings, userIntegrations: Integrations, @@ -149,12 +173,7 @@ export async function remoteLoader( const pluginPromises = (settings.remotePlugins ?? []).map( async (remotePlugin) => { - if ( - (userIntegrations.All === false && - !userIntegrations[remotePlugin.name]) || - userIntegrations[remotePlugin.name] === false - ) - return + if (isPluginDisabled(userIntegrations, remotePlugin)) return try { if (obfuscate) { const urlSplit = remotePlugin.url.split('/')