Rework how subscriptions are cleaned up. (jsonnull#138)

* Rework how subscriptions are cleaned up. * Add e2e tests. * Fix tests. * Tweak e2e pipeline. * Fix e2e test. * Add changeset.
azamuddin · Apr 20, 2023 · 68ddf63 · 68ddf63
1 parent 0a4a01f
commit 68ddf63
Show file tree

Hide file tree

Showing 15 changed files with 284 additions and 177 deletions.
diff --git a/.changeset/swift-lies-carry.md b/.changeset/swift-lies-carry.md
@@ -0,0 +1,5 @@
+---
+'electron-trpc': minor
+---
+
+Rework how subscriptions are cleaned up.
diff --git 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: xvfb-run --auto-servernum -- 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
@@ -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/examples/basic-react/src/index.tsx b/examples/basic-react/src/index.tsx
@@ -36,7 +36,7 @@ function HelloElectron() {
     return null;
   }
 
-  return <div>{data.text}</div>;
+  return <div data-testid="greeting">{data.text}</div>;
 }
 
 ReactDom.render(<App />, document.getElementById('react-root'));
diff --git 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
@@ -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/...main/__tests__/handleIPCOperation.test.ts → ...c/main/__tests__/handleIPCMessage.test.ts b/...main/__tests__/handleIPCOperation.test.ts → ...c/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();
-  });
 });