From c62387c53e357f338c315a4e9d3905962fa4873a Mon Sep 17 00:00:00 2001 From: Samuel Maddock Date: Fri, 15 Nov 2024 14:51:02 -0500 Subject: [PATCH 1/8] feat: add register-local-version protocol command --- src/ambient.d.ts | 8 ++++++++ src/interfaces.ts | 2 ++ src/ipc-events.ts | 1 + src/main/dialogs.ts | 11 +---------- src/main/protocol.ts | 33 +++++++++++++++++++++++++++++++++ src/main/utils/local-version.ts | 11 +++++++++++ src/preload/preload.ts | 1 + src/renderer/app.tsx | 18 ++++++++++++++++++ 8 files changed, 75 insertions(+), 10 deletions(-) create mode 100644 src/main/utils/local-version.ts diff --git a/src/ambient.d.ts b/src/ambient.d.ts index 6de037e74f..4f0ff98d37 100644 --- a/src/ambient.d.ts +++ b/src/ambient.d.ts @@ -74,6 +74,14 @@ declare global { type: 'open-template', listener: (name: string, editorValues: EditorValues) => void, ): void; + addEventListener( + type: 'register-local-version', + listener: (info: { + name: string; + path: string; + version: string; + }) => void, + ): void; addEventListener( type: 'saved-local-fiddle', listener: (filePath: string) => void, diff --git a/src/interfaces.ts b/src/interfaces.ts index 51f73fde5a..8694c13050 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -15,6 +15,7 @@ export type FileTransformOperation = 'dotfiles' | 'forge'; export enum VersionSource { remote = 'remote', local = 'local', + pullRequest = 'pull-request', } export enum GistActionType { @@ -186,6 +187,7 @@ export type FiddleEvent = | 'open-template' | 'package-fiddle' | 'redo-in-editor' + | 'register-local-version' | 'run-fiddle' | 'save-fiddle-gist' | 'saved-local-fiddle' diff --git a/src/ipc-events.ts b/src/ipc-events.ts index 1b90a15cfe..6a3e18024b 100644 --- a/src/ipc-events.ts +++ b/src/ipc-events.ts @@ -17,6 +17,7 @@ export enum IpcEvents { SHOW_WELCOME_TOUR = 'SHOW_WELCOME_TOUR', CLEAR_CONSOLE = 'CLEAR_CONSOLE', LOAD_LOCAL_VERSION_FOLDER = 'LOAD_LOCAL_VERSION_FOLDER', + REGISTER_LOCAL_VERSION_FOLDER = 'REGISTER_LOCAL_VERSION_FOLDER', BISECT_COMMANDS_TOGGLE = 'BISECT_COMMANDS_TOGGLE', BEFORE_QUIT = 'BEFORE_QUIT', CONFIRM_QUIT = 'CONFIRM_QUIT', diff --git a/src/main/dialogs.ts b/src/main/dialogs.ts index f999a1a09b..65655f79e2 100644 --- a/src/main/dialogs.ts +++ b/src/main/dialogs.ts @@ -1,10 +1,9 @@ import * as path from 'node:path'; -import { Installer } from '@electron/fiddle-core'; import { BrowserWindow, dialog } from 'electron'; -import * as fs from 'fs-extra'; import { ipcMainManager } from './ipc'; +import { isValidElectronPath } from './utils/local-version'; import { SelectedLocalVersion } from '../interfaces'; import { IpcEvents } from '../ipc-events'; @@ -31,14 +30,6 @@ function makeLocalName(folderPath: string): string { return `${leader} - ${buildType}`; } -/** - * Verifies if the local electron path is valid - */ -function isValidElectronPath(folderPath: string): boolean { - const execPath = Installer.getExecPath(folderPath); - return fs.existsSync(execPath); -} - /** * Listens to IPC events related to dialogs and message boxes */ diff --git a/src/main/protocol.ts b/src/main/protocol.ts index 3cc1d7d7e1..b8c3627b7a 100644 --- a/src/main/protocol.ts +++ b/src/main/protocol.ts @@ -6,6 +6,7 @@ import { app } from 'electron'; import { openFiddle } from './files'; import { ipcMainManager } from './ipc'; import { isDevMode } from './utils/devmode'; +import { isValidElectronPath } from './utils/local-version'; import { getOrCreateMainWindow } from './windows'; import { IpcEvents } from '../ipc-events'; @@ -65,6 +66,38 @@ const handlePotentialProtocolLaunch = (url: string) => { return; } break; + // electron-fiddle://register-local-version/?name={name}&path={path}&version={version} + case 'register-local-version': { + if (pathParts.length === 1) { + const name = parsed.searchParams.get('name'); + const localPath = parsed.searchParams.get('path'); + const version = parsed.searchParams.get('version'); + + if (!(name && localPath && version)) { + console.debug('register-local-version: Missing params'); + return; + } + + if (!isValidElectronPath(localPath)) { + console.debug(`register-local-version: Invalid path ${localPath}`); + return; + } + + const toAdd = { + name, + path: localPath, + version, + }; + + console.debug('register-local-version: Registering version', toAdd); + + ipcMainManager.send(IpcEvents.REGISTER_LOCAL_VERSION_FOLDER, [toAdd]); + } else { + // Invalid + return; + } + break; + } default: return; } diff --git a/src/main/utils/local-version.ts b/src/main/utils/local-version.ts new file mode 100644 index 0000000000..82c9b1ae0b --- /dev/null +++ b/src/main/utils/local-version.ts @@ -0,0 +1,11 @@ +import * as fs from 'fs'; + +import { Installer } from '@electron/fiddle-core'; + +/** + * Verifies if the local electron path is valid + */ +export function isValidElectronPath(folderPath: string): boolean { + const execPath = Installer.getExecPath(folderPath); + return fs.existsSync(execPath); +} diff --git a/src/preload/preload.ts b/src/preload/preload.ts index 3684239b7f..55f3ae926d 100644 --- a/src/preload/preload.ts +++ b/src/preload/preload.ts @@ -36,6 +36,7 @@ const channelMapping: Record = { 'open-settings': IpcEvents.OPEN_SETTINGS, 'open-template': IpcEvents.FS_OPEN_TEMPLATE, 'package-fiddle': IpcEvents.FIDDLE_PACKAGE, + 'register-local-version': IpcEvents.REGISTER_LOCAL_VERSION_FOLDER, 'redo-in-editor': IpcEvents.REDO_IN_EDITOR, 'run-fiddle': IpcEvents.FIDDLE_RUN, 'saved-local-fiddle': IpcEvents.SAVED_LOCAL_FIDDLE, diff --git a/src/renderer/app.tsx b/src/renderer/app.tsx index b443913cae..9952a0f486 100644 --- a/src/renderer/app.tsx +++ b/src/renderer/app.tsx @@ -15,6 +15,7 @@ import { PACKAGE_NAME, PackageJsonOptions, SetFiddleOptions, + Version, } from '../interfaces'; import { defaultDark, defaultLight } from '../themes-defaults'; @@ -137,6 +138,7 @@ export class App { this.setupTitleListeners(); this.setupUnloadListeners(); this.setupTypeListeners(); + this.setupProtocolListeners(); window.ElectronFiddle.sendReady(); @@ -310,4 +312,20 @@ export class App { }; }); } + + public setupProtocolListeners() { + window.ElectronFiddle.addEventListener( + 'register-local-version', + ({ name, path, version }) => { + const toAdd: Version = { + localPath: path, + version, + name, + }; + + this.state.addLocalVersion(toAdd); + this.state.setVersion(version); + }, + ); + } } From b838558f792b934eeea29541aefe5a323a135fb5 Mon Sep 17 00:00:00 2001 From: Samuel Maddock Date: Sun, 17 Nov 2024 00:20:26 -0500 Subject: [PATCH 2/8] replace existing local version --- src/renderer/versions.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/renderer/versions.ts b/src/renderer/versions.ts index 74c45cb000..57550ae7f6 100644 --- a/src/renderer/versions.ts +++ b/src/renderer/versions.ts @@ -74,10 +74,16 @@ export function getElectronVersions(): Array { export function addLocalVersion(input: Version): Array { const versions = getLocalVersions(); - if (!versions.find((v) => v.localPath === input.localPath)) { - versions.push(input); + // Replace existing local version if it exists + const existingVersionIndex = versions.findIndex( + (v) => v.localPath === input.localPath, + ); + if (existingVersionIndex > -1) { + versions.splice(existingVersionIndex, 1); } + versions.push(input); + saveLocalVersions(versions); return versions; From 6aa8c6990b0aa90a4e1f31e434143807454899ad Mon Sep 17 00:00:00 2001 From: Samuel Maddock Date: Tue, 19 Nov 2024 17:27:55 -0500 Subject: [PATCH 3/8] test: register-local-version protocol command --- tests/main/protocol-spec.ts | 64 +++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tests/main/protocol-spec.ts b/tests/main/protocol-spec.ts index c054691b7e..f452d6b4d2 100644 --- a/tests/main/protocol-spec.ts +++ b/tests/main/protocol-spec.ts @@ -3,6 +3,7 @@ */ import * as fs from 'node:fs'; +import * as path from 'node:path'; import { app } from 'electron'; import { mocked } from 'jest-mock'; @@ -13,11 +14,16 @@ import { listenForProtocolHandler, setupProtocolHandler, } from '../../src/main/protocol'; +import { isValidElectronPath } from '../../src/main/utils/local-version'; import { getOrCreateMainWindow, mainIsReady } from '../../src/main/windows'; import { overridePlatform, resetPlatform } from '../utils'; jest.mock('node:fs'); +jest.mock('../../src/main/utils/local-version', () => ({ + isValidElectronPath: jest.fn(), +})); + describe('protocol', () => { const oldArgv = [...process.argv]; @@ -192,5 +198,63 @@ describe('protocol', () => { await new Promise(process.nextTick); expect(mainWindow.focus).toHaveBeenCalled(); }); + + it('handles registering local version url', () => { + mocked(isValidElectronPath).mockReturnValueOnce(true); + + overridePlatform('darwin'); + listenForProtocolHandler(); + + const handler = mocked(app.on).mock.calls[0][1]; + const url = new URL('electron-fiddle://register-local-version/'); + const params = { + name: 'test', + path: path.resolve(__dirname), + version: '35.0.0-local', + }; + const keys = Object.keys(params) as Array; + keys.forEach((k) => { + url.searchParams.append(k, params[k]); + }); + handler({}, url.href); + + expect(ipcMainManager.send).toHaveBeenCalledWith( + IpcEvents.REGISTER_LOCAL_VERSION_FOLDER, + [params], + ); + }); + + it('handles registering local version without valid path', () => { + mocked(isValidElectronPath).mockReturnValueOnce(false); + + overridePlatform('darwin'); + listenForProtocolHandler(); + + const handler = mocked(app.on).mock.calls[0][1]; + const url = new URL('electron-fiddle://register-local-version/'); + const params = { + name: 'test', + path: path.resolve(__dirname), + version: '35.0.0-local', + }; + const keys = Object.keys(params) as Array; + keys.forEach((k) => { + url.searchParams.append(k, params[k]); + }); + handler({}, url.href); + + expect(ipcMainManager.send).not.toHaveBeenCalled(); + }); + + it('handles registering local version with missing params', () => { + overridePlatform('darwin'); + listenForProtocolHandler(); + + const handler = mocked(app.on).mock.calls[0][1]; + const url = new URL('electron-fiddle://register-local-version/'); + handler({}, url.href); + + expect(ipcMainManager.send).not.toHaveBeenCalled(); + }); }); }); From 96380717dd4530f6a673264b4c7ea87da667e7a4 Mon Sep 17 00:00:00 2001 From: Samuel Maddock Date: Tue, 19 Nov 2024 17:49:39 -0500 Subject: [PATCH 4/8] test: app protocol handler --- tests/renderer/app-spec.tsx | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/renderer/app-spec.tsx b/tests/renderer/app-spec.tsx index b0d5e473db..1c2895c2d2 100644 --- a/tests/renderer/app-spec.tsx +++ b/tests/renderer/app-spec.tsx @@ -379,6 +379,36 @@ describe('App component', () => { }); }); + describe('setupProtocolListeners()', () => { + it('handles registering new versions', () => { + const addEventListenerMock = window.ElectronFiddle + .addEventListener as any; + addEventListenerMock.mockClear(); + + app.setupProtocolListeners(); + + expect(addEventListenerMock).toHaveBeenCalledWith( + 'register-local-version', + expect.anything(), + ); + + const callback = addEventListenerMock.mock.calls[0][1]; + const addVersion = { + name: 'new-version', + path: '/version/build/path', + version: '123.0.0-local', + }; + callback(addVersion); + + expect(app.state.addLocalVersion).toHaveBeenCalledWith({ + name: addVersion.name, + localPath: addVersion.path, + version: addVersion.version, + }); + expect(app.state.setVersion).toHaveBeenCalledWith(addVersion.version); + }); + }); + describe('prompting to confirm replacing an unsaved fiddle', () => { // make a second fiddle that differs from the first const editorValues = createEditorValues(); From 57a762542105579281a5bffe3816df4a775150f6 Mon Sep 17 00:00:00 2001 From: Sam Maddock Date: Thu, 21 Nov 2024 20:27:31 -0500 Subject: [PATCH 5/8] Update src/main/utils/local-version.ts Co-authored-by: Erick Zhao --- src/main/utils/local-version.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/utils/local-version.ts b/src/main/utils/local-version.ts index 82c9b1ae0b..64850c396f 100644 --- a/src/main/utils/local-version.ts +++ b/src/main/utils/local-version.ts @@ -1,4 +1,4 @@ -import * as fs from 'fs'; +import * as fs from 'node:fs'; import { Installer } from '@electron/fiddle-core'; From a7fcf2c554813a77ff9946daf06f1104dcda952c Mon Sep 17 00:00:00 2001 From: Samuel Maddock Date: Fri, 22 Nov 2024 18:42:05 -0500 Subject: [PATCH 6/8] feat: add confirmation dialog for registering local versions --- src/renderer/app.tsx | 8 +++++++- tests/renderer/app-spec.tsx | 33 +++++++++++++++++++++++++++++---- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/renderer/app.tsx b/src/renderer/app.tsx index 9952a0f486..6e2d103d69 100644 --- a/src/renderer/app.tsx +++ b/src/renderer/app.tsx @@ -316,7 +316,13 @@ export class App { public setupProtocolListeners() { window.ElectronFiddle.addEventListener( 'register-local-version', - ({ name, path, version }) => { + async ({ name, path, version }) => { + const confirm = await this.state.showConfirmDialog({ + label: `Are you sure you want to register "${path}" with version "${version}"? Only register and run it if you trust the source.`, + ok: 'Register', + }); + if (!confirm) return; + const toAdd: Version = { localPath: path, version, diff --git a/tests/renderer/app-spec.tsx b/tests/renderer/app-spec.tsx index 1c2895c2d2..909185379a 100644 --- a/tests/renderer/app-spec.tsx +++ b/tests/renderer/app-spec.tsx @@ -380,25 +380,34 @@ describe('App component', () => { }); describe('setupProtocolListeners()', () => { - it('handles registering new versions', () => { - const addEventListenerMock = window.ElectronFiddle - .addEventListener as any; + let addEventListenerMock: jest.Mock; + + beforeEach(() => { + addEventListenerMock = window.ElectronFiddle.addEventListener as any; addEventListenerMock.mockClear(); + }); + it('registers protocol listeners', () => { app.setupProtocolListeners(); expect(addEventListenerMock).toHaveBeenCalledWith( 'register-local-version', expect.anything(), ); + }); + it('handles registering new versions', async () => { + app.setupProtocolListeners(); const callback = addEventListenerMock.mock.calls[0][1]; + + app.state.showConfirmDialog = jest.fn().mockResolvedValue(true); + const addVersion = { name: 'new-version', path: '/version/build/path', version: '123.0.0-local', }; - callback(addVersion); + await callback(addVersion); expect(app.state.addLocalVersion).toHaveBeenCalledWith({ name: addVersion.name, @@ -407,6 +416,22 @@ describe('App component', () => { }); expect(app.state.setVersion).toHaveBeenCalledWith(addVersion.version); }); + + it('skips registering new versions when not confirmed', async () => { + app.setupProtocolListeners(); + const callback = addEventListenerMock.mock.calls[0][1]; + + app.state.showConfirmDialog = jest.fn().mockResolvedValue(false); + + const addVersion = { + name: 'new-version', + path: '/version/build/path', + version: '123.0.0-local', + }; + await callback(addVersion); + + expect(app.state.addLocalVersion).not.toHaveBeenCalled(); + }); }); describe('prompting to confirm replacing an unsaved fiddle', () => { From da4be6592fafe7c5a72c3fcd1d24bedcb9d8ac20 Mon Sep 17 00:00:00 2001 From: Samuel Maddock Date: Fri, 22 Nov 2024 19:10:10 -0500 Subject: [PATCH 7/8] fix: dialog clobbered on multiple requests When multiple dialogs are requested to be shown, they now wait for the previous dialog to be closed and cleaned up. --- src/renderer/state.ts | 31 ++++++++++++++++++++--- tests/renderer/state-spec.ts | 49 +++++++++++++++++++++++++++++++++++- 2 files changed, 75 insertions(+), 5 deletions(-) diff --git a/src/renderer/state.ts b/src/renderer/state.ts index da3bc1996f..96044a5c68 100644 --- a/src/renderer/state.ts +++ b/src/renderer/state.ts @@ -4,6 +4,7 @@ import { computed, makeObservable, observable, + runInAction, when, } from 'mobx'; @@ -975,12 +976,34 @@ export class AppState { public async showGenericDialog( opts: GenericDialogOptions, ): Promise<{ confirm: boolean; input: string }> { - this.genericDialogLastResult = null; - this.genericDialogOptions = opts; - this.isGenericDialogShowing = true; + // Wait for any existing dialog to be closed and cleaned up. + await when(() => { + if ( + !this.isGenericDialogShowing && + this.genericDialogLastResult === null + ) { + // Set dialog immediately to prevent any other queued dialogs from + // showing. + runInAction(() => { + this.genericDialogOptions = opts; + this.isGenericDialogShowing = true; + }); + return true; + } + return false; + }); + + // Wait for dialog to be closed. await when(() => !this.isGenericDialogShowing); + + // Cleanup to allow queued dialog to show. + const result = this.genericDialogLastResult; + runInAction(() => { + this.genericDialogLastResult = null; + }); + return { - confirm: Boolean(this.genericDialogLastResult), + confirm: Boolean(result), input: this.genericDialogLastInput || opts.defaultInput || '', }; } diff --git a/tests/renderer/state-spec.ts b/tests/renderer/state-spec.ts index d9856900b3..e2b18630d6 100644 --- a/tests/renderer/state-spec.ts +++ b/tests/renderer/state-spec.ts @@ -1,5 +1,5 @@ import { mocked } from 'jest-mock'; -import { IReactionDisposer, reaction } from 'mobx'; +import { IReactionDisposer, reaction, runInAction, when } from 'mobx'; import { AppStateBroadcastMessageType, @@ -585,6 +585,53 @@ describe('AppState', () => { const result = await appState.showGenericDialog(Opts); expect(result).toHaveProperty('input', ''); }); + + it('queues dialogs', async () => { + const optsA = { + label: 'A', + ok: 'Close', + type: GenericDialogType.warning, + wantsInput: false, + } as const; + + const optsB = { + label: 'B', + ok: 'Close', + type: GenericDialogType.warning, + wantsInput: false, + } as const; + + // Show Dialog A + const dialogPromiseA = appState.showGenericDialog(optsA); + await when(() => appState.genericDialogOptions?.label === optsA.label); + + // Queue Dialog B + const dialogPromiseB = appState.showGenericDialog(optsB); + expect(appState).toHaveProperty('isGenericDialogShowing', true); + expect(appState.genericDialogOptions?.label).toEqual(optsA.label); + + // Close Dialog A + runInAction(() => { + appState.genericDialogLastResult = true; + appState.isGenericDialogShowing = false; + }); + expect(appState.genericDialogOptions?.label).toEqual(optsA.label); + await dialogPromiseA; + + // Expect dialog set to Dialog B + expect(appState).toHaveProperty('isGenericDialogShowing', true); + expect(appState.genericDialogOptions?.label).toEqual(optsB.label); + + // Close Dialog B + runInAction(() => { + appState.genericDialogLastResult = true; + appState.isGenericDialogShowing = false; + }); + await dialogPromiseB; + + // No remaining dialogs queued + expect(appState).toHaveProperty('isGenericDialogShowing', false); + }); }); describe('showInputDialog', () => { From ec0aa344ed113afa408034d545a8961a5d9c37db Mon Sep 17 00:00:00 2001 From: Samuel Maddock Date: Fri, 22 Nov 2024 19:43:51 -0500 Subject: [PATCH 8/8] also clear genericDialogLastInput --- src/renderer/state.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/renderer/state.ts b/src/renderer/state.ts index 96044a5c68..ac58705b48 100644 --- a/src/renderer/state.ts +++ b/src/renderer/state.ts @@ -996,16 +996,16 @@ export class AppState { // Wait for dialog to be closed. await when(() => !this.isGenericDialogShowing); + const confirm = Boolean(this.genericDialogLastResult); + const input = this.genericDialogLastInput || opts.defaultInput || ''; + // Cleanup to allow queued dialog to show. - const result = this.genericDialogLastResult; runInAction(() => { this.genericDialogLastResult = null; + this.genericDialogLastInput = null; }); - return { - confirm: Boolean(result), - input: this.genericDialogLastInput || opts.defaultInput || '', - }; + return { confirm, input }; } public async showInputDialog(opts: {