From 5e733d592d1c145b506de42ec554b603b6889f2b Mon Sep 17 00:00:00 2001 From: Fredrik Rombach Ekelund Date: Mon, 3 Feb 2025 11:34:31 +0100 Subject: [PATCH 1/6] WIP: Type safety for IPC events --- src/components/auth-provider.tsx | 9 ++- .../use-listen-deep-link-connection.ts | 42 +++++------ src/hooks/use-fullscreen.ts | 2 +- src/hooks/use-import-export.tsx | 4 +- src/hooks/use-ipc-listener.ts | 7 +- src/hooks/use-theme-details.tsx | 6 +- src/ipc-types.d.ts | 1 - src/preload.ts | 70 +++++++++++-------- 8 files changed, 77 insertions(+), 64 deletions(-) diff --git a/src/components/auth-provider.tsx b/src/components/auth-provider.tsx index 74da660ab..93f82d953 100644 --- a/src/components/auth-provider.tsx +++ b/src/components/auth-provider.tsx @@ -39,11 +39,14 @@ const AuthProvider: React.FC< AuthProviderProps > = ( { children } ) => { const authenticate = useCallback( () => getIpcApi().authenticate(), [] ); - useIpcListener( 'auth-updated', ( _event, { token, error } ) => { - if ( error ) { - Sentry.captureException( error ); + useIpcListener( 'auth-updated', ( _event, payload ) => { + if ( 'error' in payload ) { + Sentry.captureException( payload.error ); return; } + + const { token } = payload; + setIsAuthenticated( true ); setClient( createWpcomClient( token.accessToken, locale ) ); if ( token.id || token.email || token.displayName ) { diff --git a/src/hooks/sync-sites/use-listen-deep-link-connection.ts b/src/hooks/sync-sites/use-listen-deep-link-connection.ts index 4b1ab7e8b..a31636eb7 100644 --- a/src/hooks/sync-sites/use-listen-deep-link-connection.ts +++ b/src/hooks/sync-sites/use-listen-deep-link-connection.ts @@ -14,30 +14,24 @@ export function useListenDeepLinkConnection( { const { selectedSite, setSelectedSiteId } = useSiteDetails(); const { setSelectedTab, selectedTab } = useContentTabs(); - useIpcListener( - 'sync-connect-site', - async ( - _event, - { remoteSiteId, studioSiteId }: { remoteSiteId: number; studioSiteId: string } - ) => { - // Fetch latest sites from network before checking - const latestSites = await refetchSites(); - const newConnectedSiteResponse = latestSites.find( ( site ) => site.ID === remoteSiteId ); - if ( newConnectedSiteResponse ) { - if ( selectedSite?.id && selectedSite.id !== studioSiteId ) { - // Select studio site that started the sync - setSelectedSiteId( studioSiteId ); - } - const newConnectedSite = transformSingleSiteResponse( - newConnectedSiteResponse, - 'already-connected' - ); - await connectSite( newConnectedSite, studioSiteId ); - if ( selectedTab !== 'sync' ) { - // Switch to sync tab - setSelectedTab( 'sync' ); - } + useIpcListener( 'sync-connect-site', async ( _event, { remoteSiteId, studioSiteId } ) => { + // Fetch latest sites from network before checking + const latestSites = await refetchSites(); + const newConnectedSiteResponse = latestSites.find( ( site ) => site.ID === remoteSiteId ); + if ( newConnectedSiteResponse ) { + if ( selectedSite?.id && selectedSite.id !== studioSiteId ) { + // Select studio site that started the sync + setSelectedSiteId( studioSiteId ); + } + const newConnectedSite = transformSingleSiteResponse( + newConnectedSiteResponse, + 'already-connected' + ); + await connectSite( newConnectedSite, studioSiteId ); + if ( selectedTab !== 'sync' ) { + // Switch to sync tab + setSelectedTab( 'sync' ); } } - ); + } ); } diff --git a/src/hooks/use-fullscreen.ts b/src/hooks/use-fullscreen.ts index 5fca94cc4..98910d285 100644 --- a/src/hooks/use-fullscreen.ts +++ b/src/hooks/use-fullscreen.ts @@ -19,7 +19,7 @@ export function useFullscreen() { }; }, [] ); - useIpcListener( 'window-fullscreen-change', ( _, fullscreen: boolean ) => { + useIpcListener( 'window-fullscreen-change', ( _, fullscreen ) => { setIsFullscreen( fullscreen ); } ); diff --git a/src/hooks/use-import-export.tsx b/src/hooks/use-import-export.tsx index 5885c6510..35fc5ced0 100644 --- a/src/hooks/use-import-export.tsx +++ b/src/hooks/use-import-export.tsx @@ -145,7 +145,7 @@ export const ImportExportProvider = ( { children }: { children: React.ReactNode [ importState ] ); - useIpcListener( 'on-import', ( _, { event, data }: ImportExportEventData, siteId: string ) => { + useIpcListener( 'on-import', ( _, { event, data }, siteId ) => { if ( ! siteId ) { return; } @@ -349,7 +349,7 @@ export const ImportExportProvider = ( { children }: { children: React.ReactNode [ exportSite ] ); - useIpcListener( 'on-export', ( _, { event, data }: ImportExportEventData, siteId: string ) => { + useIpcListener( 'on-export', ( _, { event, data }, siteId ) => { if ( ! siteId ) { return; } diff --git a/src/hooks/use-ipc-listener.ts b/src/hooks/use-ipc-listener.ts index df66eacbd..5ec617252 100644 --- a/src/hooks/use-ipc-listener.ts +++ b/src/hooks/use-ipc-listener.ts @@ -1,6 +1,11 @@ +import { IpcRendererEvent } from 'electron'; import { useEffect } from 'react'; +import { IpcEvents } from 'src/preload'; -export function useIpcListener( channel: string, listener: ( ...args: any[] ) => void ) { +export function useIpcListener< T extends keyof IpcEvents >( + channel: T, + listener: ( event: IpcRendererEvent, ...args: IpcEvents[ T ] ) => void +) { useEffect( () => { return window.ipcListener.subscribe( channel, listener ); }, [ channel, listener ] ); diff --git a/src/hooks/use-theme-details.tsx b/src/hooks/use-theme-details.tsx index abad34609..84cfd23cf 100644 --- a/src/hooks/use-theme-details.tsx +++ b/src/hooks/use-theme-details.tsx @@ -45,7 +45,7 @@ export const ThemeDetailsProvider: React.FC< ThemeDetailsProviderProps > = ( { c ); const [ loadingThumbnails, setLoadingThumbnails ] = useState< Record< string, boolean > >( {} ); - useIpcListener( 'theme-details-changed', ( _evt, id, details ) => { + useIpcListener( 'theme-details-changed', ( _evt, { id, details } ) => { setThemeDetails( ( themeDetails ) => { return { ...themeDetails, [ id ]: details }; } ); @@ -54,7 +54,7 @@ export const ThemeDetailsProvider: React.FC< ThemeDetailsProviderProps > = ( { c } ); } ); - useIpcListener( 'thumbnail-changed', ( _evt, id, imageData ) => { + useIpcListener( 'thumbnail-changed', ( _evt, { id, imageData } ) => { setThumbnails( ( thumbnails ) => { return { ...thumbnails, [ id ]: imageData }; } ); @@ -63,7 +63,7 @@ export const ThemeDetailsProvider: React.FC< ThemeDetailsProviderProps > = ( { c } ); } ); - useIpcListener( 'theme-details-updating', ( _evt, id ) => { + useIpcListener( 'theme-details-updating', ( _evt, { id } ) => { setLoadingThemeDetails( ( loadingThemeDetails ) => { return { ...loadingThemeDetails, [ id ]: true }; } ); diff --git a/src/ipc-types.d.ts b/src/ipc-types.d.ts index 325f113ef..2a4e08449 100644 --- a/src/ipc-types.d.ts +++ b/src/ipc-types.d.ts @@ -90,7 +90,6 @@ interface IpcListener { // Our IPC objects will be attached to the `window` global interface Window { - ipcListener: IpcListener; ipcApi: IpcApi; appGlobals: AppGlobals; } diff --git a/src/preload.ts b/src/preload.ts index 6eb7bfdd1..205d14d33 100644 --- a/src/preload.ts +++ b/src/preload.ts @@ -2,10 +2,18 @@ // https://www.electronjs.org/docs/latest/tutorial/process-model#preload-scripts import '@sentry/electron/preload'; -import { SaveDialogOptions, contextBridge, ipcRenderer, webUtils } from 'electron'; +import { + IpcRendererEvent, + SaveDialogOptions, + contextBridge, + ipcRenderer, + webUtils, +} from 'electron'; import { LocaleData } from '@wordpress/i18n'; import { ExportOptions } from './lib/import-export/export/types'; +import { ImportExportEventData } from './lib/import-export/handle-events'; import { BackupArchiveInfo } from './lib/import-export/import/types'; +import { StoredToken } from './lib/oauth'; import { promptWindowsSpeedUpSites } from './lib/windows-helpers'; import type { SyncSite } from './hooks/use-fetch-wpcom-sites/types'; import type { LogLevel } from './logging'; @@ -112,33 +120,37 @@ const api: IpcApi = { }; contextBridge.exposeInMainWorld( 'ipcApi', api ); +export interface IpcEvents { + 'add-site': [ void ]; + 'auth-updated': [ { token: StoredToken } | { error: Error } ]; + 'on-export': [ ImportExportEventData, string ]; + 'on-import': [ ImportExportEventData, string ]; + 'sync-connect-site': [ { remoteSiteId: number; studioSiteId: string } ]; + 'test-render-failure': [ void ]; + 'theme-details-changed': [ { id: string; details: StartedSiteDetails[ 'themeDetails' ] } ]; + 'theme-details-updating': [ { id: string } ]; + 'thumbnail-changed': [ { id: string; imageData: string | null } ]; + 'user-settings': [ void ]; + 'window-fullscreen-change': [ boolean ]; +} -const allowedChannels = [ - 'test-render-failure', - 'add-site', - 'user-settings', - 'auth-updated', - 'sync-connect-site', - 'thumbnail-changed', - 'theme-details-changed', - 'theme-details-updating', - 'on-import', - 'on-export', - 'window-fullscreen-change', -] as const; +const subscribe = < T extends keyof IpcEvents >( + channel: T, + listener: ( event: IpcRendererEvent, ...args: IpcEvents[ T ] ) => void +) => { + ipcRenderer.on( channel, listener ); -contextBridge.exposeInMainWorld( 'ipcListener', { - subscribe: ( - channel: ( typeof allowedChannels )[ number ], - listener: ( ...args: any[] ) => void - ) => { - if ( allowedChannels.includes( channel ) ) { - ipcRenderer.on( channel, listener ); - return () => { - ipcRenderer.off( channel, listener ); - }; - } else { - throw new Error( `Attempted to listen on disallowed IPC channel: ${ channel }` ); - } - }, -} ); + return () => { + ipcRenderer.off( channel, listener ); + }; +}; + +declare global { + interface Window { + ipcListener: { + subscribe: typeof subscribe; + }; + } +} + +contextBridge.exposeInMainWorld( 'ipcListener', { subscribe } ); From add5ed353ae837f7f48700079d650e62a6adf14c Mon Sep 17 00:00:00 2001 From: Fredrik Rombach Ekelund Date: Mon, 3 Feb 2025 12:50:03 +0100 Subject: [PATCH 2/6] Type safety for sending IPC events --- src/hooks/use-import-export.tsx | 1 - src/hooks/use-ipc-listener.ts | 2 +- src/hooks/use-theme-details.tsx | 2 +- src/index.ts | 4 +-- src/ipc-handlers.ts | 47 +++++++++++++-------------------- src/ipc-types.d.ts | 4 --- src/ipc-utils.ts | 38 ++++++++++++++++++++++++++ src/lib/oauth.ts | 7 +++-- src/main-window.ts | 5 ++-- src/menu.ts | 10 +++---- src/preload.ts | 24 +++++------------ 11 files changed, 78 insertions(+), 66 deletions(-) create mode 100644 src/ipc-utils.ts diff --git a/src/hooks/use-import-export.tsx b/src/hooks/use-import-export.tsx index 35fc5ced0..255630d3f 100644 --- a/src/hooks/use-import-export.tsx +++ b/src/hooks/use-import-export.tsx @@ -5,7 +5,6 @@ import { getIpcApi } from '../lib/get-ipc-api'; import { ExportEvents } from '../lib/import-export/export/events'; import { generateBackupFilename } from '../lib/import-export/export/generate-backup-filename'; import { BackupCreateProgressEventData, ExportOptions } from '../lib/import-export/export/types'; -import { ImportExportEventData } from '../lib/import-export/handle-events'; import { ImporterEvents, BackupExtractEvents, diff --git a/src/hooks/use-ipc-listener.ts b/src/hooks/use-ipc-listener.ts index 5ec617252..72fdb68c9 100644 --- a/src/hooks/use-ipc-listener.ts +++ b/src/hooks/use-ipc-listener.ts @@ -1,6 +1,6 @@ import { IpcRendererEvent } from 'electron'; import { useEffect } from 'react'; -import { IpcEvents } from 'src/preload'; +import { IpcEvents } from 'src/ipc-utils'; export function useIpcListener< T extends keyof IpcEvents >( channel: T, diff --git a/src/hooks/use-theme-details.tsx b/src/hooks/use-theme-details.tsx index 84cfd23cf..0fe0337a5 100644 --- a/src/hooks/use-theme-details.tsx +++ b/src/hooks/use-theme-details.tsx @@ -56,7 +56,7 @@ export const ThemeDetailsProvider: React.FC< ThemeDetailsProviderProps > = ( { c useIpcListener( 'thumbnail-changed', ( _evt, { id, imageData } ) => { setThumbnails( ( thumbnails ) => { - return { ...thumbnails, [ id ]: imageData }; + return { ...thumbnails, [ id ]: imageData ?? undefined }; } ); setLoadingThumbnails( ( loadingThumbnails ) => { return { ...loadingThumbnails, [ id ]: false }; diff --git a/src/index.ts b/src/index.ts index b481a2f2c..32f870f73 100644 --- a/src/index.ts +++ b/src/index.ts @@ -14,6 +14,7 @@ import { __ } from '@wordpress/i18n'; import packageJson from '../package.json'; import { PROTOCOL_PREFIX } from './constants'; import * as ipcHandlers from './ipc-handlers'; +import { sendIpcEventToRenderer } from './ipc-utils'; import { hasActiveSyncOperations } from './lib/active-sync-operations'; import { bumpAggregatedUniqueStat, bumpStat } from './lib/bump-stats'; import { @@ -89,8 +90,7 @@ const onOpenUrlCallback = async ( url: string ) => { const remoteSiteId = parseInt( searchParams.get( 'remoteSiteId' ) ?? '' ); const studioSiteId = searchParams.get( 'studioSiteId' ); if ( remoteSiteId && studioSiteId ) { - const mainWindow = await getMainWindow(); - mainWindow.webContents.send( 'sync-connect-site', { remoteSiteId, studioSiteId } ); + sendIpcEventToRenderer( 'sync-connect-site', { remoteSiteId, studioSiteId } ); } } }; diff --git a/src/ipc-handlers.ts b/src/ipc-handlers.ts index 30185613a..193c32961 100644 --- a/src/ipc-handlers.ts +++ b/src/ipc-handlers.ts @@ -19,6 +19,7 @@ import { __, LocaleData, defaultI18n } from '@wordpress/i18n'; import archiver from 'archiver'; import { DEFAULT_PHP_VERSION } from '../vendor/wp-now/src/constants'; import { MAIN_MIN_WIDTH, SIDEBAR_WIDTH } from './constants'; +import { sendIpcEventToRendererWithWindow } from './ipc-utils'; import { ACTIVE_SYNC_OPERATIONS } from './lib/active-sync-operations'; import { calculateDirectorySize } from './lib/calculate-directory-size'; import { download } from './lib/download'; @@ -61,9 +62,10 @@ async function sendThumbnailChangedEvent( event: IpcMainInvokeEvent, id: string } const thumbnailData = await getThumbnailData( event, id ); const parentWindow = BrowserWindow.fromWebContents( event.sender ); - if ( parentWindow && ! parentWindow.isDestroyed() ) { - parentWindow.webContents.send( 'thumbnail-changed', id, thumbnailData ); - } + sendIpcEventToRendererWithWindow( parentWindow, 'thumbnail-changed', { + id, + imageData: thumbnailData, + } ); } async function mergeSiteDetailsWithRunningDetails( @@ -111,9 +113,7 @@ export async function importSite( try { const onEvent = ( data: ImportExportEventData ) => { const parentWindow = BrowserWindow.fromWebContents( event.sender ); - if ( parentWindow && ! parentWindow.isDestroyed() && ! event.sender.isDestroyed() ) { - parentWindow.webContents.send( 'on-import', data, id ); - } + sendIpcEventToRendererWithWindow( parentWindow, 'on-import', data, id ); }; const result = await importBackup( backupFile, site.details, onEvent, defaultImporterOptions ); if ( ! result ) { @@ -191,9 +191,7 @@ export async function createSite( } const parentWindow = BrowserWindow.fromWebContents( event.sender ); - if ( parentWindow && ! parentWindow.isDestroyed() && ! event.sender.isDestroyed() ) { - parentWindow.webContents.send( 'theme-details-updating', details.id ); - } + sendIpcEventToRendererWithWindow( parentWindow, 'theme-details-updating', { id: details.id } ); userData.sites.push( server.details ); sortSites( userData.sites ); @@ -399,9 +397,11 @@ export async function startServer( } throw error; } - if ( parentWindow && ! parentWindow.isDestroyed() && ! event.sender.isDestroyed() ) { - parentWindow.webContents.send( 'theme-details-changed', id, server.details.themeDetails ); - } + + sendIpcEventToRendererWithWindow( parentWindow, 'theme-details-changed', { + id, + details: server.details.themeDetails, + } ); if ( server.details.running ) { try { @@ -519,13 +519,7 @@ export async function getUserLocale( _event: IpcMainInvokeEvent ): Promise< Supp export async function showUserSettings( event: IpcMainInvokeEvent ): Promise< void > { const parentWindow = BrowserWindow.fromWebContents( event.sender ); - if ( ! parentWindow ) { - throw new Error( `No window found for sender of showUserSettings message: ${ event.frameId }` ); - } - if ( parentWindow.isDestroyed() || event.sender.isDestroyed() ) { - return; - } - parentWindow.webContents.send( 'user-settings' ); + sendIpcEventToRendererWithWindow( parentWindow, 'user-settings' ); } function archiveWordPressDirectory( { @@ -671,9 +665,7 @@ export async function exportSite( try { const onEvent = ( data: ImportExportEventData ) => { const parentWindow = BrowserWindow.fromWebContents( event.sender ); - if ( parentWindow && ! parentWindow.isDestroyed() && ! event.sender.isDestroyed() ) { - parentWindow.webContents.send( 'on-export', data, siteId ); - } + sendIpcEventToRendererWithWindow( parentWindow, 'on-export', data, siteId ); }; return await exportBackup( options, onEvent ); } catch ( e ) { @@ -801,16 +793,15 @@ export async function getThemeDetails( event: IpcMainInvokeEvent, id: string ) { const parentWindow = BrowserWindow.fromWebContents( event.sender ); if ( themeDetails?.path && themeDetails.path !== server.details.themeDetails?.path ) { - if ( parentWindow && ! parentWindow.isDestroyed() && ! event.sender.isDestroyed() ) { - parentWindow.webContents.send( 'theme-details-updating', id ); - } + sendIpcEventToRendererWithWindow( parentWindow, 'theme-details-updating', { id } ); const updatedSite = { ...server.details, themeDetails, }; - if ( parentWindow && ! parentWindow.isDestroyed() && ! event.sender.isDestroyed() ) { - parentWindow.webContents.send( 'theme-details-changed', id, themeDetails ); - } + sendIpcEventToRendererWithWindow( parentWindow, 'theme-details-changed', { + id, + details: themeDetails, + } ); server.updateCachedThumbnail().then( () => sendThumbnailChangedEvent( event, id ) ); server.details.themeDetails = themeDetails; diff --git a/src/ipc-types.d.ts b/src/ipc-types.d.ts index 2a4e08449..f3ff256a4 100644 --- a/src/ipc-types.d.ts +++ b/src/ipc-types.d.ts @@ -84,10 +84,6 @@ interface AppGlobals { quickDeploysEnabled: boolean; } -interface IpcListener { - subscribe( channel: string, listener: ( ...args: any[] ) => void ): () => void; -} - // Our IPC objects will be attached to the `window` global interface Window { ipcApi: IpcApi; diff --git a/src/ipc-utils.ts b/src/ipc-utils.ts new file mode 100644 index 000000000..e9ca1e04d --- /dev/null +++ b/src/ipc-utils.ts @@ -0,0 +1,38 @@ +import { BrowserWindow } from 'electron'; +import { ImportExportEventData } from 'src/lib/import-export/handle-events'; +import { StoredToken } from 'src/lib/oauth'; +import { getMainWindow } from 'src/main-window'; + +export interface IpcEvents { + 'add-site': [ void ]; + 'auth-updated': [ { token: StoredToken } | { error: Error } ]; + 'on-export': [ ImportExportEventData, string ]; + 'on-import': [ ImportExportEventData, string ]; + 'sync-connect-site': [ { remoteSiteId: number; studioSiteId: string } ]; + 'test-render-failure': [ void ]; + 'theme-details-changed': [ { id: string; details: StartedSiteDetails[ 'themeDetails' ] } ]; + 'theme-details-updating': [ { id: string } ]; + 'thumbnail-changed': [ { id: string; imageData: string | null } ]; + 'user-settings': [ void ]; + 'window-fullscreen-change': [ boolean ]; +} + +export async function sendIpcEventToRenderer< T extends keyof IpcEvents >( + channel: T, + ...args: IpcEvents[ T ] +): Promise< void > { + const window = await getMainWindow(); + if ( ! window.isDestroyed() && ! window.webContents.isDestroyed() ) { + window.webContents.send( channel, ...args ); + } +} + +export function sendIpcEventToRendererWithWindow< T extends keyof IpcEvents >( + window: BrowserWindow | null, + channel: T, + ...args: IpcEvents[ T ] +): void { + if ( window && ! window.isDestroyed() && ! window.webContents.isDestroyed() ) { + window.webContents.send( channel, ...args ); + } +} diff --git a/src/lib/oauth.ts b/src/lib/oauth.ts index aae2984a0..e70c6af41 100644 --- a/src/lib/oauth.ts +++ b/src/lib/oauth.ts @@ -1,9 +1,9 @@ import { ipcMain } from 'electron'; import * as Sentry from '@sentry/electron/main'; import wpcom from 'wpcom'; +import { sendIpcEventToRenderer } from 'src/ipc-utils'; import { PROTOCOL_PREFIX, WP_AUTHORIZE_ENDPOINT, CLIENT_ID, SCOPES } from '../constants'; import { shellOpenExternalWrapper } from '../lib/shell-open-external-wrapper'; -import { getMainWindow } from '../main-window'; import { loadUserData, saveUserData } from '../storage/user-data'; export interface StoredToken { @@ -105,12 +105,11 @@ export function authenticate(): void { export function setUpAuthCallbackHandler() { ipcMain.on( 'auth-callback', async ( _event, { token, error } ) => { - const mainWindow = await getMainWindow(); if ( error ) { - mainWindow.webContents.send( 'auth-updated', { error } ); + sendIpcEventToRenderer( 'auth-updated', { error } ); } else { await storeToken( token ); - mainWindow.webContents.send( 'auth-updated', { token } ); + sendIpcEventToRenderer( 'auth-updated', { token } ); } } ); } diff --git a/src/main-window.ts b/src/main-window.ts index c54f3255a..78f547459 100644 --- a/src/main-window.ts +++ b/src/main-window.ts @@ -7,6 +7,7 @@ import { MAIN_MIN_WIDTH, WINDOWS_TITLEBAR_HEIGHT, } from './constants'; +import { sendIpcEventToRendererWithWindow } from './ipc-utils'; import { isEmptyDir } from './lib/fs-utils'; import { portFinder } from './lib/port-finder'; import { keepSqliteIntegrationUpdated } from './lib/sqlite-versions'; @@ -103,11 +104,11 @@ export function createMainWindow(): BrowserWindow { } ); mainWindow.on( 'enter-full-screen', () => { - mainWindow?.webContents.send( 'window-fullscreen-change', true ); + sendIpcEventToRendererWithWindow( mainWindow, 'window-fullscreen-change', true ); } ); mainWindow.on( 'leave-full-screen', () => { - mainWindow?.webContents.send( 'window-fullscreen-change', false ); + sendIpcEventToRendererWithWindow( mainWindow, 'window-fullscreen-change', false ); } ); return mainWindow; diff --git a/src/menu.ts b/src/menu.ts index 2b7cc9b56..30e2a4b09 100644 --- a/src/menu.ts +++ b/src/menu.ts @@ -2,6 +2,7 @@ import { Menu, type MenuItemConstructorOptions, app, BrowserWindow, autoUpdater import { __ } from '@wordpress/i18n'; import { openAboutWindow } from './about-menu/open-about-menu'; import { BUG_REPORT_URL, FEATURE_REQUEST_URL, STUDIO_DOCS_URL } from './constants'; +import { sendIpcEventToRenderer } from './ipc-utils'; import { shellOpenExternalWrapper } from './lib/shell-open-external-wrapper'; import { promptWindowsSpeedUpSites } from './lib/windows-helpers'; import { getMainWindow } from './main-window'; @@ -50,8 +51,7 @@ function getAppMenu( { label: __( 'Test Render Failure (dev only)' ), click: async () => { - const window = await getMainWindow(); - window.webContents.send( 'test-render-failure' ); + sendIpcEventToRenderer( 'test-render-failure' ); }, }, ]; @@ -85,8 +85,7 @@ function getAppMenu( label: __( 'Settings…' ), accelerator: 'CommandOrControl+,', click: async () => { - const window = await getMainWindow(); - window.webContents.send( 'user-settings' ); + sendIpcEventToRenderer( 'user-settings' ); }, }, { type: 'separator' }, @@ -110,8 +109,7 @@ function getAppMenu( label: __( 'Add Site…' ), accelerator: 'CommandOrControl+N', click: async () => { - const window = await getMainWindow(); - window.webContents.send( 'add-site' ); + sendIpcEventToRenderer( 'add-site' ); }, enabled: ! needsOnboarding, }, diff --git a/src/preload.ts b/src/preload.ts index 205d14d33..db562ed06 100644 --- a/src/preload.ts +++ b/src/preload.ts @@ -10,10 +10,9 @@ import { webUtils, } from 'electron'; import { LocaleData } from '@wordpress/i18n'; +import { IpcEvents } from './ipc-utils'; import { ExportOptions } from './lib/import-export/export/types'; -import { ImportExportEventData } from './lib/import-export/handle-events'; import { BackupArchiveInfo } from './lib/import-export/import/types'; -import { StoredToken } from './lib/oauth'; import { promptWindowsSpeedUpSites } from './lib/windows-helpers'; import type { SyncSite } from './hooks/use-fetch-wpcom-sites/types'; import type { LogLevel } from './logging'; @@ -120,28 +119,19 @@ const api: IpcApi = { }; contextBridge.exposeInMainWorld( 'ipcApi', api ); -export interface IpcEvents { - 'add-site': [ void ]; - 'auth-updated': [ { token: StoredToken } | { error: Error } ]; - 'on-export': [ ImportExportEventData, string ]; - 'on-import': [ ImportExportEventData, string ]; - 'sync-connect-site': [ { remoteSiteId: number; studioSiteId: string } ]; - 'test-render-failure': [ void ]; - 'theme-details-changed': [ { id: string; details: StartedSiteDetails[ 'themeDetails' ] } ]; - 'theme-details-updating': [ { id: string } ]; - 'thumbnail-changed': [ { id: string; imageData: string | null } ]; - 'user-settings': [ void ]; - 'window-fullscreen-change': [ boolean ]; -} const subscribe = < T extends keyof IpcEvents >( channel: T, listener: ( event: IpcRendererEvent, ...args: IpcEvents[ T ] ) => void ) => { - ipcRenderer.on( channel, listener ); + function wrappedListener( event: IpcRendererEvent, ...args: any[] ) { + listener( event, ...( args as IpcEvents[ T ] ) ); + } + + ipcRenderer.on( channel, wrappedListener ); return () => { - ipcRenderer.off( channel, listener ); + ipcRenderer.off( channel, wrappedListener ); }; }; From fc4b6a9705445710201e81aabb129c688d4ab700 Mon Sep 17 00:00:00 2001 From: Fredrik Rombach Ekelund Date: Mon, 3 Feb 2025 16:08:32 +0100 Subject: [PATCH 3/6] Fix tests --- src/__mocks__/electron.ts | 12 +++++++----- src/lib/tests/oauth.test.ts | 2 ++ src/tests/main-window.test.ts | 6 +++--- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/__mocks__/electron.ts b/src/__mocks__/electron.ts index fea6b8a86..98ff09719 100644 --- a/src/__mocks__/electron.ts +++ b/src/__mocks__/electron.ts @@ -29,18 +29,20 @@ export function BrowserWindow() {} BrowserWindow.prototype.loadURL = jest.fn(); BrowserWindow.prototype.isDestroyed = jest.fn( () => false ); BrowserWindow.prototype.on = jest.fn(); -BrowserWindow.prototype.webContents = { + +const mockWebContents = { on: jest.fn(), send: jest.fn(), isDestroyed: jest.fn( () => false ), }; + +BrowserWindow.prototype.webContents = mockWebContents; + BrowserWindow.fromWebContents = jest.fn( () => ( { isDestroyed: jest.fn( () => false ), - webContents: { - isDestroyed: jest.fn( () => false ), - send: jest.fn(), - }, + webContents: mockWebContents, } ) ); + BrowserWindow.getAllWindows = jest.fn( () => [] ); BrowserWindow.getFocusedWindow = jest.fn(); diff --git a/src/lib/tests/oauth.test.ts b/src/lib/tests/oauth.test.ts index 3cc164829..0443f661d 100644 --- a/src/lib/tests/oauth.test.ts +++ b/src/lib/tests/oauth.test.ts @@ -30,7 +30,9 @@ describe( 'setUpAuthCallbackHandler', () => { } ); const mockSend = jest.fn(); ( getMainWindow as jest.Mock ).mockResolvedValue( { + isDestroyed: jest.fn( () => false ), webContents: { + isDestroyed: jest.fn( () => false ), send: mockSend, }, } ); diff --git a/src/tests/main-window.test.ts b/src/tests/main-window.test.ts index a39c0ea5d..18fa3b895 100644 --- a/src/tests/main-window.test.ts +++ b/src/tests/main-window.test.ts @@ -35,7 +35,7 @@ describe( 'getMainWindow', () => { it( 'returns the focused window when the reference is destroyed', async () => { const mockWindow1 = new BrowserWindow(); const mockWindow2 = new BrowserWindow(); - ( createdWindow.isDestroyed as jest.Mock ).mockReturnValue( true ); + ( createdWindow.isDestroyed as jest.Mock ).mockReturnValueOnce( true ); ( BrowserWindow.getFocusedWindow as jest.Mock ).mockReturnValueOnce( mockWindow2 ); ( BrowserWindow.getAllWindows as jest.Mock ).mockReturnValueOnce( [ mockWindow1, @@ -49,7 +49,7 @@ describe( 'getMainWindow', () => { it( 'returns the first window when the reference is destroyed and no window is focused', async () => { const mockWindow1 = new BrowserWindow(); const mockWindow2 = new BrowserWindow(); - ( createdWindow.isDestroyed as jest.Mock ).mockReturnValue( true ); + ( createdWindow.isDestroyed as jest.Mock ).mockReturnValueOnce( true ); ( BrowserWindow.getAllWindows as jest.Mock ).mockReturnValueOnce( [ mockWindow1, mockWindow2, @@ -62,7 +62,7 @@ describe( 'getMainWindow', () => { it( 'returns a new window when no non-destroyed windows exist', async () => { // eslint-disable-next-line @typescript-eslint/no-empty-function let didFinishLoad: ( ...args: any[] ) => void = () => {}; - ( createdWindow.isDestroyed as jest.Mock ).mockReturnValue( true ); + ( createdWindow.isDestroyed as jest.Mock ).mockReturnValueOnce( true ); ( BrowserWindow.prototype.webContents.on as jest.Mock ).mockImplementation( ( _event, callback ) => { didFinishLoad = callback; From 43bffb026bdeef3a129adf1b53e844c82c100cb4 Mon Sep 17 00:00:00 2001 From: Fredrik Rombach Ekelund Date: Wed, 5 Feb 2025 11:28:04 +0100 Subject: [PATCH 4/6] Fix types and tests --- src/ipc-utils.ts | 2 +- src/lib/tests/oauth.test.ts | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/ipc-utils.ts b/src/ipc-utils.ts index e9ca1e04d..de63bbd15 100644 --- a/src/ipc-utils.ts +++ b/src/ipc-utils.ts @@ -5,7 +5,7 @@ import { getMainWindow } from 'src/main-window'; export interface IpcEvents { 'add-site': [ void ]; - 'auth-updated': [ { token: StoredToken } | { error: Error } ]; + 'auth-updated': [ { token: StoredToken } | { error: unknown } ]; 'on-export': [ ImportExportEventData, string ]; 'on-import': [ ImportExportEventData, string ]; 'sync-connect-site': [ { remoteSiteId: number; studioSiteId: string } ]; diff --git a/src/lib/tests/oauth.test.ts b/src/lib/tests/oauth.test.ts index 1c6e8d2dd..6b27afacc 100644 --- a/src/lib/tests/oauth.test.ts +++ b/src/lib/tests/oauth.test.ts @@ -1,6 +1,7 @@ /** * @jest-environment node */ +import { BrowserWindow } from 'electron'; import wpcom from 'wpcom'; import { getMainWindow } from '../../main-window'; import { loadUserData, saveUserData } from '../../storage/user-data'; @@ -65,12 +66,9 @@ describe( 'getAuthenticationToken', () => { } ); describe( 'onOpenUrlCallback', () => { + const mockMainWindow = new BrowserWindow(); const mockSend = jest.fn(); - const mockMainWindow = { - webContents: { - send: mockSend, - }, - }; + mockMainWindow.webContents.send = mockSend; beforeEach( () => { jest.clearAllMocks(); From bb4d12b24964a9a07b9485d021bda4b7c1554372 Mon Sep 17 00:00:00 2001 From: Fredrik Rombach Ekelund Date: Wed, 5 Feb 2025 11:30:15 +0100 Subject: [PATCH 5/6] Simplify mocking --- src/lib/tests/oauth.test.ts | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/lib/tests/oauth.test.ts b/src/lib/tests/oauth.test.ts index 6b27afacc..4fea16468 100644 --- a/src/lib/tests/oauth.test.ts +++ b/src/lib/tests/oauth.test.ts @@ -1,13 +1,12 @@ /** * @jest-environment node */ -import { BrowserWindow } from 'electron'; import wpcom from 'wpcom'; -import { getMainWindow } from '../../main-window'; +import { sendIpcEventToRenderer } from 'src/ipc-utils'; import { loadUserData, saveUserData } from '../../storage/user-data'; import { getAuthenticationToken, onOpenUrlCallback } from '../oauth'; -jest.mock( '../../main-window' ); +jest.mock( 'src/ipc-utils' ); jest.mock( '../../storage/user-data' ); jest.mock( 'wpcom' ); @@ -66,13 +65,8 @@ describe( 'getAuthenticationToken', () => { } ); describe( 'onOpenUrlCallback', () => { - const mockMainWindow = new BrowserWindow(); - const mockSend = jest.fn(); - mockMainWindow.webContents.send = mockSend; - beforeEach( () => { jest.clearAllMocks(); - ( getMainWindow as jest.Mock ).mockResolvedValue( mockMainWindow ); ( loadUserData as jest.Mock ).mockResolvedValue( {} ); ( saveUserData as jest.Mock ).mockResolvedValue( undefined ); } ); @@ -91,7 +85,7 @@ describe( 'onOpenUrlCallback', () => { const url = 'studio://auth#access_token=mock-token&expires_in=3600'; await onOpenUrlCallback( url ); - expect( mockSend ).toHaveBeenCalledWith( 'auth-updated', { + expect( sendIpcEventToRenderer ).toHaveBeenCalledWith( 'auth-updated', { token: expect.objectContaining( { accessToken: 'mock-token', expiresIn: 3600, @@ -107,7 +101,7 @@ describe( 'onOpenUrlCallback', () => { const url = 'studio://auth#error=access_denied'; await onOpenUrlCallback( url ); - expect( mockSend ).toHaveBeenCalledWith( 'auth-updated', { + expect( sendIpcEventToRenderer ).toHaveBeenCalledWith( 'auth-updated', { error: new Error( 'access_denied' ), } ); expect( saveUserData ).not.toHaveBeenCalled(); @@ -117,7 +111,7 @@ describe( 'onOpenUrlCallback', () => { const url = 'studio://auth#access_token=mock-token&expires_in=invalid'; await onOpenUrlCallback( url ); - expect( mockSend ).toHaveBeenCalledWith( 'auth-updated', { + expect( sendIpcEventToRenderer ).toHaveBeenCalledWith( 'auth-updated', { error: expect.any( Error ), } ); expect( saveUserData ).not.toHaveBeenCalled(); @@ -132,7 +126,7 @@ describe( 'onOpenUrlCallback', () => { const url = 'studio://auth#access_token=mock-token&expires_in=3600'; await onOpenUrlCallback( url ); - expect( mockSend ).toHaveBeenCalledWith( 'auth-updated', { + expect( sendIpcEventToRenderer ).toHaveBeenCalledWith( 'auth-updated', { error: expect.any( Error ), } ); expect( saveUserData ).not.toHaveBeenCalled(); @@ -144,7 +138,7 @@ describe( 'onOpenUrlCallback', () => { const url = 'studio://sync-connect-site?remoteSiteId=123&studioSiteId=local-site'; await onOpenUrlCallback( url ); - expect( mockSend ).toHaveBeenCalledWith( 'sync-connect-site', { + expect( sendIpcEventToRenderer ).toHaveBeenCalledWith( 'sync-connect-site', { remoteSiteId: 123, studioSiteId: 'local-site', } ); @@ -154,7 +148,7 @@ describe( 'onOpenUrlCallback', () => { const url = 'studio://sync-connect-site?remoteSiteId=123'; await onOpenUrlCallback( url ); - expect( mockSend ).not.toHaveBeenCalled(); + expect( sendIpcEventToRenderer ).not.toHaveBeenCalled(); } ); } ); } ); From ec5bef8d4124fff1fea695698bffb6ed91516f67 Mon Sep 17 00:00:00 2001 From: Fredrik Rombach Ekelund Date: Mon, 10 Feb 2025 10:45:46 +0100 Subject: [PATCH 6/6] Fix tests --- src/lib/tests/oauth.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/tests/oauth.test.ts b/src/lib/tests/oauth.test.ts index b5e84f866..3fcd24c4a 100644 --- a/src/lib/tests/oauth.test.ts +++ b/src/lib/tests/oauth.test.ts @@ -6,7 +6,7 @@ import { sendIpcEventToRenderer } from 'src/ipc-utils'; import { getAuthenticationToken, onOpenUrlCallback } from 'src/lib/oauth'; import { loadUserData, saveUserData } from 'src/storage/user-data'; -jest.mock( 'src/main-window' ); +jest.mock( 'src/ipc-utils' ); jest.mock( 'src/storage/user-data' ); jest.mock( 'wpcom' );