From 53450ee36f9ffe90012459f89e9db91070e6349e Mon Sep 17 00:00:00 2001 From: "Tommy D. Rossi" Date: Sun, 25 Jan 2026 23:30:24 +0100 Subject: [PATCH] refactor: remove dependency injection, use direct imports - Export store, connectionManager, logger, sendMessage, getTabBySessionId from background.ts - recording.ts now imports directly instead of using initRecording() - Simplify RecordingRelay constructor to take params directly instead of interface --- extension/src/background.ts | 31 ++------- extension/src/recording.ts | 100 ++++++++++-------------------- playwriter/src/cdp-relay.ts | 6 +- playwriter/src/recording-relay.ts | 97 +++++++++-------------------- 4 files changed, 72 insertions(+), 162 deletions(-) diff --git a/extension/src/background.ts b/extension/src/background.ts index 5f22286..88a188b 100644 --- a/extension/src/background.ts +++ b/extension/src/background.ts @@ -5,7 +5,6 @@ import type { ExtensionState, ConnectionState, TabState, TabInfo } from './types import type { CDPEvent, Protocol } from 'playwriter/src/cdp-types' import type { ExtensionCommandMessage, ExtensionResponseMessage } from 'playwriter/src/protocol' import { - initRecording, getActiveRecordings, handleStartRecording, handleStopRecording, @@ -412,9 +411,9 @@ class ConnectionManager { } } -const connectionManager = new ConnectionManager() +export const connectionManager = new ConnectionManager() -const store = createStore(() => ({ +export const store = createStore(() => ({ tabs: new Map(), connectionState: 'idle', currentTabId: undefined, @@ -477,7 +476,7 @@ function sendLog(level: string, args: any[]) { }) } -const logger = { +export const logger = { log: (...args: any[]) => { console.log(...args) sendLog('log', args) @@ -518,7 +517,7 @@ self.addEventListener('unhandledrejection', (event) => { }) let messageCount = 0 -function sendMessage(message: any): void { +export function sendMessage(message: any): void { if (connectionManager.ws?.readyState === WebSocket.OPEN) { try { connectionManager.ws.send(JSON.stringify(message)) @@ -602,7 +601,7 @@ async function syncTabGroup(): Promise { } } -function getTabBySessionId(sessionId: string): { tabId: number; tab: TabInfo } | undefined { +export function getTabBySessionId(sessionId: string): { tabId: number; tab: TabInfo } | undefined { for (const [tabId, tab] of store.getState().tabs) { if (tab.sessionId === sessionId) { return { tabId, tab } @@ -620,6 +619,7 @@ function getTabByTargetId(targetId: string): { tabId: number; tab: TabInfo } | u return undefined } +<<<<<<< HEAD function emitChildDetachesForTab(tabId: number): void { const childEntries = Array.from(childSessions.entries()) .filter(([_, parentTab]) => parentTab.tabId === tabId) @@ -640,25 +640,6 @@ function emitChildDetachesForTab(tabId: number): void { }) } -// Initialize recording module with required dependencies -initRecording({ - getTabBySessionId, - getTabs: () => store.getState().tabs, - updateTabRecordingState: (tabId: number, isRecording: boolean) => { - store.setState((state) => { - const newTabs = new Map(state.tabs) - const existing = newTabs.get(tabId) - if (existing) { - newTabs.set(tabId, { ...existing, isRecording }) - } - return { tabs: newTabs } - }) - }, - sendMessage, - isWebSocketOpen: () => connectionManager.ws?.readyState === WebSocket.OPEN, - logger, -}) - async function handleCommand(msg: ExtensionCommandMessage): Promise { if (msg.method !== 'forwardCDPCommand') return diff --git a/extension/src/recording.ts b/extension/src/recording.ts index 22742d5..fb59b7b 100644 --- a/extension/src/recording.ts +++ b/extension/src/recording.ts @@ -3,7 +3,7 @@ * Uses chrome.tabCapture to record tabs via an offscreen document. */ -import type { RecordingInfo, TabInfo } from './types' +import type { RecordingInfo } from './types' import type { StartRecordingParams, StopRecordingParams, @@ -19,37 +19,14 @@ import type { OffscreenStopRecordingResult, OffscreenIsRecordingResult, } from './offscreen-types' - -// Dependencies injected from background.ts -interface RecordingDeps { - getTabBySessionId: (sessionId: string) => { tabId: number; tab: TabInfo } | undefined - getTabs: () => Map - updateTabRecordingState: (tabId: number, isRecording: boolean) => void - sendMessage: (message: unknown) => void - isWebSocketOpen: () => boolean - logger: { - debug: (...args: unknown[]) => void - error: (...args: unknown[]) => void - } -} +import { store, connectionManager, logger, sendMessage, getTabBySessionId } from './background' // Active recordings - kept outside store since MediaRecorder/MediaStream can't be serialized const activeRecordings: Map = new Map() -// Module-level dependencies (set via initRecording) -let deps: RecordingDeps | null = null - // Offscreen document management let offscreenDocumentCreating: Promise | null = null -/** - * Initialize the recording module with dependencies from background.ts. - * Must be called before using any recording functions. - */ -export function initRecording(dependencies: RecordingDeps): void { - deps = dependencies -} - /** * Get the active recordings map (for cleanup on tab disconnect). */ @@ -87,13 +64,9 @@ async function ensureOffscreenDocument(): Promise { } function resolveTabIdFromSessionId(sessionId?: string): number | undefined { - if (!deps) { - throw new Error('Recording module not initialized') - } - if (!sessionId) { // Return the first connected tab - for (const [tabId, tab] of deps.getTabs()) { + for (const [tabId, tab] of store.getState().tabs) { if (tab.state === 'connected') { return tabId } @@ -101,15 +74,22 @@ function resolveTabIdFromSessionId(sessionId?: string): number | undefined { return undefined } - const found = deps.getTabBySessionId(sessionId) + const found = getTabBySessionId(sessionId) return found?.tabId } -export async function handleStartRecording(params: StartRecordingParams): Promise { - if (!deps) { - throw new Error('Recording module not initialized') - } +function updateTabRecordingState(tabId: number, isRecording: boolean): void { + store.setState((state) => { + const newTabs = new Map(state.tabs) + const existing = newTabs.get(tabId) + if (existing) { + newTabs.set(tabId, { ...existing, isRecording }) + } + return { tabs: newTabs } + }) +} +export async function handleStartRecording(params: StartRecordingParams): Promise { const tabId = resolveTabIdFromSessionId(params.sessionId) if (!tabId) { return { success: false, error: 'No connected tab found for recording. Click the Playwriter extension icon on the tab you want to record.' } @@ -119,12 +99,12 @@ export async function handleStartRecording(params: StartRecordingParams): Promis return { success: false, error: 'Recording already in progress for this tab' } } - const tabInfo = deps.getTabs().get(tabId) + const tabInfo = store.getState().tabs.get(tabId) if (!tabInfo || tabInfo.state !== 'connected') { return { success: false, error: 'Tab is not connected' } } - deps.logger.debug('Starting recording for tab:', tabId, 'params:', params) + logger.debug('Starting recording for tab:', tabId, 'params:', params) try { // Ensure offscreen document exists @@ -150,7 +130,7 @@ export async function handleStartRecording(params: StartRecordingParams): Promis }) }) - deps.logger.debug('Got stream ID for tab:', tabId, 'streamId:', streamId.substring(0, 20) + '...') + logger.debug('Got stream ID for tab:', tabId, 'streamId:', streamId.substring(0, 20) + '...') // Send message to offscreen document to start recording const result = await chrome.runtime.sendMessage({ @@ -173,22 +153,18 @@ export async function handleStartRecording(params: StartRecordingParams): Promis activeRecordings.set(tabId, { tabId, startedAt }) // Update tab state - deps.updateTabRecordingState(tabId, true) + updateTabRecordingState(tabId, true) - deps.logger.debug('Recording started for tab:', tabId, 'mimeType:', result.mimeType) + logger.debug('Recording started for tab:', tabId, 'mimeType:', result.mimeType) return { success: true, tabId, startedAt } } catch (error: unknown) { const errorMessage = error instanceof Error ? error.message : String(error) - deps.logger.error('Failed to start recording:', error) + logger.error('Failed to start recording:', error) return { success: false, error: errorMessage } } } export async function handleStopRecording(params: StopRecordingParams): Promise { - if (!deps) { - throw new Error('Recording module not initialized') - } - const tabId = resolveTabIdFromSessionId(params.sessionId) if (!tabId) { return { success: false, error: 'No connected tab found' } @@ -199,7 +175,7 @@ export async function handleStopRecording(params: StopRecordingParams): Promise< return { success: false, error: 'No active recording for this tab' } } - deps.logger.debug('Stopping recording for tab:', tabId) + logger.debug('Stopping recording for tab:', tabId) try { // Send message to offscreen document to stop recording - include tabId for concurrent support @@ -216,22 +192,18 @@ export async function handleStopRecording(params: StopRecordingParams): Promise< // Clean up activeRecordings.delete(tabId) - deps.updateTabRecordingState(tabId, false) + updateTabRecordingState(tabId, false) - deps.logger.debug('Recording stopped for tab:', tabId, 'duration:', duration) + logger.debug('Recording stopped for tab:', tabId, 'duration:', duration) return { success: true, tabId, duration } } catch (error: unknown) { const errorMessage = error instanceof Error ? error.message : String(error) - deps.logger.error('Failed to stop recording:', error) + logger.error('Failed to stop recording:', error) return { success: false, error: errorMessage } } } export async function handleIsRecording(params: IsRecordingParams): Promise { - if (!deps) { - throw new Error('Recording module not initialized') - } - const tabId = resolveTabIdFromSessionId(params.sessionId) if (!tabId) { return { isRecording: false } @@ -261,10 +233,6 @@ export async function handleIsRecording(params: IsRecordingParams): Promise { - if (!deps) { - throw new Error('Recording module not initialized') - } - const tabId = resolveTabIdFromSessionId(params.sessionId) if (!tabId) { return { success: false, error: 'No connected tab found' } @@ -275,7 +243,7 @@ export async function handleCancelRecording(params: CancelRecordingParams): Prom return { success: true } // Already not recording } - deps.logger.debug('Cancelling recording for tab:', tabId) + logger.debug('Cancelling recording for tab:', tabId) try { // Send message to offscreen document to cancel recording - include tabId for concurrent support @@ -285,11 +253,11 @@ export async function handleCancelRecording(params: CancelRecordingParams): Prom }) activeRecordings.delete(tabId) - deps.updateTabRecordingState(tabId, false) + updateTabRecordingState(tabId, false) // Send cancel marker - if (deps.isWebSocketOpen()) { - deps.sendMessage({ + if (connectionManager.ws?.readyState === WebSocket.OPEN) { + sendMessage({ method: 'recordingCancelled', params: { tabId }, }) @@ -298,7 +266,7 @@ export async function handleCancelRecording(params: CancelRecordingParams): Prom return { success: true } } catch (error: unknown) { const errorMessage = error instanceof Error ? error.message : String(error) - deps.logger.error('Failed to cancel recording:', error) + logger.error('Failed to cancel recording:', error) return { success: false, error: errorMessage } } } @@ -307,18 +275,14 @@ export async function handleCancelRecording(params: CancelRecordingParams): Prom * Clean up recordings when tab is disconnected. */ export async function cleanupRecordingForTab(tabId: number): Promise { - if (!deps) { - return - } - const recording = activeRecordings.get(tabId) if (recording) { - deps.logger.debug('Cleaning up recording for disconnected tab:', tabId) + logger.debug('Cleaning up recording for disconnected tab:', tabId) try { // Tell offscreen document to cancel recording - include tabId for concurrent support await chrome.runtime.sendMessage({ action: 'cancelRecording', tabId }) } catch (e) { - deps.logger.debug('Error cleaning up recording:', e) + logger.debug('Error cleaning up recording:', e) } activeRecordings.delete(tabId) } diff --git a/playwriter/src/cdp-relay.ts b/playwriter/src/cdp-relay.ts index 3ee5296..92204ff 100644 --- a/playwriter/src/cdp-relay.ts +++ b/playwriter/src/cdp-relay.ts @@ -264,11 +264,11 @@ export async function startPlayWriterCDPRelayServer({ } // Recording relay for screen recording functionality - const recordingRelay = new RecordingRelay({ + const recordingRelay = new RecordingRelay( sendToExtension, - isExtensionConnected: () => extensionWs !== null, + () => extensionWs !== null, logger, - }) + ) // Auto-create initial tab when PLAYWRITER_AUTO_ENABLE is set and no targets exist. // This allows Playwright to connect and immediately have a page to work with. diff --git a/playwriter/src/recording-relay.ts b/playwriter/src/recording-relay.ts index 722ec73..1f5c97b 100644 --- a/playwriter/src/recording-relay.ts +++ b/playwriter/src/recording-relay.ts @@ -29,30 +29,28 @@ export interface ActiveRecording { resolveStop?: (result: StopRecordingResult) => void } -interface RecordingRelayDeps { - sendToExtension: (params: { method: string; params?: unknown; timeout?: number }) => Promise - isExtensionConnected: () => boolean - logger?: { - log(...args: unknown[]): void - error(...args: unknown[]): void - } -} - export class RecordingRelay { private activeRecordings = new Map() // Track which tabId just sent recordingData metadata - used to route the next binary chunk private lastRecordingMetadataTabId: number | null = null - private deps: RecordingRelayDeps + private sendToExtension: (params: { method: string; params?: unknown; timeout?: number }) => Promise + private isExtensionConnected: () => boolean + private logger?: { log(...args: unknown[]): void; error(...args: unknown[]): void } - constructor(deps: RecordingRelayDeps) { - this.deps = deps + constructor( + sendToExtension: (params: { method: string; params?: unknown; timeout?: number }) => Promise, + isExtensionConnected: () => boolean, + logger?: { log(...args: unknown[]): void; error(...args: unknown[]): void } + ) { + this.sendToExtension = sendToExtension + this.isExtensionConnected = isExtensionConnected + this.logger = logger } /** * Handle incoming binary data (recording chunks) from the extension. - * Returns true if the data was handled as a recording chunk. */ - handleBinaryData(buffer: Buffer): boolean { + handleBinaryData(buffer: Buffer): void { const tabId = this.lastRecordingMetadataTabId this.lastRecordingMetadataTabId = null @@ -60,41 +58,35 @@ export class RecordingRelay { const recording = this.activeRecordings.get(tabId) if (recording) { recording.chunks.push(buffer) - this.deps.logger?.log(pc.blue(`Received recording chunk for tab ${tabId}: ${buffer.length} bytes (total chunks: ${recording.chunks.length})`)) - return true + this.logger?.log(pc.blue(`Received recording chunk for tab ${tabId}: ${buffer.length} bytes (total chunks: ${recording.chunks.length})`)) } else { - this.deps.logger?.log(pc.yellow(`Received recording chunk for unknown tab ${tabId}, ignoring`)) + this.logger?.log(pc.yellow(`Received recording chunk for unknown tab ${tabId}, ignoring`)) } } else { - this.deps.logger?.log(pc.yellow('Received recording chunk without preceding metadata, ignoring')) + this.logger?.log(pc.yellow('Received recording chunk without preceding metadata, ignoring')) } - return false } /** * Handle recordingData message from extension. - * This is sent before binary chunks to identify which recording they belong to. */ handleRecordingData(message: RecordingDataMessage): void { const { tabId, final } = message.params const recording = this.activeRecordings.get(tabId) - // Track which tab sent this metadata for routing the next binary chunk if (!final) { this.lastRecordingMetadataTabId = tabId } if (recording && final) { - // This is the final marker - write all chunks to file try { const totalSize = recording.chunks.reduce((sum, chunk) => sum + chunk.length, 0) const combined = Buffer.concat(recording.chunks) fs.writeFileSync(recording.outputPath, combined) const duration = Date.now() - recording.startedAt - this.deps.logger?.log(pc.green(`Recording saved: ${recording.outputPath} (${totalSize} bytes, ${duration}ms)`)) + this.logger?.log(pc.green(`Recording saved: ${recording.outputPath} (${totalSize} bytes, ${duration}ms)`)) - // Resolve the stop promise if (recording.resolveStop) { recording.resolveStop({ success: true, @@ -106,7 +98,7 @@ export class RecordingRelay { } } catch (error: unknown) { const errorMessage = error instanceof Error ? error.message : String(error) - this.deps.logger?.error('Failed to write recording:', error) + this.logger?.error('Failed to write recording:', error) if (recording.resolveStop) { recording.resolveStop({ success: false, error: errorMessage }) } @@ -114,7 +106,6 @@ export class RecordingRelay { this.activeRecordings.delete(tabId) } - // Non-final recordingData is just a marker that binary follows - handled in handleBinaryData } /** @@ -124,7 +115,7 @@ export class RecordingRelay { const { tabId } = message.params const recording = this.activeRecordings.get(tabId) if (recording) { - this.deps.logger?.log(pc.yellow(`Recording cancelled for tab ${tabId}`)) + this.logger?.log(pc.yellow(`Recording cancelled for tab ${tabId}`)) if (recording.resolveStop) { recording.resolveStop({ success: false, error: 'Recording was cancelled' }) } @@ -132,9 +123,6 @@ export class RecordingRelay { } } - /** - * Start recording a tab. - */ async startRecording(params: StartRecordingParams & { outputPath: string }): Promise { const { outputPath, ...recordingParams } = params @@ -142,18 +130,17 @@ export class RecordingRelay { return { success: false, error: 'outputPath is required' } } - if (!this.deps.isExtensionConnected()) { + if (!this.isExtensionConnected()) { return { success: false, error: 'Extension not connected' } } - // Ensure output directory exists const dir = path.dirname(outputPath) if (!fs.existsSync(dir)) { fs.mkdirSync(dir, { recursive: true }) } try { - const result = await this.deps.sendToExtension({ + const result = await this.sendToExtension({ method: 'startRecording', params: recordingParams, timeout: 10000, @@ -164,7 +151,6 @@ export class RecordingRelay { } if (result.success) { - // Track this recording - store sessionId for lookup when stopping this.activeRecordings.set(result.tabId, { tabId: result.tabId, sessionId: recordingParams.sessionId, @@ -172,27 +158,22 @@ export class RecordingRelay { chunks: [], startedAt: result.startedAt, }) - this.deps.logger?.log(pc.green(`Recording started for tab ${result.tabId} (sessionId: ${recordingParams.sessionId || 'none'}), output: ${outputPath}`)) + this.logger?.log(pc.green(`Recording started for tab ${result.tabId} (sessionId: ${recordingParams.sessionId || 'none'}), output: ${outputPath}`)) } return result } catch (error: unknown) { const errorMessage = error instanceof Error ? error.message : String(error) - this.deps.logger?.error('Start recording error:', error) + this.logger?.error('Start recording error:', error) return { success: false, error: errorMessage } } } - /** - * Stop recording and save to file. - */ async stopRecording(params: StopRecordingParams): Promise { - if (!this.deps.isExtensionConnected()) { + if (!this.isExtensionConnected()) { return { success: false, error: 'Extension not connected' } } - // Find the active recording by sessionId for concurrent recording support - // If no sessionId provided, fall back to the first recording (backward compatibility) const findRecording = (): ActiveRecording | undefined => { if (params.sessionId) { for (const recording of this.activeRecordings.values()) { @@ -200,10 +181,8 @@ export class RecordingRelay { return recording } } - // SessionId provided but no matching recording found return undefined } - // No sessionId - return first recording (backward compat for single-recording case) return this.activeRecordings.values().next().value } @@ -216,7 +195,6 @@ export class RecordingRelay { return { success: false, error: errorMsg } } - // Set up promise to wait for final chunk let timeoutId: ReturnType const finalPromise = new Promise((resolve) => { const wrappedResolve = (result: StopRecordingResult) => { @@ -224,7 +202,6 @@ export class RecordingRelay { resolve(result) } recording.resolveStop = wrappedResolve - // Timeout after 30 seconds timeoutId = setTimeout(() => { if (recording.resolveStop) { recording.resolveStop = undefined @@ -234,8 +211,7 @@ export class RecordingRelay { }) try { - // Tell extension to stop recording - const result = await this.deps.sendToExtension({ + const result = await this.sendToExtension({ method: 'stopRecording', params, timeout: 10000, @@ -247,55 +223,44 @@ export class RecordingRelay { return result } - // Wait for final chunk to arrive return await finalPromise } catch (error: unknown) { const errorMessage = error instanceof Error ? error.message : String(error) - this.deps.logger?.error('Stop recording error:', error) + this.logger?.error('Stop recording error:', error) return { success: false, error: errorMessage } } } - /** - * Check if recording is active. - */ async isRecording(params: IsRecordingParams): Promise { - if (!this.deps.isExtensionConnected()) { + if (!this.isExtensionConnected()) { return { isRecording: false } } try { - return await this.deps.sendToExtension({ + return await this.sendToExtension({ method: 'isRecording', params, timeout: 5000, }) as IsRecordingResult - } catch (error: unknown) { - const errorMessage = error instanceof Error ? error.message : String(error) - this.deps.logger?.error('Recording status error:', error) + } catch { return { isRecording: false } } } - /** - * Cancel recording without saving. - */ async cancelRecording(params: CancelRecordingParams): Promise { - if (!this.deps.isExtensionConnected()) { + if (!this.isExtensionConnected()) { return { success: false, error: 'Extension not connected' } } try { - // Note: Recording cleanup is handled by the 'recordingCancelled' event handler - // which is triggered by the extension after cancellation. - return await this.deps.sendToExtension({ + return await this.sendToExtension({ method: 'cancelRecording', params, timeout: 5000, }) as CancelRecordingResult } catch (error: unknown) { const errorMessage = error instanceof Error ? error.message : String(error) - this.deps.logger?.error('Cancel recording error:', error) + this.logger?.error('Cancel recording error:', error) return { success: false, error: errorMessage } } }