fix(core): better errors when maker names are invalid (#2467)

packages/api/core/src/api/make.ts

@@ -120,6 +120,13 @@ export default async ({
       if (!maker.platforms.includes(actualTargetPlatform)) continue;
     } else {
       const resolvableTarget: IForgeResolvableMaker = target as IForgeResolvableMaker;
+
+      if (!resolvableTarget.name) {
+        throw new Error(`The following maker config is missing a maker name: ${JSON.stringify(resolvableTarget)}`);
+      } else if (typeof resolvableTarget.name !== 'string') {
+        throw new Error(`The following maker config has a maker name that is not a string: ${JSON.stringify(resolvableTarget)}`);
+      }
+
       const MakerClass = requireSearch<typeof MakerImpl>(dir, [resolvableTarget.name]);
       if (!MakerClass) {
         throw new Error(`Could not find module with name: ${resolvableTarget.name}. Make sure it's listed in the devDependencies of your package.json`);

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

@@ -3,29 +3,56 @@ import * as path from 'path';
 import proxyquire from 'proxyquire';
 
 import { MakeOptions } from '../../src/api';
+import make from '../../src/api/make';
 
 describe('make', () => {
-  let make: (opts: MakeOptions) => Promise<any[]>;
-  beforeEach(() => {
-    const electronPath = path.resolve(__dirname, 'node_modules/electron');
-    make = proxyquire.noCallThru().load('../../src/api/make', {
-      '../util/electron-version': {
-        getElectronModulePath: () => Promise.resolve(electronPath),
-        getElectronVersion: () => Promise.resolve('1.0.0'),
-      },
-    }).default;
-  });
+  const fixtureDir = path.resolve(__dirname, '..', 'fixture');
+
   describe('overrideTargets inherits from forge config', () => {
+    let stubbedMake: (opts: MakeOptions) => Promise<any[]>;
+
+    before(() => {
+      const electronPath = path.resolve(__dirname, 'node_modules/electron');
+      stubbedMake = proxyquire.noCallThru().load('../../src/api/make', {
+        '../util/electron-version': {
+          getElectronModulePath: () => Promise.resolve(electronPath),
+          getElectronVersion: () => Promise.resolve('1.0.0'),
+        },
+      }).default;
+    });
+
     it('passes config properly', async () => {
-      const results = await make({
+      const results = await stubbedMake({
         arch: 'x64',
-        dir: path.resolve(__dirname, '..', 'fixture', 'app-with-custom-maker-config'),
+        dir: path.join(fixtureDir, 'app-with-custom-maker-config'),
         overrideTargets: ['../custom-maker'],
         platform: 'linux',
         skipPackage: true,
       });
 
       expect(results[0].artifacts).to.deep.equal(['from config']);
     });
+
+    after(() => proxyquire.callThru());
+  });
+
+  describe('maker config validation', () => {
+    it('throws an error if the name is missing', async () => {
+      await expect(make({
+        arch: 'x64',
+        dir: path.join(fixtureDir, 'maker-sans-name'),
+        platform: 'linux',
+        skipPackage: true,
+      })).to.eventually.be.rejectedWith(/^The following maker config is missing a maker name:/);
+    });
+
+    it('throws an error if the name is not a string', async () => {
+      await expect(make({
+        arch: 'x64',
+        dir: path.join(fixtureDir, 'maker-name-wrong-type'),
+        platform: 'linux',
+        skipPackage: true,
+      })).to.eventually.be.rejectedWith(/^The following maker config has a maker name that is not a string:/);
+    });
   });
 });

packages/api/core/test/fixture/maker-name-wrong-type/package.json

@@ -0,0 +1,9 @@
+{
+  "config": {
+    "forge": {
+      "makers": [{
+        "name": 1
+      }]
+    }
+  }
+}

packages/api/core/test/fixture/maker-sans-name/package.json

@@ -0,0 +1,7 @@
+{
+  "config": {
+    "forge": {
+      "makers": [{}]
+    }
+  }
+}