Repository: electron/fiddle
Pull request #621 (2021-03-26 16:25:41)

refactor: avoid hardcoding filenames, editor ids
This is a minor refactor to avoid hardcoding files and editor ids. Most cases can iterate through the `FILENAME_KEYS` object which lets the code avoid hardcoding files / editor is _and_ avoid making assumptions on what editors exist.

I started this cleanup while we were prototyping a new 'test.js' editor. It's less relevant since we discarded that idea, but it's still preferable to use symbolic names instead of magic strings.

No functional changes.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/38288411/badge)](https://coveralls.io/builds/38288411)

Coverage remained the same at 93.308% when pulling **1ae9814f3d437157a81a0d76248de9bda3944ff0 on refactor/avoid-hardcoding-filenames** into **34017cd4be99cf3f26ea2b644d0c9d16c5f266ed on master**.


Patch:
diff --git a/src/main/files.ts b/src/main/files.ts
index 2afe8c67..3a146c17 100644
--- a/src/main/files.ts
+++ b/src/main/files.ts
@@ -3,13 +3,7 @@ import * as fs from 'fs-extra';
 import * as path from 'path';
 
 import { IpcEvents } from '../ipc-events';
-import {
-  INDEX_HTML_NAME,
-  MAIN_JS_NAME,
-  PACKAGE_NAME,
-  PRELOAD_JS_NAME,
-  RENDERER_JS_NAME,
-} from '../shared-constants';
+import { FILENAME_KEYS } from '../shared-constants';
 import { ipcMainManager } from './ipc';
 
 /**
@@ -69,13 +63,9 @@ export async function showSaveDialog(event?: IpcEvents, as?: string) {
  * @returns {Promise<boolean>}
  */
 async function ensureSaveTargetEmpty(filePath: string): Promise<boolean> {
-  const targetPaths = [
-    path.join(filePath, INDEX_HTML_NAME),
-    path.join(filePath, RENDERER_JS_NAME),
-    path.join(filePath, MAIN_JS_NAME),
-    path.join(filePath, PACKAGE_NAME),
-    path.join(filePath, PRELOAD_JS_NAME),
-  ];
+  const targetPaths = Object.keys(FILENAME_KEYS).map((filename) =>
+    path.join(filePath, filename),
+  );
 
   let noFilesOrOverwriteGranted = true;
 
diff --git a/src/renderer/components/commands-action-button.tsx b/src/renderer/components/commands-action-button.tsx
index a2d468ac..ac91ab57 100644
--- a/src/renderer/components/commands-action-button.tsx
+++ b/src/renderer/components/commands-action-button.tsx
@@ -10,6 +10,7 @@ import {
 } from '@blueprintjs/core';
 import { observer } from 'mobx-react';
 import * as React from 'react';
+import * as path from 'path';
 
 import { when } from 'mobx';
 import {
@@ -19,13 +20,7 @@ import {
   GistActionType,
 } from '../../interfaces';
 import { IpcEvents } from '../../ipc-events';
-import {
-  INDEX_HTML_NAME,
-  MAIN_JS_NAME,
-  PRELOAD_JS_NAME,
-  RENDERER_JS_NAME,
-  STYLES_CSS_NAME,
-} from '../../shared-constants';
+import { FILENAME_KEYS } from '../../shared-constants';
 import { getOctokit } from '../../utils/octokit';
 import { EMPTY_EDITOR_CONTENT } from '../constants';
 import { ipcRendererManager } from '../ipc';
@@ -434,22 +429,13 @@ export class GistActionButton extends React.Component<
   };
 
   private gistFilesList = (values: EditorValues) => {
-    return {
-      [INDEX_HTML_NAME]: {
-        content: values.html || EMPTY_EDITOR_CONTENT.html,
-      },
-      [MAIN_JS_NAME]: {
-        content: values.main || EMPTY_EDITOR_CONTENT.js,
-      },
-      [RENDERER_JS_NAME]: {
-        content: values.renderer || EMPTY_EDITOR_CONTENT.js,
-      },
-      [PRELOAD_JS_NAME]: {
-        content: values.preload || EMPTY_EDITOR_CONTENT.js,
-      },
-      [STYLES_CSS_NAME]: {
-        content: values.css || EMPTY_EDITOR_CONTENT.css,
-      },
-    };
+    const getSuffix = (name: string) => path.parse(name).ext.slice(1);
+    const filesList = {};
+    for (const [filename, editorId] of Object.entries(FILENAME_KEYS)) {
+      filesList[filename] = {
+        content: values[editorId] || EMPTY_EDITOR_CONTENT[getSuffix(filename)],
+      };
+    }
+    return filesList;
   };
 }
diff --git a/src/renderer/components/commands-bisect.tsx b/src/renderer/components/commands-bisect.tsx
index fc748298..f02c9aef 100644
--- a/src/renderer/components/commands-bisect.tsx
+++ b/src/renderer/components/commands-bisect.tsx
@@ -87,6 +87,7 @@ export class BisectHandler extends React.Component<BisectHandlerProps> {
         <Button
           icon="git-branch"
           text="Bisect"
+          disabled={appState.isAutoBisecting}
           onClick={appState.toggleBisectDialog}
         />
       );
diff --git a/src/renderer/components/commands-version-chooser.tsx b/src/renderer/components/commands-version-chooser.tsx
index f485a8d3..27d11f4c 100644
--- a/src/renderer/components/commands-version-chooser.tsx
+++ b/src/renderer/components/commands-version-chooser.tsx
@@ -14,7 +14,12 @@ export interface VersionChooserProps {
  * download is managed in the state.
  */
 export const VersionChooser = observer((props: VersionChooserProps) => {
-  const { currentElectronVersion, Bisector, setVersion } = props.appState;
+  const {
+    Bisector,
+    currentElectronVersion,
+    isAutoBisecting,
+    setVersion,
+  } = props.appState;
 
   return (
     <ButtonGroup>
@@ -22,7 +27,7 @@ export const VersionChooser = observer((props: VersionChooserProps) => {
         appState={props.appState}
         onVersionSelect={({ version }) => setVersion(version)}
         currentVersion={currentElectronVersion}
-        disabled={!!Bisector}
+        disabled={!!Bisector || isAutoBisecting}
       />
     </ButtonGroup>
   );
diff --git a/src/renderer/components/dialog-bisect.tsx b/src/renderer/components/dialog-bisect.tsx
index a98ddf5d..12647cc4 100644
--- a/src/renderer/components/dialog-bisect.tsx
+++ b/src/renderer/components/dialog-bisect.tsx
@@ -33,6 +33,7 @@ export class BisectDialog extends React.Component<
     super(props);
 
     this.onSubmit = this.onSubmit.bind(this);
+    this.onAuto = this.onAuto.bind(this);
     this.onClose = this.onClose.bind(this);
     this.onBeginSelect = this.onBeginSelect.bind(this);
     this.onEndSelect = this.onEndSelect.bind(this);
@@ -55,25 +56,35 @@ export class BisectDialog extends React.Component<
     this.setState({ endIndex: this.state.allVersions.indexOf(version) });
   }
 
+  getBisectRange(): RunnableVersion[] {
+    const { endIndex, startIndex, allVersions } = this.state;
+    return endIndex !== undefined && startIndex !== undefined
+      ? allVersions.slice(endIndex, startIndex + 1).reverse()
+      : [];
+  }
+
   /**
    * Handles the submission of the dialog
    *
    * @returns {Promise<void>}
    */
   public async onSubmit(): Promise<void> {
-    const { endIndex, startIndex, allVersions } = this.state;
-    const { appState } = this.props;
-
-    if (endIndex === undefined || startIndex === undefined) {
-      return;
+    const range = this.getBisectRange();
+    if (range.length > 1) {
+      const { appState } = this.props;
+      appState.Bisector = new Bisector(range);
+      const initialBisectPivot = appState.Bisector.getCurrentVersion().version;
+      appState.setVersion(initialBisectPivot);
+      this.onClose();
     }
+  }
 
-    const bisectRange = allVersions.slice(endIndex, startIndex + 1).reverse();
-
-    appState.Bisector = new Bisector(bisectRange);
-    const initialBisectPivot = appState.Bisector.getCurrentVersion().version;
-    appState.setVersion(initialBisectPivot);
-    this.onClose();
+  public async onAuto(): Promise<void> {
+    const range = this.getBisectRange();
+    if (range.length > 1) {
+      window.ElectronFiddle.app.runner.autobisect(range);
+      this.onClose();
+    }
   }
 
   /**
@@ -97,6 +108,13 @@ export class BisectDialog extends React.Component<
     return this.state.startIndex > this.state.endIndex;
   }
 
+  /**
+   * Can we autobisect?
+   */
+  get canAuto(): boolean {
+    return this.canSubmit;
+  }
+
   /**
    * Renders the buttons
    */
@@ -109,6 +127,13 @@ export class BisectDialog extends React.Component<
         onClick={this.onSubmit}
         text="Begin"
       />,
+      <Button
+        icon="lab-test"
+        key="auto"
+        disabled={!this.canAuto}
+        onClick={this.onAuto}
+        text="Auto"
+      />,
       <Button icon="cross" key="cancel" onClick={this.onClose} text="Cancel" />,
     ];
   }
diff --git a/src/renderer/components/editors.tsx b/src/renderer/components/editors.tsx
index 16709482..32c99f1e 100644
--- a/src/renderer/components/editors.tsx
+++ b/src/renderer/components/editors.tsx
@@ -2,6 +2,13 @@ import { reaction } from 'mobx';
 import { observer } from 'mobx-react';
 import * as MonacoType from 'monaco-editor';
 import * as React from 'react';
+import {
+  INDEX_HTML_NAME,
+  MAIN_JS_NAME,
+  PRELOAD_JS_NAME,
+  RENDERER_JS_NAME,
+  STYLES_CSS_NAME,
+} from '../../shared-constants';
 import {
   Mosaic,
   MosaicBranch,
@@ -37,12 +44,12 @@ const defaultMonacoOptions: MonacoType.editor.IEditorOptions = {
 };
 
 export const TITLE_MAP: Record<MosaicId, string> = {
-  main: 'Main Process (main.js)',
-  renderer: 'Renderer Process (renderer.js)',
-  preload: 'Preload (preload.js)',
-  html: 'HTML (index.html)',
-  css: 'Stylesheet (styles.css)',
-  docsDemo: 'Docs & Demos',
+  [EditorId.main]: `Main Process (${MAIN_JS_NAME})`,
+  [EditorId.renderer]: `Renderer Process (${RENDERER_JS_NAME})`,
+  [EditorId.preload]: `Preload (${PRELOAD_JS_NAME})`,
+  [EditorId.html]: `HTML (${INDEX_HTML_NAME})`,
+  [EditorId.css]: `Stylesheet (${STYLES_CSS_NAME})`,
+  [PanelId.docsDemo]: 'Docs & Demos',
 };
 
 export interface EditorsProps {
diff --git a/src/renderer/components/version-select.tsx b/src/renderer/components/version-select.tsx
index d142b13d..7b056937 100644
--- a/src/renderer/components/version-select.tsx
+++ b/src/renderer/components/version-select.tsx
@@ -42,11 +42,16 @@ export function getItemLabel({ source, state, name }: RunnableVersion): string {
  * @returns
  */
 export function getItemIcon({ state }: RunnableVersion) {
-  return state === 'ready'
-    ? 'saved'
-    : state === 'downloading'
-    ? 'cloud-download'
-    : 'cloud';
+  switch (state) {
+    case VersionState.unknown:
+      return 'cloud';
+    case VersionState.ready:
+      return 'saved';
+    case VersionState.downloading:
+      return 'cloud-download';
+    case VersionState.unzipping:
+      return 'compressed';
+  }
 }
 
 /**
diff --git a/src/renderer/file-manager.ts b/src/renderer/file-manager.ts
index 7551b6b9..1bbf2505 100644
--- a/src/renderer/file-manager.ts
+++ b/src/renderer/file-manager.ts
@@ -3,14 +3,7 @@ import * as path from 'path';
 
 import { Files, FileTransform } from '../interfaces';
 import { IpcEvents } from '../ipc-events';
-import {
-  INDEX_HTML_NAME,
-  MAIN_JS_NAME,
-  PACKAGE_NAME,
-  PRELOAD_JS_NAME,
-  RENDERER_JS_NAME,
-  STYLES_CSS_NAME,
-} from '../shared-constants';
+import { FILENAME_KEYS, PACKAGE_NAME } from '../shared-constants';
 import { DEFAULT_OPTIONS, PackageJsonOptions } from '../utils/get-package';
 import { fancyImport } from '../utils/import';
 import { readFiddle } from '../utils/read-fiddle';
@@ -133,13 +126,11 @@ export class FileManager {
   ): Promise<Files> {
     const pOptions = typeof options === 'object' ? options : DEFAULT_OPTIONS;
     const values = await window.ElectronFiddle.app.getEditorValues(pOptions);
-    let output: Files = new Map();
 
-    output.set(RENDERER_JS_NAME, values.renderer);
-    output.set(MAIN_JS_NAME, values.main);
-    output.set(INDEX_HTML_NAME, values.html);
-    output.set(PRELOAD_JS_NAME, values.preload);
-    output.set(STYLES_CSS_NAME, values.css);
+    let output: Files = new Map();
+    for (const [filename, editorId] of Object.entries(FILENAME_KEYS)) {
+      output.set(filename, values[editorId]);
+    }
     output.set(PACKAGE_NAME, values.package!);
 
     for (const transform of transforms) {
diff --git a/src/renderer/remote-loader.ts b/src/renderer/remote-loader.ts
index d61a8c68..78300f64 100644
--- a/src/renderer/remote-loader.ts
+++ b/src/renderer/remote-loader.ts
@@ -1,14 +1,7 @@
 import { Octokit } from '@octokit/rest';
 import { when } from 'mobx';
 import { EditorValues, GenericDialogType } from '../interfaces';
-import {
-  INDEX_HTML_NAME,
-  MAIN_JS_NAME,
-  PRELOAD_JS_NAME,
-  RENDERER_JS_NAME,
-  STYLES_CSS_NAME,
-  FILENAME_KEYS,
-} from '../shared-constants';
+import { FILENAME_KEYS } from '../shared-constants';
 import { getOctokit } from '../utils/octokit';
 import { sortedElectronMap } from '../utils/sorted-electron-map';
 import { ELECTRON_ORG, ELECTRON_REPO } from './constants';
@@ -136,16 +129,11 @@ export class RemoteLoader {
       const octo = await getOctokit(this.appState);
       const gist = await octo.gists.get({ gist_id: gistId });
 
-      return this.handleLoadingSuccess(
-        {
-          html: this.getContentOrEmpty(gist, INDEX_HTML_NAME),
-          main: this.getContentOrEmpty(gist, MAIN_JS_NAME),
-          renderer: this.getContentOrEmpty(gist, RENDERER_JS_NAME),
-          preload: this.getContentOrEmpty(gist, PRELOAD_JS_NAME),
-          css: this.getContentOrEmpty(gist, STYLES_CSS_NAME),
-        },
-        gistId,
-      );
+      const values: Partial<EditorValues> = {};
+      for (const [filename, editorId] of Object.entries(FILENAME_KEYS)) {
+        values[editorId] = this.getContentOrEmpty(gist, filename);
+      }
+      return this.handleLoadingSuccess(values, gistId);
     } catch (error) {
       return this.handleLoadingFailed(error);
     }
diff --git a/src/renderer/runner.ts b/src/renderer/runner.ts
index d3d12046..f8ad6ea6 100644
--- a/src/renderer/runner.ts
+++ b/src/renderer/runner.ts
@@ -60,7 +60,21 @@ export class Runner {
    * @returns {Promise<RunResult>}
    * @memberof Runner
    */
-  public async autobisect(
+  public autobisect(versions: Array<RunnableVersion>): Promise<RunResult> {
+    const { appState } = this;
+    appState.isAutoBisecting = true;
+    const done = () => (appState.isAutoBisecting = false);
+    return this.autobisectImpl(versions).finally(done);
+  }
+
+  /**
+   * Bisect the current fiddle across the specified versions.
+   *
+   * @param {Array<RunnableVersion>} versions - versions to bisect
+   * @returns {Promise<RunResult>}
+   * @memberof Runner
+   */
+  public async autobisectImpl(
     versions: Array<RunnableVersion>,
   ): Promise<RunResult> {
     const { appState } = this;
diff --git a/src/renderer/state.ts b/src/renderer/state.ts
index 915f62b0..d422d7ec 100644
--- a/src/renderer/state.ts
+++ b/src/renderer/state.ts
@@ -147,6 +147,7 @@ export class AppState {
 
   @observable public activeGistAction: GistActionState = GistActionState.none;
   @observable public isRunning = false;
+  @observable public isAutoBisecting = false;
   @observable public isInstallingModules = false;
   @observable public isUnsaved: boolean;
   @observable public isUpdatingElectronVersions = false;
diff --git a/src/utils/get-package.ts b/src/utils/get-package.ts
index 3f8b673a..a233b6c4 100644
--- a/src/utils/get-package.ts
+++ b/src/utils/get-package.ts
@@ -1,4 +1,5 @@
 import { EditorValues } from '../interfaces';
+import { MAIN_JS_NAME } from '../shared-constants';
 import { findModulesInEditors } from '../renderer/npm';
 import { AppState } from '../renderer/state';
 import { getUsername } from './get-username';
@@ -50,7 +51,7 @@ export async function getPackageJson(
       productName: name,
       description: 'My Electron application description',
       keywords: [],
-      main: './main.js',
+      main: `./${MAIN_JS_NAME}`,
       version: '1.0.0',
       author: getUsername(),
       scripts: {


Test patch:
diff --git a/tests/renderer/components/__snapshots__/commands-version-chooser-spec.tsx.snap b/tests/renderer/components/__snapshots__/commands-version-chooser-spec.tsx.snap
index 84940e53..3a6a3cb0 100644
--- a/tests/renderer/components/__snapshots__/commands-version-chooser-spec.tsx.snap
+++ b/tests/renderer/components/__snapshots__/commands-version-chooser-spec.tsx.snap
@@ -104,7 +104,6 @@ exports[`VersionSelect component renders 1`] = `
         "version": "2.0.2",
       }
     }
-    disabled={false}
     onVersionSelect={[Function]}
   />
 </Blueprint3.ButtonGroup>
diff --git a/tests/renderer/components/__snapshots__/dialog-bisect-spec.tsx.snap b/tests/renderer/components/__snapshots__/dialog-bisect-spec.tsx.snap
index 015df037..38059a6c 100644
--- a/tests/renderer/components/__snapshots__/dialog-bisect-spec.tsx.snap
+++ b/tests/renderer/components/__snapshots__/dialog-bisect-spec.tsx.snap
@@ -221,6 +221,13 @@ exports[`BisectDialog component renders 1`] = `
         onClick={[Function]}
         text="Begin"
       />
+      <Blueprint3.Button
+        disabled={false}
+        icon="lab-test"
+        key="auto"
+        onClick={[Function]}
+        text="Auto"
+      />
       <Blueprint3.Button
         icon="cross"
         key="cancel"
@@ -439,6 +446,13 @@ exports[`BisectDialog component renders 2`] = `
         onClick={[Function]}
         text="Begin"
       />
+      <Blueprint3.Button
+        disabled={true}
+        icon="lab-test"
+        key="auto"
+        onClick={[Function]}
+        text="Auto"
+      />
       <Blueprint3.Button
         icon="cross"
         key="cancel"
@@ -664,6 +678,13 @@ exports[`BisectDialog component renders 3`] = `
         onClick={[Function]}
         text="Begin"
       />
+      <Blueprint3.Button
+        disabled={true}
+        icon="lab-test"
+        key="auto"
+        onClick={[Function]}
+        text="Auto"
+      />
       <Blueprint3.Button
         icon="cross"
         key="cancel"
@@ -896,6 +917,13 @@ exports[`BisectDialog component renders 4`] = `
         onClick={[Function]}
         text="Begin"
       />
+      <Blueprint3.Button
+        disabled={true}
+        icon="lab-test"
+        key="auto"
+        onClick={[Function]}
+        text="Auto"
+      />
       <Blueprint3.Button
         icon="cross"
         key="cancel"
@@ -1129,6 +1157,13 @@ exports[`BisectDialog component renders 5`] = `
         onClick={[Function]}
         text="Begin"
       />
+      <Blueprint3.Button
+        disabled={true}
+        icon="lab-test"
+        key="auto"
+        onClick={[Function]}
+        text="Auto"
+      />
       <Blueprint3.Button
         icon="cross"
         key="cancel"
diff --git a/tests/renderer/components/dialog-bisect-spec.tsx b/tests/renderer/components/dialog-bisect-spec.tsx
index b8f59204..f1994de1 100644
--- a/tests/renderer/components/dialog-bisect-spec.tsx
+++ b/tests/renderer/components/dialog-bisect-spec.tsx
@@ -1,6 +1,10 @@
 import { shallow } from 'enzyme';
 import * as React from 'react';
-import { VersionSource, VersionState } from '../../../src/interfaces';
+import {
+  RunResult,
+  VersionSource,
+  VersionState,
+} from '../../../src/interfaces';
 import { Bisector } from '../../../src/renderer/bisect';
 import { BisectDialog } from '../../../src/renderer/components/dialog-bisect';
 import { ElectronReleaseChannel } from '../../../src/renderer/versions';
@@ -137,6 +141,54 @@ describe('BisectDialog component', () => {
     });
   });
 
+  describe('onAuto()', () => {
+    it('initiates autobisect', async () => {
+      // setup: dialog state
+      const wrapper = shallow(<BisectDialog appState={store} />);
+      wrapper.setState({
+        allVersions: generateVersionRange(5),
+        endIndex: 0,
+        startIndex: 4,
+      });
+
+      // setup: the spy
+      const spy = jest
+        .spyOn(window.ElectronFiddle.app.runner, 'autobisect')
+        .mockResolvedValueOnce(RunResult.SUCCESS);
+
+      // click the 'auto' button
+      const instance1: BisectDialog = wrapper.instance() as any;
+      await instance1.onAuto();
+
+      // check the results
+      expect(spy).toHaveBeenCalled();
+
+      // cleanup
+      spy.mockRestore();
+    });
+
+    it('does nothing if endIndex or startIndex are falsy', async () => {
+      const wrapper = shallow(<BisectDialog appState={store} />);
+
+      wrapper.setState({
+        startIndex: undefined,
+        endIndex: 0,
+      });
+      const instance1: BisectDialog = wrapper.instance() as any;
+      await instance1.onAuto();
+      expect(Bisector).not.toHaveBeenCalled();
+
+      wrapper.setState({
+        startIndex: 4,
+        endIndex: undefined,
+      });
+
+      const instance2: BisectDialog = wrapper.instance() as any;
+      await instance2.onAuto();
+      expect(Bisector).not.toHaveBeenCalled();
+    });
+  });
+
   describe('items disabled', () => {
     let instance: BisectDialog;
 
diff --git a/tests/renderer/components/version-select-spec.tsx b/tests/renderer/components/version-select-spec.tsx
index bf359e49..394ba10e 100644
--- a/tests/renderer/components/version-select-spec.tsx
+++ b/tests/renderer/components/version-select-spec.tsx
@@ -16,7 +16,7 @@ import {
 import { ElectronReleaseChannel } from '../../../src/renderer/versions';
 import { mockVersions } from '../../mocks/electron-versions';
 
-const { ready, unknown, downloading } = VersionState;
+const { downloading, ready, unknown, unzipping } = VersionState;
 const { remote, local } = VersionSource;
 
 describe('VersionSelect component', () => {
@@ -133,14 +133,15 @@ describe('VersionSelect component', () => {
 
   describe('getItemIcon()', () => {
     it('returns the correct icon', () => {
-      const vDownloaded = { ...mockVersion1, state: ready };
-      expect(getItemIcon(vDownloaded)).toBe('saved');
-
-      const vDownloading = { ...mockVersion1, state: downloading };
-      expect(getItemIcon(vDownloading)).toBe('cloud-download');
-
-      const vUnknown = { ...mockVersion1, state: unknown };
-      expect(getItemIcon(vUnknown)).toBe('cloud');
+      const icons: Array<{ state: VersionState; expected: string }> = [
+        { state: downloading, expected: 'cloud-download' },
+        { state: ready, expected: 'saved' },
+        { state: unknown, expected: 'cloud' },
+        { state: unzipping, expected: 'compressed' },
+      ];
+      icons.forEach(({ state, expected }) => {
+        expect(getItemIcon({ ...mockVersion1, state })).toBe(expected);
+      });
     });
   });