🐛 pr-feedback: replace implementation code with a function

Author: mormubis

test/e2e/lib/framework/createTest.ts

@@ -58,7 +58,7 @@ class TestBuilder {
     logsConfiguration?: LogsInitConfiguration
   } = {}
   private hostName?: string
-  private workerImplementationFactory?: (setup: string) => string
+  private workerImplementationFactory?: WorkerImplementationFactory
 
   constructor(private title: string) {}
 
@@ -130,18 +130,16 @@ class TestBuilder {
     return this
   }
 
-  withWorker(implementation: WorkerImplementationFactory, isModule = false) {
-    implementation.isModule = isModule
+  withWorker(implementation: WorkerImplementationFactory, options: RegistrationOptions = {}) {
+    implementation.isModule = options.type === 'module'
     this.workerImplementationFactory = implementation
 
-    const options = isModule ? '{ type: "module" }' : '{}'
-
     // Service workers require HTTPS or localhost due to browser security restrictions
     this.withHostName('localhost')
     this.withBody(html`
       <script>
         if (!window.myServiceWorker && 'serviceWorker' in navigator) {
-          navigator.serviceWorker.register('/sw.js', ${options}).then((registration) => {
+          navigator.serviceWorker.register('/sw.js', ${JSON.stringify(options)}).then((registration) => {
             window.myServiceWorker = registration
           })
         }

test/e2e/lib/framework/pageSetups.ts

@@ -1,13 +1,15 @@
+import { stripTypeScriptTypes } from 'node:module'
 import { generateUUID, INTAKE_URL_PARAMETERS } from '@datadog/browser-core'
-import type { LogsInitConfiguration } from '@datadog/browser-logs'
+import type { LogsInitConfiguration, DatadogLogs } from '@datadog/browser-logs'
 import type { RumInitConfiguration, RemoteConfiguration } from '@datadog/browser-rum-core'
+import type { DatadogRum } from '@datadog/browser-rum'
 import type test from '@playwright/test'
 import { DEFAULT_LOGS_CONFIGURATION } from '../helpers/configuration'
 import { isBrowserStack, isContinuousIntegration } from './environment'
 import type { Servers } from './httpServers'
 
 export interface WorkerImplementationFactory {
-  (setup: string): string
+  (self: WorkerGlobalScope & { DD_LOGS: DatadogLogs; DD_RUM: DatadogRum }): void
   isModule?: boolean
 }
 export interface SetupOptions {
@@ -230,7 +232,9 @@ export function workerSetup(options: SetupOptions, servers: Servers) {
     `
   }
 
-  return options.workerImplementation(script)
+  script += `;(${options.workerImplementation.toString()})(self);`
+
+  return script
 }
 
 export function basePage({ header, body }: { header?: string; body?: string }) {

test/e2e/scenario/logs.scenario.ts

@@ -1,6 +1,6 @@
 import { DEFAULT_REQUEST_ERROR_RESPONSE_LENGTH_LIMIT } from '@datadog/browser-logs/cjs/domain/configuration'
 import { test, expect } from '@playwright/test'
-import { createTest, js } from '../lib/framework'
+import { createTest } from '../lib/framework'
 import { APPLICATION_ID } from '../lib/helpers/configuration'
 
 const UNREACHABLE_URL = 'http://localhost:9999/unreachable'
@@ -15,18 +15,14 @@ test.describe('logs', () => {
   createTest('service worker with worker logs - esm')
     .withLogs()
     .withWorker(
-      (setup) => js`
-        // Initialize DD_LOGS in service worker
-        ${setup}
-
-        // Handle messages from main thread
+      function (self) {
         self.addEventListener('message', (event) => {
-          const message = event.data;
+          const message = (event as MessageEvent<string>).data
 
-          DD_LOGS.logger.log(message);
-        });
-      `,
-      true
+          self.DD_LOGS.logger.log(message)
+        })
+      },
+      { type: 'module' }
     )
     .run(async ({ flushEvents, intakeRegistry, browserName, interactWithWorker }) => {
       test.skip(browserName !== 'chromium', 'Non-Chromium browsers do not support ES modules in Service Workers')
@@ -43,19 +39,13 @@ test.describe('logs', () => {
 
   createTest('service worker with worker logs - importScripts')
     .withLogs()
-    .withWorker(
-      (setup) => js`
-        // Initialize DD_LOGS in service worker
-        ${setup}
-
-        // Handle messages from main thread
-        self.addEventListener('message', (event) => {
-          const message = event.data;
+    .withWorker(function (self) {
+      self.addEventListener('message', (event) => {
+        const message = (event as MessageEvent<string>).data
 
-          DD_LOGS.logger.log(message);
-        });
-      `
-    )
+        self.DD_LOGS.logger.log(message)
+      })
+    })
     .run(async ({ flushEvents, intakeRegistry, browserName, interactWithWorker }) => {
       test.skip(
         browserName === 'webkit',
@@ -74,20 +64,23 @@ test.describe('logs', () => {
 
   createTest('service worker console forwarding')
     .withLogs({ forwardConsoleLogs: 'all', forwardErrorsToLogs: true })
-    .withWorker(
-      (setup) => js`
-        // Initialize DD_LOGS in service worker
-        ${setup}
-  
-        console.log('SW console log test');
-      `
-    )
-    .run(async ({ flushEvents, intakeRegistry, browserName }) => {
+    .withWorker(function (self) {
+      self.addEventListener('message', (event) => {
+        const message = (event as MessageEvent<string>).data
+
+        console.log(message)
+      })
+    })
+    .run(async ({ flushEvents, intakeRegistry, interactWithWorker, browserName }) => {
       test.skip(
         browserName === 'webkit',
         'BrowserStack overrides the localhost URL with bs-local.com and cannot be used to install a Service Worker'
       )
 
+      await interactWithWorker((worker) => {
+        worker.postMessage('SW console log test')
+      })
+
       await flushEvents()
 
       // Expect logs for console, error, and report events from service worker

test/e2e/tsconfig.json

@@ -10,6 +10,7 @@
     "target": "ES2022",
     "module": "ES2020",
     "types": ["node", "ajv"],
+    "lib": ["WebWorker"],
     "allowJs": true,
     "noEmit": true
   }