Skip to content

Commit

Permalink
Fix actions loading (#710)
Browse files Browse the repository at this point in the history
  • Loading branch information
chrisradek authored Dec 2, 2022
1 parent c5d2a99 commit ef5cd39
Show file tree
Hide file tree
Showing 3 changed files with 191 additions and 6 deletions.
5 changes: 5 additions & 0 deletions .changeset/breezy-kids-clean.md
Original file line number Diff line number Diff line change
@@ -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.
161 changes: 161 additions & 0 deletions packages/browser/src/plugins/remote-loader/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
31 changes: 25 additions & 6 deletions packages/browser/src/plugins/remote-loader/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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('/')
Expand Down

0 comments on commit ef5cd39

Please sign in to comment.