fix some break others

PostHog · Nov 25, 2024 · a365dfb · a365dfb
1 parent 3a9a317
commit a365dfb
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 44 deletions.
diff --git a/src/__tests__/decide.ts b/src/__tests__/decide.ts
@@ -1,7 +1,6 @@
 import { Decide } from '../decide'
 import { PostHogPersistence } from '../posthog-persistence'
 import { RequestRouter } from '../utils/request-router'
-import { expectScriptToExist, expectScriptToNotExist } from './helpers/script-utils'
 import { PostHog } from '../posthog-core'
 import { DecideResponse, PostHogConfig, Properties } from '../types'
 import '../entrypoints/external-scripts-loader'
@@ -246,35 +245,5 @@ describe('Decide', () => {
             expect(posthog._afterDecideResponse).toHaveBeenCalledWith(decideResponse)
             expect(posthog.featureFlags.receivedFeatureFlags).not.toHaveBeenCalled()
         })
-
-        it('runs site apps if opted in', () => {
-            posthog.config = {
-                api_host: 'https://test.com',
-                opt_in_site_apps: true,
-                persistence: 'memory',
-            } as PostHogConfig
-
-            subject({ siteApps: [{ id: 1, url: '/site_app/1/tokentoken/hash/' }] } as DecideResponse)
-
-            expectScriptToExist('https://test.com/site_app/1/tokentoken/hash/')
-        })
-
-        it('does not run site apps code if not opted in', () => {
-            ;(window as any).POSTHOG_DEBUG = true
-            // don't technically need to run this but this test assumes opt_in_site_apps is false, let's make that explicit
-            posthog.config = {
-                api_host: 'https://test.com',
-                opt_in_site_apps: false,
-                persistence: 'memory',
-            } as unknown as PostHogConfig
-
-            subject({ siteApps: [{ id: 1, url: '/site_app/1/tokentoken/hash/' }] } as DecideResponse)
-
-            expect(console.error).toHaveBeenCalledWith(
-                '[PostHog.js]',
-                'PostHog site apps are disabled. Enable the "opt_in_site_apps" config to proceed.'
-            )
-            expectScriptToNotExist('https://test.com/site_app/1/tokentoken/hash/')
-        })
     })
 })
diff --git a/src/entrypoints/external-scripts-loader.ts b/src/entrypoints/external-scripts-loader.ts
@@ -2,11 +2,7 @@ import type { PostHog } from '../posthog-core'
 import { assignableWindow, document, PostHogExtensionKind } from '../utils/globals'
 import { logger } from '../utils/logger'
 
-export const loadScript = (
-    posthog: PostHog,
-    url: string,
-    callback: (error?: string | Event, event?: Event) => void
-) => {
+const loadScript = (posthog: PostHog, url: string, callback: (error?: string | Event, event?: Event) => void) => {
     if (posthog.config.disable_external_dependency_loading) {
         logger.warn(`${url} was requested but loading of external scripts is disabled.`)
         return callback('Loading of external scripts is disabled')
@@ -60,3 +56,13 @@ assignableWindow.__PosthogExtensions__.loadExternalDependency = (
 
     loadScript(posthog, url, callback)
 }
+
+assignableWindow.__PosthogExtensions__.loadSiteApp = (
+    posthog: PostHog,
+    url: string,
+    callback: (error?: string | Event, event?: Event) => void
+): void => {
+    const scriptUrl = posthog.requestRouter.endpointFor('api', url)
+
+    loadScript(posthog, scriptUrl, callback)
+}
diff --git a/src/site-apps.ts b/src/site-apps.ts
@@ -1,8 +1,8 @@
-import { loadScript } from './entrypoints/external-scripts-loader'
 import { PostHog } from './posthog-core'
 import { CaptureResult, DecideResponse } from './types'
 import { assignableWindow } from './utils/globals'
 import { logger } from './utils/logger'
+import { isArray } from './utils/type-utils'
 
 export class SiteApps {
     instance: PostHog
@@ -74,13 +74,8 @@ export class SiteApps {
         return globals
     }
 
-    loadSiteApp(posthog: PostHog, url: string, callback: (error?: string | Event, event?: Event) => void) {
-        const scriptUrl = posthog.requestRouter.endpointFor('api', url)
-        loadScript(posthog, scriptUrl, callback)
-    }
-
     afterDecideResponse(response?: DecideResponse): void {
-        if (response?.['siteApps']) {
+        if (isArray(response?.siteApps) && response.siteApps.length > 0) {
             if (this.enabled && this.instance.config.opt_in_site_apps) {
                 const checkIfAllLoaded = () => {
                     // Stop collecting events once all site apps are loaded
@@ -97,7 +92,7 @@ export class SiteApps {
                         this.appsLoading.delete(id)
                         checkIfAllLoaded()
                     }
-                    this.loadSiteApp(this.instance, url, (err) => {
+                    assignableWindow.__PosthogExtensions__?.loadSiteApp?.(this.instance, url, (err) => {
                         if (err) {
                             this.appsLoading.delete(id)
                             checkIfAllLoaded()

diff --git a/src/utils/globals.ts b/src/utils/globals.ts
@@ -48,6 +48,8 @@ interface PostHogExtensions {
         callback: (error?: string | Event, event?: Event) => void
     ) => void
 
+    loadSiteApp?: (posthog: PostHog, appUrl: string, callback: (error?: string | Event, event?: Event) => void) => void
+
     parseErrorAsProperties?: (
         [event, source, lineno, colno, error]: ErrorEventArgs,
         metadata?: ErrorMetadata