refactor(core): avoid forEach and increase test coverage for forge-co…

…nfig (#1395)

Also fixes an edge case with config templates.

packages/api/core/src/util/forge-config.ts

@@ -20,14 +20,13 @@ const proxify = <T extends object>(
  newObject = [] as any;
  }
 
- Object.keys(object).forEach((key) => {
- const val = (object as any)[key];
+ for (const [key, val] of Object.entries(object)) {
  if (typeof val === 'object' && key !== 'pluginInterface' && !(val instanceof RegExp)) {
  (newObject as any)[key] = proxify(buildIdentifier, (object as any)[key], `${envPrefix}_${underscoreCase(key)}`);
  } else {
  (newObject as any)[key] = (object as any)[key];
  }
- });
+ }
 
  return new Proxy<T>(newObject, {
  get(target, name, receiver) {
@@ -82,14 +81,28 @@ export function fromBuildIdentifier<T>(map: { [key: string]: T | undefined }) {
  };
 }
 
-async function forgeConfigIsValidFilePath(dir: string, forgeConfig: string | ForgeConfig) {
+export async function forgeConfigIsValidFilePath(dir: string, forgeConfig: string | ForgeConfig) {
  return typeof forgeConfig === 'string'
  && (
  await fs.pathExists(path.resolve(dir, forgeConfig))
  || fs.pathExists(path.resolve(dir, `${forgeConfig}.js`))
  );
 }
 
+export function renderConfigTemplate(dir: string, templateObj: any, obj: any) {
+ for (const [key, value] of Object.entries(obj)) {
+ if (typeof value === 'object' && value !== null) {
+ renderConfigTemplate(dir, templateObj, value);
+ } else if (typeof value === 'string') {
+ obj[key] = _template(value)(templateObj);
+ if (obj[key].startsWith('require:')) {
+ // eslint-disable-next-line global-require, import/no-dynamic-require
+ obj[key] = require(path.resolve(dir, obj[key].substr(8)));
+ }
+ }
+ }
+}
+
 export default async (dir: string) => {
  const packageJSON = await readRawPackageJson(dir);
  let forgeConfig: ForgeConfig | string | null = (packageJSON.config && packageJSON.config.forge)
@@ -126,21 +139,7 @@ export default async (dir: string) => {
  };
 
  const templateObj = { ...packageJSON, year: (new Date()).getFullYear() };
- const template = (obj: any) => {
- Object.keys(obj).forEach((objKey) => {
- if (typeof obj[objKey] === 'object' && obj !== null) {
- template(obj[objKey]);
- } else if (typeof obj[objKey] === 'string') {
- obj[objKey] = _template(obj[objKey])(templateObj);
- if (obj[objKey].startsWith('require:')) {
- // eslint-disable-next-line global-require, import/no-dynamic-require
- obj[objKey] = require(path.resolve(dir, obj[objKey].substr(8)));
- }
- }
- });
- };
-
- template(forgeConfig);
+ renderConfigTemplate(dir, templateObj, forgeConfig);
 
  forgeConfig.pluginInterface = new PluginInterface(dir, forgeConfig);
 

packages/api/core/test/fast/forge-config_spec.ts

@@ -1,7 +1,7 @@
 import { expect } from 'chai';
 import path from 'path';
 
-import findConfig from '../../src/util/forge-config';
+import findConfig, { forgeConfigIsValidFilePath, renderConfigTemplate, setInitialForgeConfig } from '../../src/util/forge-config';
 
 const defaults = {
  packagerConfig: {},
@@ -12,7 +12,21 @@ const defaults = {
 };
 
 describe('forge-config', () => {
- it('should resolve the object in package.json with defaults if one exists', async () => {
+ it('should fail if the config is not an object or requirable path', async () => {
+ await expect(findConfig(path.resolve(__dirname, '../fixture/bad_forge_config'))).to.eventually.be.rejectedWith('Expected packageJSON.config.forge to be an object or point to a requirable JS file');
+ });
+
+ it('should fail if the external config is not parseable', async () => {
+ await expect(findConfig(path.resolve(__dirname, '../fixture/bad_external_forge_config'))).to.eventually.be.rejectedWith(/bad.js: Unexpected token/);
+ });
+
+ it('should be set to the defaults if no Forge config is specified in package.json', async () => {
+ const config = await findConfig(path.resolve(__dirname, '../fixture/no_forge_config'));
+ delete config.pluginInterface;
+ expect(config).to.deep.equal(defaults);
+ });
+
+ it('should resolve the object in package.json with defaults if one exists', async () => {
  const config = await findConfig(path.resolve(__dirname, '../fixture/dummy_app'));
  delete config.pluginInterface;
  expect(config).to.be.deep.equal({
@@ -126,4 +140,70 @@ describe('forge-config', () => {
  expect(conf.regexp.test('foo')).to.equal(true, 'regexp should match foo');
  expect(conf.regexp.test('bar')).to.equal(false, 'regexp should not match bar');
  });
+
+ describe('forgeConfigIsValidFilePath', () => {
+ it('succeeds for a file extension-less path', async () => {
+ await expect(forgeConfigIsValidFilePath(path.resolve(__dirname, '../fixture/dummy_js_conf/'), 'forge.different.config')).to.eventually.equal(true);
+ });
+
+ it('fails when a file is nonexistent', async () => {
+ await expect(forgeConfigIsValidFilePath(path.resolve(__dirname, '../fixture/dummy_js_conf/'), 'forge.nonexistent.config')).to.eventually.equal(false);
+ });
+ });
+
+ describe('renderConfigTemplate', () => {
+ it('should import a JS file when a string starts with "require:"', () => {
+ const dir = path.resolve(__dirname, '../fixture/dummy_js_conf');
+ const config = {
+ foo: 'require:foo',
+ };
+ renderConfigTemplate(dir, {}, config);
+ expect(config.foo).to.deep.equal({
+ bar: {
+ baz: 'quux',
+ },
+ });
+ });
+ });
+
+ describe('setInitialForgeConfig', () => {
+ it('should normalize hyphens in maker names to underscores', () => {
+ const packageJSON = {
+ name: 'foo-bar',
+ config: {
+ forge: {
+ makers: [
+ {
+ name: '@electron-forge/maker-test',
+ config: {
+ name: 'will be overwritten',
+ },
+ },
+ ],
+ },
+ },
+ };
+ setInitialForgeConfig(packageJSON);
+ expect(packageJSON.config.forge.makers[0].config.name).to.equal('foo_bar');
+ });
+
+ it('should handle when package.json name is not set', () => {
+ const packageJSON = {
+ config: {
+ forge: {
+ makers: [
+ {
+ name: '@electron-forge/maker-test',
+ config: {
+ name: 'will be overwritten',
+ },
+ },
+ ],
+ },
+ },
+ };
+ setInitialForgeConfig(packageJSON);
+ expect(packageJSON.config.forge.makers[0].config.name).to.equal('');
+ });
+ });
 });

packages/api/core/test/fixture/bad_external_forge_config/bad.js

@@ -0,0 +1 @@
+}

packages/api/core/test/fixture/bad_external_forge_config/package.json

@@ -0,0 +1,5 @@
+{
+ "config": {
+ "forge": "bad.js"
+ }
+}

packages/api/core/test/fixture/bad_forge_config/package.json

@@ -0,0 +1,5 @@
+{
+ "config": {
+ "forge": 1
+ }
+}

packages/api/core/test/fixture/dummy_js_conf/foo.js

@@ -0,0 +1,5 @@
+module.exports = {
+ bar: {
+ baz: 'quux',
+ },
+}

packages/api/core/test/fixture/no_forge_config/package.json

@@ -0,0 +1 @@
+{}