From d8a56a6d58debfda376791871be466f6c8b1f089 Mon Sep 17 00:00:00 2001 From: Jason Nall Date: Thu, 20 Apr 2023 13:36:29 -0400 Subject: [PATCH 1/6] Rework how subscriptions are cleaned up. --- .../src/main/createIPCHandler.ts | 38 +++++++++--- ...dleIPCOperation.ts => handleIPCMessage.ts} | 29 ++++++--- .../src/renderer/__tests__/ipcLink.test.ts | 60 ++++++++++++------- .../electron-trpc/src/renderer/ipcLink.ts | 34 ++++++----- packages/electron-trpc/src/types.ts | 6 +- 5 files changed, 116 insertions(+), 51 deletions(-) rename packages/electron-trpc/src/main/{handleIPCOperation.ts => handleIPCMessage.ts} (77%) diff --git a/packages/electron-trpc/src/main/createIPCHandler.ts b/packages/electron-trpc/src/main/createIPCHandler.ts index c8bb355c..8e1ca77b 100644 --- a/packages/electron-trpc/src/main/createIPCHandler.ts +++ b/packages/electron-trpc/src/main/createIPCHandler.ts @@ -1,15 +1,17 @@ -import type { Operation } from '@trpc/client'; import type { AnyRouter, inferRouterContext } from '@trpc/server'; import { ipcMain } from 'electron'; import type { BrowserWindow, IpcMainInvokeEvent } from 'electron'; -import { handleIPCOperation } from './handleIPCOperation'; +import { handleIPCMessage } from './handleIPCMessage'; import { CreateContextOptions } from './types'; import { ELECTRON_TRPC_CHANNEL } from '../constants'; +import { ETRPCRequest } from '../types'; +import { Unsubscribable } from '@trpc/server/observable'; type Awaitable = T | Promise; class IPCHandler { - #windows: BrowserWindow[]; + #windows: BrowserWindow[] = []; + #subscriptions: Map = new Map(); constructor({ createContext, @@ -20,24 +22,46 @@ class IPCHandler { router: TRouter; windows?: BrowserWindow[]; }) { - this.#windows = windows; + windows.forEach((win) => this.attachWindow(win)); - ipcMain.on(ELECTRON_TRPC_CHANNEL, (event: IpcMainInvokeEvent, args: Operation) => { - handleIPCOperation({ + ipcMain.on(ELECTRON_TRPC_CHANNEL, (event: IpcMainInvokeEvent, args: ETRPCRequest) => { + const internalId = `${event.sender.id}-${event.senderFrame.routingId}:${args.id}`; + + handleIPCMessage({ router, createContext, + internalId, event, - operation: args, + message: args, + subscriptions: this.#subscriptions, }); }); } attachWindow(win: BrowserWindow) { + if (this.#windows.includes(win)) { + return; + } + this.#windows.push(win); + this.#attachSubscriptionCleanupHandler(win); } detachWindow(win: BrowserWindow) { this.#windows = this.#windows.filter((w) => w !== win); + + for (const [key, sub] of this.#subscriptions.entries()) { + if (key.startsWith(`${win.webContents.id}-`)) { + sub.unsubscribe(); + this.#subscriptions.delete(key); + } + } + } + + #attachSubscriptionCleanupHandler(win: BrowserWindow) { + win.webContents.on('destroyed', () => { + this.detachWindow(win); + }); } } diff --git a/packages/electron-trpc/src/main/handleIPCOperation.ts b/packages/electron-trpc/src/main/handleIPCMessage.ts similarity index 77% rename from packages/electron-trpc/src/main/handleIPCOperation.ts rename to packages/electron-trpc/src/main/handleIPCMessage.ts index 05512a86..325807e8 100644 --- a/packages/electron-trpc/src/main/handleIPCOperation.ts +++ b/packages/electron-trpc/src/main/handleIPCMessage.ts @@ -2,24 +2,39 @@ import { callProcedure, TRPCError } from '@trpc/server'; import type { AnyRouter, inferRouterContext } from '@trpc/server'; import type { TRPCResponseMessage } from '@trpc/server/rpc'; import type { IpcMainInvokeEvent } from 'electron'; -import { isObservable } from '@trpc/server/observable'; -import { Operation } from '@trpc/client'; +import { isObservable, Unsubscribable } from '@trpc/server/observable'; import { CreateContextOptions } from './types'; import { getTRPCErrorFromUnknown, transformTRPCResponseItem } from './utils'; import { ELECTRON_TRPC_CHANNEL } from '../constants'; +import { ETRPCRequest } from '../types'; -export async function handleIPCOperation({ +export async function handleIPCMessage({ router, createContext, - operation, + internalId, + message, event, + subscriptions, }: { router: TRouter; createContext?: (opts: CreateContextOptions) => Promise>; - operation: Operation; + internalId: string; + message: ETRPCRequest; event: IpcMainInvokeEvent; + subscriptions: Map; }) { - const { type, input: serializedInput, id, path } = operation; + if (message.method === 'subscription.stop') { + const subscription = subscriptions.get(internalId); + if (!subscription) { + return; + } + + subscription.unsubscribe(); + subscriptions.delete(internalId); + return; + } + + const { type, input: serializedInput, path, id } = message.operation; const input = serializedInput ? router._def._config.transformer.input.deserialize(serializedInput) : undefined; @@ -91,7 +106,7 @@ export async function handleIPCOperation({ }, }); - event.sender.on('destroyed', () => subscription.unsubscribe()); + subscriptions.set(internalId, subscription); } catch (cause) { const error: TRPCError = getTRPCErrorFromUnknown(cause); diff --git a/packages/electron-trpc/src/renderer/__tests__/ipcLink.test.ts b/packages/electron-trpc/src/renderer/__tests__/ipcLink.test.ts index 139da985..b3f7f225 100644 --- a/packages/electron-trpc/src/renderer/__tests__/ipcLink.test.ts +++ b/packages/electron-trpc/src/renderer/__tests__/ipcLink.test.ts @@ -58,11 +58,15 @@ describe('ipcLink', () => { expect(mock.sendMessage).toHaveBeenCalledTimes(1); expect(mock.sendMessage).toHaveBeenCalledWith({ - context: {}, id: 1, - input: undefined, - path: 'testQuery', - type: 'query', + method: 'request', + operation: { + context: {}, + id: 1, + input: undefined, + path: 'testQuery', + type: 'query', + }, }); expect(queryResponse).not.toHaveBeenCalled(); @@ -88,11 +92,15 @@ describe('ipcLink', () => { expect(mock.sendMessage).toHaveBeenCalledTimes(1); expect(mock.sendMessage).toHaveBeenCalledWith({ - context: {}, id: 1, - input: 'test input', - path: 'testMutation', - type: 'mutation', + method: 'request', + operation: { + context: {}, + id: 1, + input: 'test input', + path: 'testMutation', + type: 'mutation', + }, }); mock.sendMessage.mockClear(); @@ -125,11 +133,15 @@ describe('ipcLink', () => { expect(mock.sendMessage).toHaveBeenCalledTimes(1); expect(mock.sendMessage).toHaveBeenCalledWith({ - context: {}, id: 1, - input: undefined, - path: 'testSubscription', - type: 'subscription', + method: 'request', + operation: { + context: {}, + id: 1, + input: undefined, + path: 'testSubscription', + type: 'subscription', + }, }); /* @@ -234,11 +246,15 @@ describe('ipcLink', () => { expect(mock.sendMessage).toHaveBeenCalledTimes(1); expect(mock.sendMessage).toHaveBeenCalledWith({ - context: {}, id: 1, - input: superjson.serialize(input), - path: 'testInputs', - type: 'query', + method: 'request', + operation: { + context: {}, + id: 1, + input: superjson.serialize(input), + path: 'testInputs', + type: 'query', + }, }); expect(queryResponse).not.toHaveBeenCalled(); @@ -278,11 +294,15 @@ describe('ipcLink', () => { expect(mock.sendMessage).toHaveBeenCalledTimes(1); expect(mock.sendMessage).toHaveBeenCalledWith({ - context: {}, id: 1, - input: JSON.stringify(input), - path: 'testInputs', - type: 'query', + method: 'request', + operation: { + id: 1, + context: {}, + input: JSON.stringify(input), + path: 'testInputs', + type: 'query', + }, }); expect(queryResponse).not.toHaveBeenCalled(); diff --git a/packages/electron-trpc/src/renderer/ipcLink.ts b/packages/electron-trpc/src/renderer/ipcLink.ts index eccc13de..4c2227e0 100644 --- a/packages/electron-trpc/src/renderer/ipcLink.ts +++ b/packages/electron-trpc/src/renderer/ipcLink.ts @@ -22,16 +22,16 @@ type IPCRequest = { }; const getElectronTRPC = () => { - const electronTRPC: RendererGlobalElectronTRPC = (globalThis as any).electronTRPC; + const electronTRPC: RendererGlobalElectronTRPC = (globalThis as any).electronTRPC; - if (!electronTRPC) { - throw new Error( - 'Could not find `electronTRPC` global. Check that `exposeElectronTPRC` has been called in your preload file.' - ); - } + if (!electronTRPC) { + throw new Error( + 'Could not find `electronTRPC` global. Check that `exposeElectronTPRC` has been called in your preload file.' + ); + } - return electronTRPC; -} + return electronTRPC; +}; class IPCClient { #pendingRequests = new Map(); @@ -57,7 +57,7 @@ class IPCClient { } request(op: Operation, callbacks: IPCCallbacks) { - const { id, type } = op; + const { type, id } = op; this.#pendingRequests.set(id, { type, @@ -65,19 +65,21 @@ class IPCClient { op, }); - this.#electronTRPC.sendMessage(op); + this.#electronTRPC.sendMessage({ method: 'request', operation: op, id: id as number }); return () => { - const callbacks = this.#pendingRequests.get(op.id)?.callbacks; + const callbacks = this.#pendingRequests.get(id)?.callbacks; - this.#pendingRequests.delete(op.id); + this.#pendingRequests.delete(id); callbacks?.complete(); - this.#electronTRPC.sendMessage({ - id, - method: 'subscription.stop', - } as any); + if (type === 'subscription') { + this.#electronTRPC.sendMessage({ + id, + method: 'subscription.stop', + }); + } }; } } diff --git a/packages/electron-trpc/src/types.ts b/packages/electron-trpc/src/types.ts index f0c55a1a..8a01a66c 100644 --- a/packages/electron-trpc/src/types.ts +++ b/packages/electron-trpc/src/types.ts @@ -1,7 +1,11 @@ import type { Operation } from '@trpc/client'; import type { TRPCResponseMessage } from '@trpc/server/rpc'; +export type ETRPCRequest = + | { method: 'request'; id: number; operation: Operation } + | { method: 'subscription.stop'; id: number }; + export interface RendererGlobalElectronTRPC { - sendMessage: (args: Operation) => void; + sendMessage: (args: ETRPCRequest) => void; onMessage: (callback: (args: TRPCResponseMessage) => void) => void; } From 03d65b2321da8254e2379363118950d72d283cd4 Mon Sep 17 00:00:00 2001 From: Jason Nall Date: Thu, 20 Apr 2023 14:17:10 -0400 Subject: [PATCH 2/6] Add e2e tests. --- .github/workflows/main.yml | 33 ++++++++++++-- examples/basic-react/index.e2e.ts | 14 ++++++ package.json | 3 ++ packages/electron-trpc/package.json | 4 ++ .../electron-trpc/src/renderer/ipcLink.ts | 4 ++ playwright.config.ts | 5 +++ pnpm-lock.yaml | 45 +++++++++++++++++++ shell.nix | 2 + 8 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 examples/basic-react/index.e2e.ts create mode 100644 playwright.config.ts diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index cbe08172..2f5fdcf0 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -1,11 +1,16 @@ name: CI + on: [push] + +env: + PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: 1 + jobs: test: runs-on: ${{ matrix.os }} strategy: matrix: - node: ['16.x'] + node: ['18.x'] os: [ubuntu-latest] steps: @@ -19,15 +24,35 @@ jobs: cache: 'pnpm' - run: pnpm install --frozen-lockfile - run: pnpm build - - run: pnpm --filter=examples/basic build - run: pnpm test:coverage - uses: codecov/codecov-action@v3 + test-e2e: + runs-on: ${{ matrix.os }} + strategy: + matrix: + node: ['18.x'] + os: [ubuntu-latest] + + steps: + - uses: actions/checkout@v3 + - uses: pnpm/action-setup@v2 + with: + version: 7 + - uses: actions/setup-node@v3 + with: + node-version: ${{ matrix.node }} + cache: 'pnpm' + - run: pnpm install --frozen-lockfile + - run: pnpm build + - run: pnpm --filter="examples/*" build + - run: pnpm test:e2e + release-main: runs-on: ${{ matrix.os }} strategy: matrix: - node: ['16.x'] + node: ['18.x'] os: [ubuntu-latest] needs: test @@ -55,7 +80,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - node: ['16.x'] + node: ['18.x'] os: [ubuntu-latest] needs: test diff --git a/examples/basic-react/index.e2e.ts b/examples/basic-react/index.e2e.ts new file mode 100644 index 00000000..96314eb6 --- /dev/null +++ b/examples/basic-react/index.e2e.ts @@ -0,0 +1,14 @@ +import { _electron as electron } from 'playwright'; +import { test, expect } from '@playwright/test'; + +test('Hello Electron', async () => { + const electronApp = await electron.launch({ args: [`${__dirname}`] }); + + const window = await electronApp.firstWindow(); + expect(await window.title()).toBe('Hello from Electron renderer!'); + + const response = await window.textContent('[data-testid="greeting"]'); + expect(response).toBe('Hello Electron'); + + await electronApp.close(); +}); diff --git a/package.json b/package.json index 49778f1c..18894314 100644 --- a/package.json +++ b/package.json @@ -3,6 +3,7 @@ "scripts": { "build": "pnpm --filter=electron-trpc build", "test": "pnpm --filter=electron-trpc test", + "test:e2e": "playwright test", "test:coverage": "pnpm --filter=electron-trpc test:coverage", "prepublish": "yarn build", "changeset": "changeset", @@ -15,6 +16,8 @@ "devDependencies": { "@changesets/changelog-github": "^0.4.8", "@changesets/cli": "^2.26.1", + "@playwright/test": "^1.32.3", + "playwright": "^1.32.3", "prettier": "^2.8.7", "typescript": "^5.0.4", "unocss": "^0.51.4", diff --git a/packages/electron-trpc/package.json b/packages/electron-trpc/package.json index fd0ff3f6..004421ac 100644 --- a/packages/electron-trpc/package.json +++ b/packages/electron-trpc/package.json @@ -36,6 +36,7 @@ "@tanstack/react-query": "^4.29.1", "@trpc/client": "10.20.0", "@trpc/server": "10.20.0", + "@types/debug": "^4.1.7", "@types/node": "^18.15.11", "@vitest/coverage-c8": "^0.30.1", "builtin-modules": "^3.3.0", @@ -53,5 +54,8 @@ "@trpc/client": ">10.0.0", "@trpc/server": ">10.0.0", "electron": ">19.0.0" + }, + "dependencies": { + "debug": "^4.3.4" } } diff --git a/packages/electron-trpc/src/renderer/ipcLink.ts b/packages/electron-trpc/src/renderer/ipcLink.ts index 4c2227e0..f598cf9e 100644 --- a/packages/electron-trpc/src/renderer/ipcLink.ts +++ b/packages/electron-trpc/src/renderer/ipcLink.ts @@ -4,6 +4,9 @@ import type { TRPCResponseMessage } from '@trpc/server/rpc'; import type { RendererGlobalElectronTRPC } from '../types'; import { observable, Observer } from '@trpc/server/observable'; import { transformResult } from './utils'; +import debug from 'debug'; + +const log = debug('electron-trpc:renderer:ipcLink'); type IPCCallbackResult = TRPCResponseMessage< unknown, @@ -44,6 +47,7 @@ class IPCClient { } #handleResponse(response: TRPCResponseMessage) { + log('handling response', response); const request = response.id && this.#pendingRequests.get(response.id); if (!request) { return; diff --git a/playwright.config.ts b/playwright.config.ts new file mode 100644 index 00000000..c9fef873 --- /dev/null +++ b/playwright.config.ts @@ -0,0 +1,5 @@ +import { defineConfig } from '@playwright/test'; + +export default defineConfig({ + testMatch: /.*\.e2e\.ts/, +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a5b6901b..b81bd08a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -6,6 +6,8 @@ importers: specifiers: '@changesets/changelog-github': ^0.4.8 '@changesets/cli': ^2.26.1 + '@playwright/test': ^1.32.3 + playwright: ^1.32.3 prettier: ^2.8.7 typescript: ^5.0.4 unocss: ^0.51.4 @@ -15,6 +17,8 @@ importers: devDependencies: '@changesets/changelog-github': 0.4.8 '@changesets/cli': 2.26.1 + '@playwright/test': 1.32.3 + playwright: 1.32.3 prettier: 2.8.7 typescript: 5.0.4 unocss: 0.51.4_vite@4.2.1 @@ -99,9 +103,11 @@ importers: '@tanstack/react-query': ^4.29.1 '@trpc/client': 10.20.0 '@trpc/server': 10.20.0 + '@types/debug': ^4.1.7 '@types/node': ^18.15.11 '@vitest/coverage-c8': ^0.30.1 builtin-modules: ^3.3.0 + debug: ^4.3.4 dts-bundle-generator: ^8.0.0 electron: ^24.1.1 react: ^18.2.0 @@ -111,10 +117,13 @@ importers: vite-plugin-commonjs-externals: ^0.1.1 vitest: ^0.30.1 zod: ^3.21.4 + dependencies: + debug: 4.3.4 devDependencies: '@tanstack/react-query': 4.29.1_biqbaboplfbrettd7655fr4n2y '@trpc/client': 10.20.0_@trpc+server@10.20.0 '@trpc/server': 10.20.0 + '@types/debug': 4.1.7 '@types/node': 18.15.11 '@vitest/coverage-c8': 0.30.1_vitest@0.30.1 builtin-modules: 3.3.0 @@ -1046,6 +1055,17 @@ packages: fastq: 1.15.0 dev: true + /@playwright/test/1.32.3: + resolution: {integrity: sha512-BvWNvK0RfBriindxhLVabi8BRe3X0J9EVjKlcmhxjg4giWBD/xleLcg2dz7Tx0agu28rczjNIPQWznwzDwVsZQ==} + engines: {node: '>=14'} + hasBin: true + dependencies: + '@types/node': 18.15.11 + playwright-core: 1.32.3 + optionalDependencies: + fsevents: 2.3.2 + dev: true + /@polka/url/1.0.0-next.21: resolution: {integrity: sha512-a5Sab1C4/icpTZVzZc5Ghpz88yQtGOyNqYXcZgOssB2uuAr+wF/MvN6bgtW32q7HHrvBki+BsZ0OuNv6EV3K9g==} dev: true @@ -1138,6 +1158,12 @@ packages: resolution: {integrity: sha512-KnRanxnpfpjUTqTCXslZSEdLfXExwgNxYPdiO2WGUj8+HDjFi8R3k5RVKPeSCzLjCcshCAtVO2QBbVuAV4kTnw==} dev: true + /@types/debug/4.1.7: + resolution: {integrity: sha512-9AonUzyTjXXhEOa0DnqpzZi6VHlqKMswga9EXjpXnnqxwLtdvPPtlO8evrI5D9S6asFRCQ6v+wpiUKbw+vKqyg==} + dependencies: + '@types/ms': 0.7.31 + dev: true + /@types/estree/1.0.0: resolution: {integrity: sha512-WulqXMDUTYAXCjZnk6JtIHPigp55cVtDgDrO2gHRwhyJto21+1zbVCtOYB2L1F9w4qCQ0rOGWBnBe0FNTiEJIQ==} dev: true @@ -1164,6 +1190,10 @@ packages: resolution: {integrity: sha512-jhuKLIRrhvCPLqwPcx6INqmKeiA5EWrsCOPhrlFSrbrmU4ZMPjj5Ul/oLCMDO98XRUIwVm78xICz4EPCektzeQ==} dev: true + /@types/ms/0.7.31: + resolution: {integrity: sha512-iiUgKzV9AuaEkZqkOLDIvlQiL6ltuZd9tGcW3gwpnX8JbuiuhFlEGmmFXEXkN50Cvq7Os88IY2v0dkDqXYWVgA==} + dev: true + /@types/node/12.20.55: resolution: {integrity: sha512-J8xLz7q2OFulZ2cyGTLE1TbbZcjpno7FaN6zdJNrgAdrJ+DZzh/uFR6YrTb4C+nXakvud8Q4+rbhoIWlYQbUFQ==} dev: true @@ -3434,6 +3464,21 @@ packages: pathe: 1.1.0 dev: true + /playwright-core/1.32.3: + resolution: {integrity: sha512-SB+cdrnu74ZIn5Ogh/8278ngEh9NEEV0vR4sJFmK04h2iZpybfbqBY0bX6+BLYWVdV12JLLI+JEFtSnYgR+mWg==} + engines: {node: '>=14'} + hasBin: true + dev: true + + /playwright/1.32.3: + resolution: {integrity: sha512-h/ylpgoj6l/EjkfUDyx8cdOlfzC96itPpPe8BXacFkqpw/YsuxkpPyVbzEq4jw+bAJh5FLgh31Ljg2cR6HV3uw==} + engines: {node: '>=14'} + hasBin: true + requiresBuild: true + dependencies: + playwright-core: 1.32.3 + dev: true + /postcss/8.4.21: resolution: {integrity: sha512-tP7u/Sn/dVxK2NnruI4H9BG+x+Wxz6oeZ1cJ8P6G/PZY0IKk4k/63TDsQf2kQq3+qoJeLm2kIBUNlZe3zgb4Zg==} engines: {node: ^10 || ^12 || >=14} diff --git a/shell.nix b/shell.nix index 8edb6a13..29a46f84 100644 --- a/shell.nix +++ b/shell.nix @@ -8,4 +8,6 @@ mkShell { nodejs-18_x nodePackages.pnpm ]; + + PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1; } From cef8f617affd3a2386dc3be64bb97b96dd1c3dea Mon Sep 17 00:00:00 2001 From: Jason Nall Date: Thu, 20 Apr 2023 14:47:44 -0400 Subject: [PATCH 3/6] Fix tests. --- ...ation.test.ts => handleIPCMessage.test.ts} | 169 ++++++------------ .../src/main/createIPCHandler.ts | 13 +- .../src/renderer/__tests__/ipcLink.test.ts | 5 - .../electron-trpc/src/renderer/ipcLink.ts | 2 +- packages/electron-trpc/src/types.ts | 2 +- 5 files changed, 63 insertions(+), 128 deletions(-) rename packages/electron-trpc/src/main/__tests__/{handleIPCOperation.test.ts => handleIPCMessage.test.ts} (57%) diff --git a/packages/electron-trpc/src/main/__tests__/handleIPCOperation.test.ts b/packages/electron-trpc/src/main/__tests__/handleIPCMessage.test.ts similarity index 57% rename from packages/electron-trpc/src/main/__tests__/handleIPCOperation.test.ts rename to packages/electron-trpc/src/main/__tests__/handleIPCMessage.test.ts index f2a9965c..3b7aafb1 100644 --- a/packages/electron-trpc/src/main/__tests__/handleIPCOperation.test.ts +++ b/packages/electron-trpc/src/main/__tests__/handleIPCMessage.test.ts @@ -3,7 +3,7 @@ import { z } from 'zod'; import * as trpc from '@trpc/server'; import { observable } from '@trpc/server/observable'; import { EventEmitter } from 'events'; -import { handleIPCOperation } from '../handleIPCOperation'; +import { handleIPCMessage } from '../handleIPCMessage'; import { IpcMainInvokeEvent } from 'electron'; interface MockEvent { @@ -51,11 +51,22 @@ describe('api', () => { }, }); - await handleIPCOperation({ + await handleIPCMessage({ createContext: async () => ({}), - operation: { context: {}, id: 1, input: { id: 'test-id' }, path: 'testQuery', type: 'query' }, - router: testRouter, event, + internalId: '1-1:1', + message: { + method: 'request', + operation: { + context: {}, + id: 1, + input: { id: 'test-id' }, + path: 'testQuery', + type: 'query', + }, + }, + router: testRouter, + subscriptions: new Map(), }); expect(event.sender.send).toHaveBeenCalledOnce(); @@ -79,11 +90,22 @@ describe('api', () => { }, }); - await handleIPCOperation({ + await handleIPCMessage({ createContext: async () => ({}), - operation: { context: {}, id: 1, input: { id: 'test-id' }, path: 'testQuery', type: 'query' }, - router: testRouter, event, + internalId: '1-1:1', + message: { + method: 'request', + operation: { + context: {}, + id: 1, + input: { id: 'test-id' }, + path: 'testQuery', + type: 'query', + }, + }, + router: testRouter, + subscriptions: new Map(), }); expect(event.sender.send).not.toHaveBeenCalled(); @@ -98,15 +120,20 @@ describe('api', () => { }, }); - await handleIPCOperation({ + await handleIPCMessage({ createContext: async () => ({}), - operation: { - context: {}, - id: 1, - input: undefined, - path: 'testSubscription', - type: 'subscription', + message: { + method: 'request', + operation: { + context: {}, + id: 1, + input: undefined, + path: 'testSubscription', + type: 'subscription', + }, }, + internalId: '1-1:1', + subscriptions: new Map(), router: testRouter, event, }); @@ -124,41 +151,6 @@ describe('api', () => { }); }); - test('cancels subscriptions when webcontents are closed', async () => { - let isDestroyed = false; - let onDestroyed: () => void; - const event = makeEvent({ - sender: { - isDestroyed: () => isDestroyed, - send: vi.fn(), - on: (_event: string, cb: () => void) => { - onDestroyed = cb; - }, - }, - }); - - await handleIPCOperation({ - createContext: async () => ({}), - operation: { - context: {}, - id: 1, - input: undefined, - path: 'testSubscription', - type: 'subscription', - }, - router: testRouter, - event, - }); - - expect(event.sender.send).not.toHaveBeenCalled(); - - onDestroyed(); - - ee.emit('test'); - - expect(event.sender.send).not.toHaveBeenCalled(); - }); - test('subscription responds using custom serializer', async () => { const event = makeEvent({ sender: { @@ -193,15 +185,20 @@ describe('api', () => { }), }); - await handleIPCOperation({ + await handleIPCMessage({ createContext: async () => ({}), - operation: { - context: {}, - id: 1, - input: undefined, - path: 'testSubscription', - type: 'subscription', + message: { + method: 'request', + operation: { + context: {}, + id: 1, + input: undefined, + path: 'testSubscription', + type: 'subscription', + }, }, + internalId: '1-1:1', + subscriptions: new Map(), router: testRouter, event, }); @@ -219,64 +216,4 @@ describe('api', () => { }, }); }); - - test("doesn't crash when canceling subscriptions when custom deserializer doesn't allow undefined", async () => { - const t = trpc.initTRPC.create({ - transformer: { - deserialize: (input: unknown) => { - if (!input) throw new Error("Can't parse empty input"); - return JSON.parse(input as string); - }, - serialize: (input) => { - return JSON.stringify(input); - }, - }, - }); - - const testRouter = t.router({ - testSubscription: t.procedure.subscription(() => { - return observable((emit) => { - function testResponse() { - emit.next('test response'); - } - - ee.on('test', testResponse); - return () => ee.off('test', testResponse); - }); - }), - }); - - let isDestroyed = false; - let onDestroyed: () => void; - const event = makeEvent({ - sender: { - isDestroyed: () => isDestroyed, - send: vi.fn(), - on: (_event: string, cb: () => void) => { - onDestroyed = cb; - }, - }, - }); - - await handleIPCOperation({ - createContext: async () => ({}), - operation: { - context: {}, - id: 1, - input: undefined, - path: 'testSubscription', - type: 'subscription', - }, - router: testRouter, - event, - }); - - expect(event.sender.send).not.toHaveBeenCalled(); - - onDestroyed(); - - ee.emit('test'); - - expect(event.sender.send).not.toHaveBeenCalled(); - }); }); diff --git a/packages/electron-trpc/src/main/createIPCHandler.ts b/packages/electron-trpc/src/main/createIPCHandler.ts index 8e1ca77b..67801f44 100644 --- a/packages/electron-trpc/src/main/createIPCHandler.ts +++ b/packages/electron-trpc/src/main/createIPCHandler.ts @@ -9,6 +9,11 @@ import { Unsubscribable } from '@trpc/server/observable'; type Awaitable = T | Promise; +const getInternalId = (event: IpcMainInvokeEvent, request: ETRPCRequest) => { + const messageId = request.method === 'request' ? request.operation.id : request.id; + return `${event.sender.id}-${event.senderFrame.routingId}:${messageId}`; +}; + class IPCHandler { #windows: BrowserWindow[] = []; #subscriptions: Map = new Map(); @@ -24,15 +29,13 @@ class IPCHandler { }) { windows.forEach((win) => this.attachWindow(win)); - ipcMain.on(ELECTRON_TRPC_CHANNEL, (event: IpcMainInvokeEvent, args: ETRPCRequest) => { - const internalId = `${event.sender.id}-${event.senderFrame.routingId}:${args.id}`; - + ipcMain.on(ELECTRON_TRPC_CHANNEL, (event: IpcMainInvokeEvent, request: ETRPCRequest) => { handleIPCMessage({ router, createContext, - internalId, + internalId: getInternalId(event, request), event, - message: args, + message: request, subscriptions: this.#subscriptions, }); }); diff --git a/packages/electron-trpc/src/renderer/__tests__/ipcLink.test.ts b/packages/electron-trpc/src/renderer/__tests__/ipcLink.test.ts index b3f7f225..5aaf4b4e 100644 --- a/packages/electron-trpc/src/renderer/__tests__/ipcLink.test.ts +++ b/packages/electron-trpc/src/renderer/__tests__/ipcLink.test.ts @@ -58,7 +58,6 @@ describe('ipcLink', () => { expect(mock.sendMessage).toHaveBeenCalledTimes(1); expect(mock.sendMessage).toHaveBeenCalledWith({ - id: 1, method: 'request', operation: { context: {}, @@ -92,7 +91,6 @@ describe('ipcLink', () => { expect(mock.sendMessage).toHaveBeenCalledTimes(1); expect(mock.sendMessage).toHaveBeenCalledWith({ - id: 1, method: 'request', operation: { context: {}, @@ -133,7 +131,6 @@ describe('ipcLink', () => { expect(mock.sendMessage).toHaveBeenCalledTimes(1); expect(mock.sendMessage).toHaveBeenCalledWith({ - id: 1, method: 'request', operation: { context: {}, @@ -246,7 +243,6 @@ describe('ipcLink', () => { expect(mock.sendMessage).toHaveBeenCalledTimes(1); expect(mock.sendMessage).toHaveBeenCalledWith({ - id: 1, method: 'request', operation: { context: {}, @@ -294,7 +290,6 @@ describe('ipcLink', () => { expect(mock.sendMessage).toHaveBeenCalledTimes(1); expect(mock.sendMessage).toHaveBeenCalledWith({ - id: 1, method: 'request', operation: { id: 1, diff --git a/packages/electron-trpc/src/renderer/ipcLink.ts b/packages/electron-trpc/src/renderer/ipcLink.ts index f598cf9e..830a2c7f 100644 --- a/packages/electron-trpc/src/renderer/ipcLink.ts +++ b/packages/electron-trpc/src/renderer/ipcLink.ts @@ -69,7 +69,7 @@ class IPCClient { op, }); - this.#electronTRPC.sendMessage({ method: 'request', operation: op, id: id as number }); + this.#electronTRPC.sendMessage({ method: 'request', operation: op }); return () => { const callbacks = this.#pendingRequests.get(id)?.callbacks; diff --git a/packages/electron-trpc/src/types.ts b/packages/electron-trpc/src/types.ts index 8a01a66c..3043455b 100644 --- a/packages/electron-trpc/src/types.ts +++ b/packages/electron-trpc/src/types.ts @@ -2,7 +2,7 @@ import type { Operation } from '@trpc/client'; import type { TRPCResponseMessage } from '@trpc/server/rpc'; export type ETRPCRequest = - | { method: 'request'; id: number; operation: Operation } + | { method: 'request'; operation: Operation } | { method: 'subscription.stop'; id: number }; export interface RendererGlobalElectronTRPC { From 162b29d2c5d97d18fc4da313364deb0fdf095fbb Mon Sep 17 00:00:00 2001 From: Jason Nall Date: Thu, 20 Apr 2023 14:58:21 -0400 Subject: [PATCH 4/6] Tweak e2e pipeline. --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 2f5fdcf0..390cfcac 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -46,7 +46,7 @@ jobs: - run: pnpm install --frozen-lockfile - run: pnpm build - run: pnpm --filter="examples/*" build - - run: pnpm test:e2e + - run: xvfb-run --auto-servernum -- pnpm test:e2e release-main: runs-on: ${{ matrix.os }} From 4de9acef9aaf23172de21cf3484976c854fb1d4b Mon Sep 17 00:00:00 2001 From: Jason Nall Date: Thu, 20 Apr 2023 15:02:03 -0400 Subject: [PATCH 5/6] Fix e2e test. --- examples/basic-react/src/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/basic-react/src/index.tsx b/examples/basic-react/src/index.tsx index d71a4168..4efe1a86 100644 --- a/examples/basic-react/src/index.tsx +++ b/examples/basic-react/src/index.tsx @@ -36,7 +36,7 @@ function HelloElectron() { return null; } - return
{data.text}
; + return
{data.text}
; } ReactDom.render(, document.getElementById('react-root')); From a2992352b3473c0388cd2baa47a5d2431968dda9 Mon Sep 17 00:00:00 2001 From: Jason Nall Date: Thu, 20 Apr 2023 15:06:32 -0400 Subject: [PATCH 6/6] Add changeset. --- .changeset/swift-lies-carry.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/swift-lies-carry.md diff --git a/.changeset/swift-lies-carry.md b/.changeset/swift-lies-carry.md new file mode 100644 index 00000000..85f68d7f --- /dev/null +++ b/.changeset/swift-lies-carry.md @@ -0,0 +1,5 @@ +--- +'electron-trpc': minor +--- + +Rework how subscriptions are cleaned up.