Repository: electron/fiddle (ts)

Issue:
commented npm modules not ignored if comments are multi-line
Given this code in an editor pane:

```js
    // const cow = require('cow');
    /* const say = require('say'); */
    /**
     * const hello = require('hello'); 
     * const world = require('world'); 
    */
```

Fiddle will still attempt to install the `say`, `hello`, and `world` packages from NPM. This is because we only check if lines start with `//` to check if they're commented.
npm modules installing multiple times if required in multiple editors
Given the below code:

```js
// main.js
const log = require('electron-log');
```

```js
// renderer.js
const log = require('electron-log');
```

Fiddle will attempt to `npm install` the `electron-log` package twice, once for each file it's in.
npm modules don't load on preload editor
Given this code:

```js
// preload.js
const _ = require('lodash');
```

Fiddle will not attempt to install the `electron-log` module, leading to the following error in the renderer console:

```
Error: Cannot find module 'lodash'
```

Pull request:
fix: various automagic npm module installation fixes
This PR has 3 bugfixes relating to the way we import `npm` modules. Each is in a separate commit.

* 5e3a588 Fixes #509 by using the `decomment` module inside a Web Worker thread.
* 57aa3d1 Fixes #510 by adding the list of modules detected into a `Set()`.
* 7ebbeac Fixes #511 by adding `preload` to the list of editors that we check modules for.
* 93c24dd adds tests to make sure everything works. :)

**Note**: this PR adds in a new dependency to the [`decomment`](https://github.com/vitaly-t/decomment) module, which uses the [Esprima](https://esprima.org/) JavaScript parser under the hood to strip any JS code fed in of its comments. I decided to use a library rather than play around with regexes for reliability's sake. `decomment` is licensed under MIT, so it should be compatible with Electron Fiddle for licensing purposes.

Patch:
diff --git a/package.json b/package.json
index d1620058..bc52b534 100644
--- a/package.json
+++ b/package.json
@@ -41,6 +41,7 @@
     "@octokit/rest": "^16.43.1",
     "@sentry/electron": "^2.0.1",
     "classnames": "^2.2.6",
+    "decomment": "^0.9.3",
     "electron-default-menu": "^1.0.2",
     "electron-devtools-installer": "^3.1.1",
     "electron-squirrel-startup": "^1.0.0",
@@ -71,6 +72,7 @@
     "@octokit/action": "^2.0.0",
     "@types/classnames": "^2.2.10",
     "@types/enzyme": "^3.10.7",
+    "@types/decomment": "^0.9.0",
     "@types/fs-extra": "^9.0.2",
     "@types/jest": "^26.0.14",
     "@types/log-symbols": "^3.0.0",
diff --git a/src/main/windows.ts b/src/main/windows.ts
index 08c56df0..54b0231c 100644
--- a/src/main/windows.ts
+++ b/src/main/windows.ts
@@ -25,6 +25,7 @@ export function getMainWindowOptions(): Electron.BrowserWindowConstructorOptions
       webviewTag: false,
       nodeIntegration: true,
       enableRemoteModule: true,
+      nodeIntegrationInWorker: true,
     },
   };
 }
