Repository: electron/fiddle
Pull request #653 (2021-04-12 15:30:57)

fix: onbeforeunload triggered for link click
Closes https://github.com/electron/fiddle/issues/649.



erickzhao: CI failures are unrelated, seems to be failing on one of our README links giving a 503 https://img.shields.io/discord/745037351163527189?color=%237289DA&label=chat&logo=discord&logoColor=white

This seems unfortunate.

Patch:
diff --git a/src/renderer/app.tsx b/src/renderer/app.tsx
index 6114e1b5..a3af5f72 100644
--- a/src/renderer/app.tsx
+++ b/src/renderer/app.tsx
@@ -21,8 +21,9 @@ import { EMPTY_EDITOR_CONTENT, SORTED_EDITORS } from './constants';
 import { FileManager } from './file-manager';
 import { RemoteLoader } from './remote-loader';
 import { Runner } from './runner';
+import { AppState } from './state';
+import { getElectronVersions } from './versions';
 import { TaskRunner } from './task-runner';
-import { appState } from './state';
 import { getTheme } from './themes';
 import { defaultDark, defaultLight } from './themes-defaults';
 
@@ -35,10 +36,10 @@ import { defaultDark, defaultLight } from './themes-defaults';
 export class App {
   public typeDefDisposable: MonacoType.IDisposable | null = null;
   public monaco: typeof MonacoType | null = null;
-  public state = appState;
-  public fileManager = new FileManager(appState);
-  public remoteLoader = new RemoteLoader(appState);
-  public runner = new Runner(appState);
+  public state = new AppState(getElectronVersions());
+  public fileManager = new FileManager(this.state);
+  public remoteLoader = new RemoteLoader(this.state);
+  public runner = new Runner(this.state);
   public readonly taskRunner: TaskRunner;
 
   constructor() {
diff --git a/src/renderer/components/output.tsx b/src/renderer/components/output.tsx
index a4d52a32..77369fc7 100644
--- a/src/renderer/components/output.tsx
+++ b/src/renderer/components/output.tsx
@@ -1,3 +1,4 @@
+import { shell } from 'electron';
 import { autorun } from 'mobx';
 import { observer } from 'mobx-react';
 import * as React from 'react';
@@ -113,7 +114,7 @@ export class Output extends React.Component<CommandsProps> {
         <div key={`${entry.timestamp}--${index}--${lineIndex}`}>
           <span style={style} className="output-message">
             {timestamp}
-            <a href={text}>{text}</a>
+            <a onClick={() => shell.openExternal(text)}>{text}</a>
           </span>
         </div>
       ) : (
diff --git a/src/renderer/fetch-types.ts b/src/renderer/fetch-types.ts
index 8f7a8603..95edcec5 100644
--- a/src/renderer/fetch-types.ts
+++ b/src/renderer/fetch-types.ts
@@ -137,8 +137,7 @@ export async function updateEditorTypeDefinitions(
   };
 
   // If this method is called before we're ready, we'll delay this work a bit
-  if (!window.ElectronFiddle.app || !window.ElectronFiddle.app.monaco)
-    return defer();
+  if (!window.ElectronFiddle?.app?.monaco) return defer();
 
   const { app } = window.ElectronFiddle;
   const monaco: typeof MonacoType = app.monaco!;
diff --git a/src/renderer/state.ts b/src/renderer/state.ts
index c0c0dfa1..8be4f9bc 100644
--- a/src/renderer/state.ts
+++ b/src/renderer/state.ts
@@ -53,9 +53,6 @@ import {
   saveLocalVersions,
 } from './versions';
 
-const knownVersions = getElectronVersions();
-const defaultVersion = getDefaultVersion(knownVersions);
-
 /**
  * The application's state. Exported as a singleton below.
  *
@@ -64,7 +61,6 @@ const defaultVersion = getDefaultVersion(knownVersions);
  */
 export class AppState {
   // -- Persisted settings ------------------
-  @observable public version: string = defaultVersion;
   @observable public theme: string | null = localStorage.getItem('theme');
   @observable public gitHubAvatarUrl: string | null = localStorage.getItem(
     'gitHubAvatarUrl',
@@ -116,10 +112,8 @@ export class AppState {
     (this.retrieve('acceleratorsToBlock') as Array<BlockableAccelerator>) || [];
   // -- Various session-only state ------------------
   @observable public gistId: string | undefined;
-  @observable public versions: Record<
-    string,
-    RunnableVersion
-  > = Object.fromEntries(knownVersions.map((ver) => [ver.version, ver]));
+  @observable public versions: Record<string, RunnableVersion>;
+  @observable public version: string;
   @observable public output: Array<OutputEntry> = [];
   @observable public localPath: string | undefined;
   @observable public genericDialogOptions: GenericDialogOptions = {
@@ -165,9 +159,10 @@ export class AppState {
 
   private outputBuffer = '';
   private name: string;
+  private readonly defaultVersion: string;
   public appData: string;
 
-  constructor() {
+  constructor(versions: RunnableVersion[]) {
     // Bind all actions
     this.downloadVersion = this.downloadVersion.bind(this);
     this.pushError = this.pushError.bind(this);
@@ -192,10 +187,16 @@ export class AppState {
     this.hideChannels = this.hideChannels.bind(this);
     this.showChannels = this.showChannels.bind(this);
 
+    // init fields
+    this.versions = Object.fromEntries(versions.map((v) => [v.version, v]));
+    this.defaultVersion = getDefaultVersion(versions);
+    this.version = this.defaultVersion;
+
+    ipcRendererManager.removeAllListeners(IpcEvents.BEFORE_QUIT);
+    ipcRendererManager.removeAllListeners(IpcEvents.BISECT_COMMANDS_TOGGLE);
+    ipcRendererManager.removeAllListeners(IpcEvents.CLEAR_CONSOLE);
     ipcRendererManager.removeAllListeners(IpcEvents.OPEN_SETTINGS);
     ipcRendererManager.removeAllListeners(IpcEvents.SHOW_WELCOME_TOUR);
-    ipcRendererManager.removeAllListeners(IpcEvents.CLEAR_CONSOLE);
-    ipcRendererManager.removeAllListeners(IpcEvents.BISECT_COMMANDS_TOGGLE);
 
     ipcRendererManager.on(IpcEvents.OPEN_SETTINGS, this.toggleSettings);
     ipcRendererManager.on(IpcEvents.SHOW_WELCOME_TOUR, this.showTour);
@@ -291,6 +292,8 @@ export class AppState {
     ipcRendererManager.send(IpcEvents.BLOCK_ACCELERATORS, [
       ...this.acceleratorsToBlock,
     ]);
+
+    this.setVersion(this.version);
   }
 
   /**
@@ -325,11 +328,7 @@ export class AppState {
    * one that can be found.
    */
   @computed get currentElectronVersion(): RunnableVersion {
-    if (this.versions[this.version]) {
-      return this.versions[this.version];
-    } else {
-      return this.versions[defaultVersion];
-    }
+    return this.versions[this.version] || this.versions[this.defaultVersion];
   }
 
   /**
@@ -561,10 +560,8 @@ export class AppState {
     const version = normalizeVersion(input);
 
     if (!this.hasVersion(input)) {
-      console.warn(
-        `State: Called setVersion() with ${version}, which does not exist.`,
-      );
-      await this.setVersion(knownVersions[0].version);
+      console.warn(`State: setVersion() got an unknown version ${version}`);
+      await this.setVersion(this.versionsToShow[0].version);
       return;
     }
 
@@ -874,6 +871,3 @@ export class AppState {
     return JSON.parse(value || 'null') as T;
   }
 }
-
-export const appState = new AppState();
-appState.setVersion(appState.version);


Test patch:
diff --git a/tests/renderer/app-spec.tsx b/tests/renderer/app-spec.tsx
index 376d8ac0..c8d0e4c3 100644
--- a/tests/renderer/app-spec.tsx
+++ b/tests/renderer/app-spec.tsx
@@ -10,17 +10,6 @@ import { defaultDark, defaultLight } from '../../src/renderer/themes-defaults';
 jest.mock('../../src/renderer/file-manager', () =>
   require('../mocks/file-manager'),
 );
-jest.mock('../../src/renderer/state', () => ({
-  appState: {
-    closedPanels: {},
-    getName: () => 'Test',
-    hideChannels: jest.fn(),
-    pushOutput: jest.fn(),
-    setVersion: jest.fn(),
-    showChannels: jest.fn(),
-    theme: 'defaultDark',
-  },
-}));
 jest.mock('../../src/renderer/components/header', () => ({
   Header: () => 'Header;',
 }));
diff --git a/tests/renderer/state-spec.ts b/tests/renderer/state-spec.ts
index fd98c789..e41fa603 100644
--- a/tests/renderer/state-spec.ts
+++ b/tests/renderer/state-spec.ts
@@ -8,6 +8,7 @@ import {
   GenericDialogType,
   PanelId,
   RunnableVersion,
+  Version,
   VersionSource,
   VersionState,
 } from '../../src/interfaces';
@@ -25,6 +26,7 @@ import {
 import { getTemplate, isContentUnchanged } from '../../src/renderer/content';
 import { ipcRendererManager } from '../../src/renderer/ipc';
 import { AppState } from '../../src/renderer/state';
+import { getElectronVersions } from '../../src/renderer/versions';
 import {
   getUpdatedElectronVersions,
   saveLocalVersions,
@@ -56,7 +58,7 @@ jest.mock('../../src/renderer/versions', () => {
 
   return {
     getUpdatedElectronVersions: jest.fn().mockResolvedValue(mockVersionsArray),
-    getElectronVersions: () => mockVersionsArray,
+    getElectronVersions: jest.fn(),
     getDefaultVersion: () => '2.0.2',
     addLocalVersion: jest.fn(),
     saveLocalVersions: jest.fn(),
@@ -81,8 +83,7 @@ describe('AppState', () => {
     );
     (getVersionState as jest.Mock).mockImplementation((v) => v.state);
 
-    appState = new AppState();
-    appState.versions = mockVersions;
+    appState = new AppState(mockVersionsArray);
     appState.updateVersionStates = jest.fn();
 
     ipcRendererManager.removeAllListeners();
@@ -518,16 +519,19 @@ describe('AppState', () => {
     it('refreshes version state', async () => {
       appState.versions = {};
 
-      await appState.addLocalVersion({
+      const version = '4.0.0';
+      const ver: Version = {
         localPath: '/fake/path',
         name: 'local-foo',
-        version: '4.0.0',
-      });
+        version,
+      };
+
+      (getElectronVersions as jest.Mock).mockReturnValue([ver]);
+
+      await appState.addLocalVersion(ver);
 
-      // We just want to verify that the version state was
-      // refreshed - we didn't actually add the local version
-      // above, since versions.ts is mocked
-      expect(Object.keys(appState.versions)).toEqual(Object.keys(mockVersions));
+      expect(getElectronVersions).toHaveBeenCalledTimes(1);
+      expect(appState.getVersion(version)).toStrictEqual(ver);
     });
   });
 
@@ -541,7 +545,7 @@ describe('AppState', () => {
           return VersionState.unknown;
         },
       );
-      appState = new AppState();
+      appState = new AppState(mockVersionsArray);
     });
 
     it('downloads a version if necessary', async () => {