fix(core): better error detection for electron-prebuilt-compile (#2268)

package.json

@@ -112,6 +112,7 @@
     "@types/proxyquire": "^1.3.28",
     "@types/semver": "^7.3.4",
     "@types/sinon": "^9.0.10",
+    "@types/sinon-chai": "^3.2.5",
     "@types/username": "^5.0.0",
     "@types/webpack": "^4.41.21",
     "@types/webpack-dev-middleware": "^4.1.0",
@@ -143,6 +144,7 @@
     "proxyquire": "^2.1.3",
     "rimraf": "^3.0.1",
     "sinon": "^9.2.2",
+    "sinon-chai": "^3.6.0",
     "ts-node": "^9.1.0",
     "typedoc": "^0.20.29",
     "typescript": "~4.0.2"

packages/api/core/package.json

@@ -7,6 +7,12 @@
   "typings": "dist/api/index.d.ts",
   "author": "Samuel Attard",
   "license": "MIT",
+  "scripts": {
+    "coverage:base": "nyc yarn test:base",
+    "test": "yarn test:base test/**/**/*_spec.ts",
+    "test:base": "cross-env TS_NODE_FILES=1 mocha --config ../../../.mocharc.js",
+    "test:fast": "yarn test:base test/fast/**/*_spec.ts"
+  },
   "devDependencies": {
     "@electron-forge/maker-appx": "6.0.0-beta.54",
     "@electron-forge/maker-deb": "6.0.0-beta.54",
@@ -20,11 +26,13 @@
     "@electron-forge/test-utils": "6.0.0-beta.54",
     "chai": "^4.3.3",
     "chai-as-promised": "^7.0.0",
+    "cross-env": "^7.0.2",
     "electron-installer-common": "^0.10.2",
     "fetch-mock": "^9.10.7",
     "mocha": "^8.1.3",
     "proxyquire": "^2.1.3",
-    "sinon": "^9.2.2"
+    "sinon": "^9.2.2",
+    "sinon-chai": "^3.6.0"
   },
   "dependencies": {
     "@electron-forge/async-ora": "6.0.0-beta.54",

packages/api/core/src/api/start.ts

@@ -1,17 +1,20 @@
 import 'colors';
 import { asyncOra } from '@electron-forge/async-ora';
+import debug from 'debug';
 import {
   ElectronProcess, ForgeArch, ForgePlatform, StartOptions,
 } from '@electron-forge/shared-types';
-import path from 'path';
 import { spawn, SpawnOptions } from 'child_process';
 
+import { getElectronVersion } from '../util/electron-version';
+import getForgeConfig from '../util/forge-config';
+import locateElectronExecutable from '../util/electron-executable';
 import { readMutatedPackageJson } from '../util/read-package-json';
 import rebuild from '../util/rebuild';
 import resolveDir from '../util/resolve-dir';
-import getForgeConfig from '../util/forge-config';
 import { runHook } from '../util/hook';
-import { getElectronModulePath, getElectronVersion } from '../util/electron-version';
+
+const d = debug('electron-forge:start');
 
 export { StartOptions };
 
@@ -77,10 +80,11 @@ export default async ({
     }
 
     if (!electronExecPath) {
-      // eslint-disable-next-line import/no-dynamic-require, global-require
-      electronExecPath = require((await getElectronModulePath(dir, packageJSON)) || path.resolve(dir, 'node_modules/electron'));
+      electronExecPath = await locateElectronExecutable(dir, packageJSON);
     }
 
+    d('Electron binary path:', electronExecPath);
+
     const spawnOpts = {
       cwd: dir,
       stdio: 'inherit',

packages/api/core/src/util/electron-executable.ts

@@ -0,0 +1,52 @@
+import logSymbols from 'log-symbols';
+import path from 'path';
+
+import { getElectronModulePath } from './electron-version';
+
+type PackageJSON = Record<string, unknown>;
+type Dependencies = Record<string, string>;
+
+export function pluginCompileExists(packageJSON: PackageJSON): boolean {
+  if (!packageJSON.devDependencies) {
+    return false;
+  }
+
+  const pluginCompileName = '@electron-forge/plugin-compile';
+  const findPluginCompile = (packageName: string): boolean => packageName === pluginCompileName;
+
+  if (Object.keys(packageJSON.devDependencies as Dependencies).find(findPluginCompile)) {
+    return true;
+  }
+
+  if (Object.keys(packageJSON.dependencies as Dependencies || {}).find(findPluginCompile)) {
+    // eslint-disable-next-line no-console
+    console.warn(logSymbols.warning, `${pluginCompileName} was detected in dependencies, it should be in devDependencies`.yellow);
+    return true;
+  }
+
+  return false;
+}
+
+export default async function locateElectronExecutable(
+  dir: string,
+  packageJSON: PackageJSON,
+): Promise<string> {
+  let electronModulePath: string | undefined = await getElectronModulePath(dir, packageJSON);
+  if (electronModulePath?.endsWith('electron-prebuilt-compile') && !pluginCompileExists(packageJSON)) {
+    // eslint-disable-next-line no-console
+    console.warn(logSymbols.warning, 'WARNING: found electron-prebuilt-compile without the Electron Forge compile plugin. Please remove the deprecated electron-prebuilt-compile from your devDependencies.'.yellow);
+    electronModulePath = undefined;
+  }
+
+  // eslint-disable-next-line import/no-dynamic-require, global-require
+  let electronExecPath = require(electronModulePath || path.resolve(dir, 'node_modules/electron'));
+
+  if (typeof electronExecPath !== 'string') {
+    // eslint-disable-next-line no-console
+    console.warn(logSymbols.warning, 'Returned Electron executable path is not a string, defaulting to a hardcoded location. Value:', electronExecPath);
+    // eslint-disable-next-line import/no-dynamic-require, global-require
+    electronExecPath = require(path.resolve(dir, 'node_modules/electron'));
+  }
+
+  return electronExecPath;
+}

packages/api/core/src/util/electron-version.ts

@@ -48,7 +48,7 @@ export class PackageNotFoundError extends Error {
 
 function getElectronModuleName(packageJSON: any): string {
   if (!packageJSON.devDependencies) {
-    throw new Error('package.json for app does not have any devDependencies'.red);
+    throw new Error('package.json for app does not have any devDependencies');
   }
 
   const packageName = electronPackageNames.find((pkg) => packageJSON.devDependencies[pkg]);

packages/api/core/test/fast/electron-executable_spec.ts

@@ -0,0 +1,120 @@
+import chai, { expect } from 'chai';
+import path from 'path';
+import sinon from 'sinon';
+import sinonChai from 'sinon-chai';
+
+import locateElectronExecutable, { pluginCompileExists } from '../../src/util/electron-executable';
+
+chai.use(sinonChai);
+
+const fixtureDir = path.resolve(__dirname, '..', 'fixture', 'electron-executable');
+
+describe('locateElectronExecutable', () => {
+  const sandbox = sinon.createSandbox();
+
+  beforeEach(() => {
+    sandbox.spy(console, 'warn');
+  });
+
+  afterEach(() => {
+    sandbox.restore();
+  });
+
+  it('returns the correct path to electron', async () => {
+    const appFixture = path.join(fixtureDir, 'electron_app');
+    const packageJSON = {
+      devDependencies: { electron: '^100.0.0' },
+    };
+
+    await expect(locateElectronExecutable(appFixture, packageJSON)).to.eventually.equal('execPath');
+
+    // eslint-disable-next-line no-console, no-unused-expressions
+    expect(console.warn).to.not.have.been.called;
+  });
+
+  it('warns and returns a hardcoded path to electron if another electron module does not export a string', async () => {
+    const appFixture = path.join(fixtureDir, 'bad-export');
+    const packageJSON = {
+      devDependencies: {
+        '@electron-forge/plugin-compile': '^6.0.0-beta.1',
+        'electron-prebuilt-compile': '^1.4.0',
+      },
+    };
+
+    await expect(locateElectronExecutable(appFixture, packageJSON)).to.eventually.equal('execPath');
+
+    // eslint-disable-next-line no-console, no-unused-expressions
+    expect(console.warn).to.have.been.calledOnce;
+  });
+
+  it('warns if prebuilt-compile exists without the corresponding plugin', async () => {
+    const packageJSON = {
+      devDependencies: { 'electron-prebuilt-compile': '1.0.0' },
+    };
+    const compileFixture = path.join(fixtureDir, 'prebuilt-compile');
+
+    await expect(locateElectronExecutable(compileFixture, packageJSON)).to.eventually.be.rejected;
+
+    // eslint-disable-next-line no-console, no-unused-expressions
+    expect(console.warn).to.have.been.calledOnce;
+  });
+
+  it('does not warn if prebuilt-compile exists with the corresponding plugin', async () => {
+    const packageJSON = {
+      devDependencies: {
+        '@electron-forge/plugin-compile': '^6.0.0-beta.1',
+        'electron-prebuilt-compile': '1.0.0',
+      },
+    };
+
+    const compileFixture = path.join(fixtureDir, 'prebuilt-compile');
+    // eslint-disable-next-line no-unused-expressions
+    await expect(locateElectronExecutable(compileFixture, packageJSON)).to.eventually.be.rejected;
+
+    // eslint-disable-next-line no-console, no-unused-expressions
+    expect(console.warn).to.not.have.been.called;
+  });
+});
+
+describe('pluginCompileExists', () => {
+  const sandbox = sinon.createSandbox();
+
+  beforeEach(() => {
+    sandbox.spy(console, 'warn');
+  });
+
+  afterEach(() => {
+    sandbox.restore();
+  });
+
+  it('returns false if there is no devDependencies', () => {
+    expect(pluginCompileExists({})).to.equal(false);
+  });
+
+  it('returns false if the plugin is not found in devDependencies', () => {
+    expect(pluginCompileExists({ devDependencies: {} })).to.equal(false);
+  });
+
+  it('returns true if the plugin is found in devDependencies', () => {
+    const packageJSON = {
+      devDependencies: { '@electron-forge/plugin-compile': '^6.0.0-beta.1' },
+    };
+
+    expect(pluginCompileExists(packageJSON)).to.equal(true);
+
+    // eslint-disable-next-line no-console, no-unused-expressions
+    expect(console.warn).to.not.have.been.called;
+  });
+
+  it('warns and returns true if the plugin is found in dependencies', () => {
+    const packageJSON = {
+      dependencies: { '@electron-forge/plugin-compile': '^6.0.0-beta.1' },
+      devDependencies: {},
+    };
+
+    expect(pluginCompileExists(packageJSON)).to.equal(true);
+
+    // eslint-disable-next-line no-console, no-unused-expressions
+    expect(console.warn).to.have.been.calledOnce;
+  });
+});

packages/api/core/test/fast/start_spec.ts

@@ -1,6 +1,5 @@
 import { ElectronProcess } from '@electron-forge/shared-types';
 import { expect } from 'chai';
-import path from 'path';
 import proxyquire from 'proxyquire';
 import sinon, { SinonStub } from 'sinon';
 
@@ -19,11 +18,10 @@ describe('start', () => {
     spawnStub = sinon.stub();
     shouldOverride = false;
     packageJSON = require('../fixture/dummy_app/package.json');
-    const electronPath = path.resolve(__dirname, 'node_modules/electron');
 
     start = proxyquire.noCallThru().load('../../src/api/start', {
+      '../util/electron-executable': () => Promise.resolve('fake_electron_path'),
       '../util/electron-version': {
-        getElectronModulePath: () => Promise.resolve(electronPath),
         getElectronVersion: () => Promise.resolve('1.0.0'),
       },
       '../util/forge-config': async () => ({
@@ -32,7 +30,6 @@ describe('start', () => {
           triggerHook: async () => false,
         },
       }),
-      [electronPath]: 'fake_electron_path',
       '../util/resolve-dir': async (dir: string) => resolveStub(dir),
       '../util/read-package-json': {
         readMutatedPackageJson: () => Promise.resolve(packageJSON),

packages/api/core/typings

@@ -0,0 +1 @@
+../../../typings

yarn.lock

@@ -1270,6 +1270,13 @@
   dependencies:
     "@sinonjs/commons" "^1.7.0"
 
+"@sinonjs/fake-timers@^7.0.4":
+  version "7.0.5"
+  resolved "https://registry.yarnpkg.com/@sinonjs/fake-timers/-/fake-timers-7.0.5.tgz#558a7f8145a01366c44b3dcbdd7172c05c461564"
+  integrity sha512-fUt6b15bjV/VW93UP5opNXJxdwZSbK1EdiwnhN7XrQrcpaOhMJpZ/CjwFpM3THpxwA+YviBUJKSuEqKlCK5alw==
+  dependencies:
+    "@sinonjs/commons" "^1.7.0"
+
 "@sinonjs/samsam@^5.3.1":
   version "5.3.1"
   resolved "https://registry.yarnpkg.com/@sinonjs/samsam/-/samsam-5.3.1.tgz#375a45fe6ed4e92fca2fb920e007c48232a6507f"
@@ -1531,6 +1538,21 @@
     "@types/mime" "^1"
     "@types/node" "*"
 
+"@types/sinon-chai@^3.2.5":
+  version "3.2.5"
+  resolved "https://registry.yarnpkg.com/@types/sinon-chai/-/sinon-chai-3.2.5.tgz#df21ae57b10757da0b26f512145c065f2ad45c48"
+  integrity sha512-bKQqIpew7mmIGNRlxW6Zli/QVyc3zikpGzCa797B/tRnD9OtHvZ/ts8sYXV+Ilj9u3QRaUEM8xrjgd1gwm1BpQ==
+  dependencies:
+    "@types/chai" "*"
+    "@types/sinon" "*"
+
+"@types/sinon@*":
+  version "10.0.0"
+  resolved "https://registry.yarnpkg.com/@types/sinon/-/sinon-10.0.0.tgz#eecc3847af03d45ffe53d55aaaaf6ecb28b5e584"
+  integrity sha512-jDZ55oCKxqlDmoTBBbBBEx+N8ZraUVhggMZ9T5t+6/Dh8/4NiOjSUfpLrPiEwxQDlAe3wpAkoXhWvE6LibtsMQ==
+  dependencies:
+    "@sinonjs/fake-timers" "^7.0.4"
+
 "@types/sinon@^9.0.10":
   version "9.0.11"
   resolved "https://registry.yarnpkg.com/@types/sinon/-/sinon-9.0.11.tgz#7af202dda5253a847b511c929d8b6dda170562eb"
@@ -9380,6 +9402,11 @@ single-line-log@^1.1.2:
   dependencies:
     string-width "^1.0.1"
 
+sinon-chai@^3.6.0:
+  version "3.6.0"
+  resolved "https://registry.yarnpkg.com/sinon-chai/-/sinon-chai-3.6.0.tgz#25bd59a37ef8990245e085a40f1f79d6bf023b7a"
+  integrity sha512-bk2h+0xyKnmvazAnc7HE5esttqmCerSMcBtuB2PS2T4tG6x8woXAxZeJaOJWD+8reXHngnXn0RtIbfEW9OTHFg==
+
 sinon@^9.2.2:
   version "9.2.4"
   resolved "https://registry.yarnpkg.com/sinon/-/sinon-9.2.4.tgz#e55af4d3b174a4443a8762fa8421c2976683752b"