diff --git a/src/renderer/app.tsx b/src/renderer/app.tsx
index 808d55e8..ffa9db2f 100644
--- a/src/renderer/app.tsx
+++ b/src/renderer/app.tsx
@@ -71,11 +71,28 @@ export class App {
 
     // display all editors that have content
     const visibleEditors = [];
-    for (const id of Object.keys(editorValues)) {
+
+    // ensure consistent sorting of mosaics
+    const sortedEditors = Object.keys(editorValues).sort((a, b) => {
+      const order: Array<string> = [
+        'main',
+        'renderer',
+        'html',
+        'preload',
+        'css',
+      ];
+
+      return order.indexOf(a) - order.indexOf(b);
+    });
+
+    for (const id of sortedEditors) {
       const content = editorValues[id];
 
-      // if the gist content matches the empty file output, don't show it
-      if (!Object.values(EMPTY_EDITOR_CONTENT).includes(content)) {
+      // if the gist content is empty or matches the empty file output, don't show it
+      if (
+        content.length > 0 &&
+        !Object.values(EMPTY_EDITOR_CONTENT).includes(content)
+      ) {
         visibleEditors.push(id as EditorId);
       }
     }
diff --git a/src/renderer/npm.ts b/src/renderer/npm.ts
index f1737977..837e676d 100644
--- a/src/renderer/npm.ts
+++ b/src/renderer/npm.ts
@@ -1,7 +1,7 @@
 import { EditorValues } from '../interfaces';
 import { exec } from '../utils/exec';
 
-const { builtinModules } = require('module');
+import { builtinModules } from 'module';
 
 export type IPackageManager = 'npm' | 'yarn';
 
@@ -22,7 +22,7 @@ const ignoredModules: Array<string> = [
 ];
 
 /* regular expression to both match and extract module names */
-const requiregx = /^.*require\(['"](.*?)['"]\)/gm;
+const requiregx = /require\(['"](.*?)['"]\)/gm;
 
 /*
  Quick and dirty filter functions for filtering module names
@@ -72,24 +72,22 @@ export async function getIsPackageManagerInstalled(
 
 /**
  * Finds npm modules in editor values, returning an array of modules.
- *
- * @param {EditorValues} values
- * @returns {Array<string>}
  */
-export function findModulesInEditors(values: EditorValues) {
-  const files = [values.main, values.renderer];
-  const modules = files.reduce(
-    (agg, file) => [...agg, ...findModules(file)],
-    [],
-  );
+export async function findModulesInEditors(values: EditorValues) {
+  const files = [values.main, values.renderer, values.preload];
+  const modules: Array<string> = [];
+
+  for (const file of files) {
+    modules.push(...(await findModules(file)));
+  }
 
   console.log('Modules Found:', modules);
 
-  return modules;
+  return Array.from(new Set(modules));
 }
 
 /**
- * Uses a simple regex to find `require()` statements in a string.
+ * Finds `require()` statements in a string.
  * Tries to exclude electron and Node built-ins as well as file-path
  * references. Also will try to install base packages of modules
  * that have a slash in them, for example: `lodash/fp` as the actual package
@@ -101,18 +99,18 @@ export function findModulesInEditors(values: EditorValues) {
  * @param {string} input
  * @returns {Array<string>}
  */
-export function findModules(input: string): Array<string> {
+export async function findModules(input: string) {
   /* container definitions */
   const modules: Array<string> = [];
   let match: RegExpMatchArray | null;
 
+  /* decomment code with the esprima parser */
+  const code = await decommentWithWorker(input);
+
   /* grab all global require matches in the text */
-  while ((match = requiregx.exec(input) || null)) {
-    // ensure commented-out requires aren't downloaded
-    if (!match[0].startsWith('//')) {
-      const mod = match[1];
-      modules.push(mod);
-    }
+  while ((match = requiregx.exec(code) || null)) {
+    const mod = match[1];
+    modules.push(mod);
   }
 
   /* map and reduce */
@@ -162,3 +160,16 @@ export function packageRun(
 ): Promise<string> {
   return exec(dir, `${packageManager} run ${command}`);
 }
+
+export function decommentWithWorker(input: string): Promise<string> {
+  return new Promise((resolve, reject) => {
+    const worker = new Worker('../utils/decomment.ts');
+    worker.onmessage = function (event: MessageEvent<string>) {
+      resolve(event.data);
+    };
+    worker.postMessage(input);
+    worker.onerror = function (e) {
+      reject(e);
+    };
+  });
+}
diff --git a/src/utils/decomment.ts b/src/utils/decomment.ts
new file mode 100644
index 00000000..4f683b3c
--- /dev/null
+++ b/src/utils/decomment.ts
@@ -0,0 +1,5 @@
+import decomment from 'decomment';
+
+onmessage = function (event: MessageEvent<string>) {
+  postMessage(decomment(event.data));
+};
diff --git a/tsconfig.json b/tsconfig.json
index d605a9ca..1d5bd5d6 100644
--- a/tsconfig.json
+++ b/tsconfig.json
@@ -10,7 +10,8 @@
     "sourceMap": true,
     "lib": [
       "es2017",
-      "dom"
+      "dom",
+      "WebWorker"
     ],
     "noImplicitAny": true,
     "noImplicitReturns": true,
@@ -41,4 +42,4 @@
     "indentSize": 2,
     "tabSize": 2
   }
-}
\ No newline at end of file
+}
diff --git a/yarn.lock b/yarn.lock
index 4b443b84..59b21123 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -1924,6 +1924,11 @@
   resolved "https://registry.yarnpkg.com/@types/classnames/-/classnames-2.2.10.tgz#cc658ca319b6355399efc1f5b9e818f1a24bf999"
   integrity sha512-1UzDldn9GfYYEsWWnn/P4wkTlkZDH7lDb0wBMGbtIQc9zXEQq7FlKBdZUn6OBqD8sKZZ2RQO2mAjGpXiDGoRmQ==
 
+"@types/decomment@^0.9.0":
+  version "0.9.0"
+  resolved "https://registry.yarnpkg.com/@types/decomment/-/decomment-0.9.0.tgz#ef4701e474eef774e9ddd65e4e85f6c3dab5ce9f"
+  integrity sha512-3K8zqMlLQREJtIMboTH9GyuX/CSL5cFg+B1gVezUdEJ9lm8IPIJzO+WH0L1sQnkHIdU8Wz65rzfrXb+NojWr1Q==
+
 "@types/dom4@^2.0.1":
   version "2.0.1"
   resolved "https://registry.yarnpkg.com/@types/dom4/-/dom4-2.0.1.tgz#506d5781b9bcab81bd9a878b198aec7dee2a6033"
@@ -3974,6 +3979,13 @@ decode-uri-component@^0.2.0:
   resolved "https://registry.yarnpkg.com/decode-uri-component/-/decode-uri-component-0.2.0.tgz#eb3913333458775cb84cd1a1fae062106bb87545"
   integrity sha1-6zkTMzRYd1y4TNGh+uBiEGu4dUU=
 
+decomment@^0.9.3:
+  version "0.9.3"
+  resolved "https://registry.yarnpkg.com/decomment/-/decomment-0.9.3.tgz#b913f32e5fe1113848f516caa5c7afefa9544d38"
+  integrity sha512-5skH5BfUL3n09RDmMVaHS1QGCiZRnl2nArUwmsE9JRY93Ueh3tihYl5wIrDdAuXnoFhxVis/DmRWREO2c6DG3w==
+  dependencies:
+    esprima "4.0.1"
+
 decompress-response@^3.3.0:
   version "3.3.0"
   resolved "https://registry.yarnpkg.com/decompress-response/-/decompress-response-3.3.0.tgz#80a4dd323748384bfa248083622aedec982adff3"
@@ -4982,16 +4994,16 @@ espree@^7.3.0:
     acorn-jsx "^5.2.0"
     eslint-visitor-keys "^1.3.0"
 
+esprima@4.0.1, esprima@^4.0.0, esprima@^4.0.1:
+  version "4.0.1"
+  resolved "https://registry.yarnpkg.com/esprima/-/esprima-4.0.1.tgz#13b04cdb3e6c5d19df91ab6987a8695619b0aa71"
+  integrity sha512-eGuFFw7Upda+g4p+QHvnW0RyTX/SVeJBDM/gCtMARO0cLuT2HcEKnTPvhjV6aGeqrCB/sbNop0Kszm0jsaWU4A==
+
 esprima@^3.1.3:
   version "3.1.3"
   resolved "https://registry.yarnpkg.com/esprima/-/esprima-3.1.3.tgz#fdca51cee6133895e3c88d535ce49dbff62a4633"
   integrity sha1-/cpRzuYTOJXjyI1TXOSdv/YqRjM=
 
-esprima@^4.0.0, esprima@^4.0.1:
-  version "4.0.1"
-  resolved "https://registry.yarnpkg.com/esprima/-/esprima-4.0.1.tgz#13b04cdb3e6c5d19df91ab6987a8695619b0aa71"
-  integrity sha512-eGuFFw7Upda+g4p+QHvnW0RyTX/SVeJBDM/gCtMARO0cLuT2HcEKnTPvhjV6aGeqrCB/sbNop0Kszm0jsaWU4A==
-
 esquery@^1.0.1, esquery@^1.2.0:
   version "1.3.1"
   resolved "https://registry.yarnpkg.com/esquery/-/esquery-1.3.1.tgz#b78b5828aa8e214e29fb74c4d5b752e1c033da57"


Test patch:
diff --git a/tests/main/windows-spec.ts b/tests/main/windows-spec.ts
index e6aadb10..79df2f91 100644
--- a/tests/main/windows-spec.ts
+++ b/tests/main/windows-spec.ts
@@ -35,6 +35,7 @@ describe('windows', () => {
         webviewTag: false,
         enableRemoteModule: true,
         nodeIntegration: true,
+        nodeIntegrationInWorker: true,
       },
     };
 
diff --git a/tests/mocks/worker.ts b/tests/mocks/worker.ts
new file mode 100644
index 00000000..9305a742
--- /dev/null
+++ b/tests/mocks/worker.ts
@@ -0,0 +1,20 @@
+import * as decomment from 'decomment';
+
+export default class MockWorker {
+  public onmessage: (msg: any) => any;
+  public onmessageerror: () => any;
+  public onerror: () => any;
+  public addEventListener: () => any;
+  public removeEventListener: () => any;
+  public dispatchEvent: () => any;
+  public terminate: () => any;
+  constructor() {
+    this.onmessage = () => {
+      /*no-op*/
+    };
+  }
+
+  public postMessage(input: string) {
+    this.onmessage({ data: decomment(input) });
+  }
+}
diff --git a/tests/renderer/app-spec.tsx b/tests/renderer/app-spec.tsx
index a5bdc23e..c2cca04f 100644
--- a/tests/renderer/app-spec.tsx
+++ b/tests/renderer/app-spec.tsx
@@ -4,6 +4,7 @@ import { EditorBackup } from '../../src/utils/editor-backup';
 import { ElectronFiddleMock } from '../mocks/electron-fiddle';
 import { MockState } from '../mocks/state';
 import { overridePlatform, resetPlatform } from '../utils';
+import { EditorId } from '../../src/interfaces';
 
 jest.mock('../../src/renderer/file-manager', () =>
   require('../mocks/file-manager'),
@@ -110,18 +111,24 @@ describe('Editors component', () => {
   });
 
   describe('replaceFiddle()', () => {
-    it('sets editor values and source info', (done) => {
-      const app = new App();
+    let app: App;
+
+    beforeEach(() => {
+      app = new App();
       (app.state as Partial<AppState>) = new MockState();
       app.state.isUnsaved = false;
+      app.state.setGenericDialogOptions = jest.fn();
       app.state.setVisibleMosaics = jest.fn();
       app.setEditorValues = jest.fn();
+    });
 
+    it('sets editor values and source info', (done) => {
       const editorValues = {
         html: 'html-value',
         main: 'main-value',
         renderer: 'renderer-value',
         preload: '',
+        css: '',
       };
 
       app
@@ -139,14 +146,55 @@ describe('Editors component', () => {
         });
     });
 
+    it('only shows mosaic for non-empty editor contents', (done) => {
+      const editorValues = {
+        main: 'main-value',
+        renderer: 'renderer-value',
+        html: 'html-value',
+        preload: '',
+        css: '/* Empty */',
+      };
+
+      app
+        .replaceFiddle(editorValues, {
+          gistId: 'gistId',
+        })
+        .then(() => {
+          expect(app.state.setVisibleMosaics).toHaveBeenCalledWith([
+            EditorId.main,
+            EditorId.renderer,
+            EditorId.html,
+          ]);
+          done();
+        });
+    });
+
+    it('shows visible mosaics in the correct pre-defined order', (done) => {
+      // this order is defined inside the replaceFiddle() function
+      const editorValues = {
+        css: 'css-value',
+        html: 'html-value',
+        renderer: 'renderer-value',
+        main: 'main-value',
+      };
+      app
+        .replaceFiddle(editorValues, {
+          gistId: 'gistId',
+        })
+        .then(() => {
+          expect(app.state.setVisibleMosaics).toHaveBeenCalledWith([
+            EditorId.main,
+            EditorId.renderer,
+            EditorId.html,
+            EditorId.css,
+          ]);
+          done();
+        });
+    });
+
     it('unsets state of previous source when called', (done) => {
-      const app = new App();
-      (app.state as Partial<AppState>) = new MockState();
       app.state.isUnsaved = true;
       app.state.localPath = '/fake/path';
-      app.state.setGenericDialogOptions = jest.fn();
-      app.state.setVisibleMosaics = jest.fn();
-      app.setEditorValues = jest.fn();
 
       const editorValues = {
         html: 'html-value',
@@ -173,12 +221,6 @@ describe('Editors component', () => {
     });
 
     it('marks the new Fiddle as Saved', (done) => {
-      const app = new App();
-      (app.state as Partial<AppState>) = new MockState();
-      app.state.isUnsaved = false;
-      app.state.setVisibleMosaics = jest.fn();
-      app.setEditorValues = jest.fn();
-
       const editorValues = {
         html: 'html-value',
         main: 'main-value',
@@ -196,15 +238,10 @@ describe('Editors component', () => {
           done();
         });
     });
+
     describe('when current Fiddle is unsaved and prompt appears', () => {
       it('takes no action if prompt is rejected', (done) => {
-        const app = new App();
-        (app.state as Partial<AppState>) = new MockState();
         app.state.isUnsaved = true;
-        app.state.setVisibleMosaics = jest.fn();
-        app.state.setGenericDialogOptions = jest.fn();
-        app.setEditorValues = jest.fn();
-
         expect(app.state.localPath).toBeUndefined();
         expect(app.state.gistId).toBe('');
         expect(app.state.templateName).toBeUndefined();
diff --git a/tests/renderer/npm-spec.ts b/tests/renderer/npm-spec.ts
index 6fe019c3..5180478f 100644
--- a/tests/renderer/npm-spec.ts
+++ b/tests/renderer/npm-spec.ts
@@ -1,4 +1,8 @@
+import { mocked } from 'ts-jest/utils';
+import * as decomment from 'decomment';
+
 import {
+  findModules,
   findModulesInEditors,
   getIsPackageManagerInstalled,
   installModules,
@@ -6,7 +10,8 @@ import {
 } from '../../src/renderer/npm';
 import { exec } from '../../src/utils/exec';
 import { overridePlatform, resetPlatform } from '../utils';
-
+import MockDecommentWorker from '../mocks/worker';
+jest.mock('decomment');
 jest.mock('../../src/utils/exec');
 jest.mock('../../src/utils/import', () => ({
   fancyImport: async (_p: string) => {
@@ -14,10 +19,10 @@ jest.mock('../../src/utils/import', () => ({
   },
 }));
 
-describe('npm', () => {
-  const mockMain = `
-    const say = require('say');
+window.Worker = MockDecommentWorker;
 
+describe('npm', () => {
+  const mockBuiltins = `
     function hello() {
       const electron = require('electron');
       const originalFs = require('original-fs');
@@ -26,6 +31,20 @@ describe('npm', () => {
     }
   `;
 
+  const mockPackages = `
+    const cow = require('cow');
+    const say = require('say');
+  `;
+
+  const mockComments = `
+    // const cow = require('cow');
+    /* const say = require('say'); */
+    /**
+     * const hello = require('hello'); 
+     * const world = require('world'); 
+    */
+  `;
+
   describe('getIsPackageManagerInstalled()', () => {
     describe('npm()', () => {
       beforeEach(() => {
@@ -150,17 +169,39 @@ describe('npm', () => {
     });
   });
 
+  describe('findModules()', () => {
+    it('returns required modules in a JS file', async () => {
+      mocked(decomment).mockReturnValue(mockPackages);
+      const modules = await findModules(mockPackages);
+      expect(modules).toEqual(['cow', 'say']);
+    });
+
+    it('ignores node and electron builtins', async () => {
+      mocked(decomment).mockReturnValue(mockBuiltins);
+      const modules = await findModules(mockBuiltins);
+      expect(modules).toHaveLength(0);
+    });
+
+    it('ignores commented modules', async () => {
+      mocked(decomment).mockReturnValue('');
+      const modules = await findModules(mockComments);
+      expect(modules).toHaveLength(0);
+    });
+  });
+
   describe('findModulesInEditors()', () => {
-    it('finds modules, ignoring node and electron builtins', async () => {
+    it('installs modules across all JavaScript files only once', async () => {
+      mocked(decomment).mockReturnValue(mockPackages);
       const result = await findModulesInEditors({
         html: '',
-        main: mockMain,
-        renderer: '',
-        preload: '',
+        main: mockPackages,
+        renderer: mockPackages,
+        preload: mockPackages,
         css: '',
       });
 
-      expect(result).toEqual(['say']);
+      expect(result).toHaveLength(2);
+      expect(result).toEqual(['cow', 'say']);
     });
   });
 
diff --git a/tests/renderer/state-spec.ts b/tests/renderer/state-spec.ts
index 3c6a8b14..2f7220b8 100644
--- a/tests/renderer/state-spec.ts
+++ b/tests/renderer/state-spec.ts
@@ -74,6 +74,7 @@ describe('AppState', () => {
 
   beforeEach(() => {
     appState = new AppState();
+    appState.updateDownloadedVersionState = jest.fn();
     ipcRendererManager.removeAllListeners();
   });
 
@@ -475,6 +476,8 @@ describe('AppState', () => {
 
   describe('updateDownloadedVersionState()', () => {
     beforeEach(() => {
+      appState = new AppState();
+      ipcRendererManager.removeAllListeners();
       (getDownloadingVersions as jest.Mock).mockReturnValue(['2.0.1']);
       (getDownloadedVersions as jest.Mock).mockReturnValue(
         Promise.resolve(['2.0.2']),