Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix actions loading #710

Merged
merged 4 commits into from
Dec 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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