From 0242ba240e16355e27746ca98fd5df65e5241102 Mon Sep 17 00:00:00 2001 From: Kristóf Marussy Date: Mon, 27 Dec 2021 17:41:48 +0100 Subject: fix: Allow the shared store listener to re-register in dev mode This way the shared store will be able to stay connected even if vite HMR replaces the renderer code. --- .../src/contextBridge/SophieRendererImpl.ts | 8 ++-- .../__tests__/SophieRendererImpl.spec.ts | 51 +++++++++++++++------- packages/preload/src/index.ts | 4 +- packages/preload/tsconfig.json | 3 +- 4 files changed, 45 insertions(+), 21 deletions(-) (limited to 'packages') diff --git a/packages/preload/src/contextBridge/SophieRendererImpl.ts b/packages/preload/src/contextBridge/SophieRendererImpl.ts index 18ab07e..4c24b74 100644 --- a/packages/preload/src/contextBridge/SophieRendererImpl.ts +++ b/packages/preload/src/contextBridge/SophieRendererImpl.ts @@ -35,7 +35,7 @@ class SophieRendererImpl implements SophieRenderer { private listener: SharedStoreListener | null = null; - constructor() { + constructor(private readonly allowReplaceListener: boolean) { ipcRenderer.on(MainToRendererIpcMessage.SharedStorePatch, (_event, patch) => { try { // `mobx-state-tree` will validate the patch, so we can safely cast here. @@ -48,7 +48,7 @@ class SophieRendererImpl implements SophieRenderer { } async onSharedStoreChange(listener: SharedStoreListener): Promise { - if (this.onSharedStoreChangeCalled) { + if (this.onSharedStoreChangeCalled && !this.allowReplaceListener) { throw new Error('Shared store change listener was already set'); } this.onSharedStoreChangeCalled = true; @@ -86,8 +86,8 @@ class SophieRendererImpl implements SophieRenderer { } } -export function createSophieRenderer(): SophieRenderer { - const impl = new SophieRendererImpl(); +export function createSophieRenderer(allowReplaceListener: boolean): SophieRenderer { + const impl = new SophieRendererImpl(allowReplaceListener); return { onSharedStoreChange: impl.onSharedStoreChange.bind(impl), dispatchAction: impl.dispatchAction.bind(impl), diff --git a/packages/preload/src/contextBridge/__tests__/SophieRendererImpl.spec.ts b/packages/preload/src/contextBridge/__tests__/SophieRendererImpl.spec.ts index 2f295e6..71ac2a1 100644 --- a/packages/preload/src/contextBridge/__tests__/SophieRendererImpl.spec.ts +++ b/packages/preload/src/contextBridge/__tests__/SophieRendererImpl.spec.ts @@ -60,7 +60,7 @@ const invalidAction = { describe('createSophieRenderer', () => { it('registers a shared store patch listener', () => { - createSophieRenderer(); + createSophieRenderer(false); expect(ipcRenderer.on).toHaveBeenCalledWith( MainToRendererIpcMessage.SharedStorePatch, expect.anything(), @@ -77,26 +77,21 @@ describe('SophieRendererImpl', () => { }; beforeEach(() => { - sut = createSophieRenderer(); + sut = createSophieRenderer(false); onSharedStorePatch = mocked(ipcRenderer.on).mock.calls.find(([channel]) => { return channel === MainToRendererIpcMessage.SharedStorePatch; })?.[1]!; }); describe('onSharedStoreChange', () => { - it('requests a snapshot from the service', async () => { + it('requests a snapshot from the main process', async () => { mocked(ipcRenderer.invoke).mockResolvedValueOnce(snapshot); await sut.onSharedStoreChange(listener); expect(ipcRenderer.invoke).toBeCalledWith(RendererToMainIpcMessage.GetSharedStoreSnapshot); - }); - - it('passes the snapshot to the listener', async () => { - mocked(ipcRenderer.invoke).mockResolvedValueOnce(snapshot); - await sut.onSharedStoreChange(listener); expect(listener.onSnapshot).toBeCalledWith(snapshot); }); - it('catches service errors without exposing them', async () => { + it('catches IPC errors without exposing them', async () => { mocked(ipcRenderer.invoke).mockRejectedValue(new Error('s3cr3t')); await expect(sut.onSharedStoreChange(listener)).rejects.not.toHaveProperty( 'message', @@ -125,7 +120,7 @@ describe('SophieRendererImpl', () => { }); describe('when no listener is registered', () => { - it('discards the received patch', () => { + it('discards the received patch without any error', () => { onSharedStorePatch(event, patch); }); }); @@ -136,8 +131,10 @@ describe('SophieRendererImpl', () => { }); } - function itDoesNotPassPatchesToTheListener() { - it('does not pass patches to the listener', () => { + function itDoesNotPassPatchesToTheListener( + name: string = 'does not pass patches to the listener', + ) { + it(name, () => { onSharedStorePatch(event, patch); expect(listener.onPatch).toBeCalledTimes(0); }); @@ -161,18 +158,18 @@ describe('SophieRendererImpl', () => { itRefusesToRegisterAnotherListener(); - describe('that later threw an error', () => { + describe('after the listener threw in onPatch', () => { beforeEach(() => { mocked(listener.onPatch).mockImplementation(() => { throw new Error(); }); onSharedStorePatch(event, patch); listener.onPatch.mockRestore(); }); - itDoesNotPassPatchesToTheListener(); + itDoesNotPassPatchesToTheListener('does not pass on patches any more'); }); }); - describe('when a listener failed to register due to service error', () => { + describe('when a listener failed to register due to IPC error', () => { beforeEach(async () => { mocked(ipcRenderer.invoke).mockRejectedValue(new Error()); try { @@ -217,4 +214,28 @@ describe('SophieRendererImpl', () => { itDoesNotPassPatchesToTheListener(); }); + + describe('when it is allowed to replace listeners', () => { + const snapshot2 = { + shouldUseDarkColors: false, + } + const listener2 = { + onSnapshot: jest.fn((_snapshot: SharedStoreSnapshotIn) => { }), + onPatch: jest.fn((_patch: IJsonPatch) => { }), + }; + + it('should fetch a second snapshot', async () => { + mocked(ipcRenderer.invoke).mockResolvedValueOnce(snapshot2); + await sut.onSharedStoreChange(listener2); + expect(ipcRenderer.invoke).toBeCalledWith(RendererToMainIpcMessage.GetSharedStoreSnapshot); + expect(listener2.onSnapshot).toBeCalledWith(snapshot2); + }); + + it('should pass the second snapshot to the new listener', async () => { + mocked(ipcRenderer.invoke).mockResolvedValueOnce(snapshot2); + await sut.onSharedStoreChange(listener2); + onSharedStorePatch(event, patch); + expect(listener2.onPatch).toBeCalledWith(patch); + }); + }); }); diff --git a/packages/preload/src/index.ts b/packages/preload/src/index.ts index ef466b4..de91742 100644 --- a/packages/preload/src/index.ts +++ b/packages/preload/src/index.ts @@ -22,6 +22,8 @@ import { contextBridge } from 'electron'; import { createSophieRenderer } from './contextBridge/SophieRendererImpl'; -const sophieRenderer = createSophieRenderer(); +const isDevelopment = import.meta.env.MODE === 'development'; + +const sophieRenderer = createSophieRenderer(isDevelopment); contextBridge.exposeInMainWorld('sophieRenderer', sophieRenderer); diff --git a/packages/preload/tsconfig.json b/packages/preload/tsconfig.json index ab274a1..49f223a 100644 --- a/packages/preload/tsconfig.json +++ b/packages/preload/tsconfig.json @@ -11,7 +11,8 @@ "esnext" ], "types": [ - "@types/jest" + "@types/jest", + "vite/client" ] }, "references": [ -- cgit v1.2.3-70-g09d2