Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): allow mutating packageJSON on load #513

Merged
merged 1 commit into from
May 27, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/api/core/src/api/import.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { deps, devDeps, exactDevDeps } from './init-scripts/init-npm';
import { setInitialForgeConfig } from '../util/forge-config';
import { info, warn } from '../util/messages';
import installDepList from '../util/install-dependencies';
import readPackageJSON from '../util/read-package-json';
import { readRawPackageJson } from '../util/read-package-json';

const d = debug('electron-forge:import');

Expand Down Expand Up @@ -72,7 +72,7 @@ export default async ({

await initGit(dir);

let packageJSON = await readPackageJSON(dir);
let packageJSON = await readRawPackageJson(dir);
if (packageJSON.config && packageJSON.config.forge) {
warn(interactive, 'It looks like this project is already configured for "electron-forge"'.green);
if (typeof shouldContinueOnExisting === 'function') {
Expand Down Expand Up @@ -161,14 +161,14 @@ export default async ({
await installDepList(dir, exactDevDeps, true, true);
});

packageJSON = await readPackageJSON(dir);
packageJSON = await readRawPackageJson(dir);

if (!packageJSON.version) {
warn(interactive, 'Please set the "version" in your application\'s package.json'.yellow);
}

packageJSON.config = packageJSON.config || {};
const templatePackageJSON = await readPackageJSON(path.resolve(__dirname, '../../tmpl'));
const templatePackageJSON = await readRawPackageJson(path.resolve(__dirname, '../../tmpl'));
packageJSON.config.forge = templatePackageJSON.config.forge;
setInitialForgeConfig(packageJSON);

Expand Down
4 changes: 2 additions & 2 deletions packages/api/core/src/api/init-scripts/init-npm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import username from 'username';

import { setInitialForgeConfig } from '../../util/forge-config';
import installDepList from '../../util/install-dependencies';
import readPackageJSON from '../../util/read-package-json';
import { readRawPackageJson } from '../../util/read-package-json';

const d = debug('electron-forge:init:npm');

Expand All @@ -22,7 +22,7 @@ export const exactDevDeps = ['electron'];

export default async (dir: string) => {
await asyncOra('Initializing NPM Module', async () => {
const packageJSON = await readPackageJSON(path.resolve(__dirname, '../../../tmpl'));
const packageJSON = await readRawPackageJson(path.resolve(__dirname, '../../../tmpl'));
packageJSON.productName = packageJSON.name = path.basename(dir).toLowerCase();
packageJSON.author = await username();
setInitialForgeConfig(packageJSON);
Expand Down
15 changes: 5 additions & 10 deletions packages/api/core/src/api/make.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ import fs from 'fs-extra';
import path from 'path';

import getForgeConfig from '../util/forge-config';
import runHook from '../util/hook';
import { runHook, runMutatingHook } from '../util/hook';
import { info, warn } from '../util/messages';
import parseArchs from '../util/parse-archs';
import readPackageJSON from '../util/read-package-json';
import { readMutatedPackageJson } from '../util/read-package-json';
import resolveDir from '../util/resolve-dir';
import getCurrentOutDir from '../util/out-dir';
import getElectronVersion from '../util/electron-version';
Expand Down Expand Up @@ -147,9 +147,9 @@ export default async ({

info(interactive, `Making for the following targets: ${`${targets.map((t, i) => makers[i].name).join(', ')}`.cyan}`);

const packageJSON = await readPackageJSON(dir);
const packageJSON = await readMutatedPackageJson(dir, forgeConfig);
const appName = forgeConfig.packagerConfig.name || packageJSON.productName || packageJSON.name;
let outputs: ForgeMakeResult[] = [];
const outputs: ForgeMakeResult[] = [];

await runHook(forgeConfig, 'preMake');

Expand Down Expand Up @@ -197,12 +197,7 @@ export default async ({
}
}

const result = await runHook(forgeConfig, 'postMake', outputs) as ForgeMakeResult[] | undefined;
// If the postMake hooks modifies the locations / names of the outputs it must return
// the new locations so that the publish step knows where to look
if (Array.isArray(result)) {
outputs = result;
}

return outputs;
return await runMutatingHook(forgeConfig, 'postMake', outputs);
};
10 changes: 5 additions & 5 deletions packages/api/core/src/api/package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import pify from 'pify';
import packager from 'electron-packager';

import getForgeConfig from '../util/forge-config';
import runHook from '../util/hook';
import { runHook } from '../util/hook';
import { warn } from '../util/messages';
import readPackageJSON from '../util/read-package-json';
import { readMutatedPackageJson } from '../util/read-package-json';
import rebuildHook from '../util/rebuild';
import requireSearch from '../util/require-search';
import resolveDir from '../util/resolve-dir';
Expand Down Expand Up @@ -96,13 +96,13 @@ export default async ({
}
dir = resolvedDir;

const packageJSON = await readPackageJSON(dir);
const forgeConfig = await getForgeConfig(dir);
const packageJSON = await readMutatedPackageJson(dir, forgeConfig);

if (!packageJSON.main) {
throw 'packageJSON.main must be set to a valid entry point for your Electron app';
}

const forgeConfig = await getForgeConfig(dir);
const calculatedOutDir = outDir || getCurrentOutDir(dir, forgeConfig);
let packagerSpinner: OraImpl | null = null;

Expand Down Expand Up @@ -133,7 +133,7 @@ export default async ({
];

afterCopyHooks.push(async (buildPath, electronVersion, pPlatform, pArch, done) => {
const copiedPackageJSON = await readPackageJSON(buildPath);
const copiedPackageJSON = await readMutatedPackageJson(buildPath, forgeConfig);
if (copiedPackageJSON.config && copiedPackageJSON.config.forge) {
delete copiedPackageJSON.config.forge;
}
Expand Down
6 changes: 3 additions & 3 deletions packages/api/core/src/api/publish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import fs from 'fs-extra';
import path from 'path';

import getForgeConfig from '../util/forge-config';
import readPackageJSON from '../util/read-package-json';
import { readMutatedPackageJson } from '../util/read-package-json';
import resolveDir from '../util/resolve-dir';
import PublishState from '../util/publish-state';
import getCurrentOutDir from '../util/out-dir';
Expand Down Expand Up @@ -73,9 +73,9 @@ const publish = async ({
throw 'Can\'t resume a dry run and use the provided makeResults at the same time';
}

let packageJSON = await readPackageJSON(dir);

const forgeConfig = await getForgeConfig(dir);
let packageJSON = await readMutatedPackageJson(dir, forgeConfig);

const calculatedOutDir = outDir || getCurrentOutDir(dir, forgeConfig);
const dryRunDir = path.resolve(calculatedOutDir, 'publish-dry-run');

Expand Down
9 changes: 4 additions & 5 deletions packages/api/core/src/api/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import { StartOptions, ForgePlatform, ForgeArch } from '@electron-forge/shared-t
import { spawn, ChildProcess } from 'child_process';
import path from 'path';

import readPackageJSON from '../util/read-package-json';
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 { runHook } from '../util/hook';
import getElectronVersion from '../util/electron-version';

export { StartOptions };
Expand All @@ -32,14 +32,13 @@ export default async ({
dir = resolvedDir;
});

const packageJSON = await readPackageJSON(dir);
const forgeConfig = await getForgeConfig(dir);
const packageJSON = await readMutatedPackageJson(dir, forgeConfig);

if (!packageJSON.version) {
throw `Please set your application's 'version' in '${dir}/package.json'.`;
}

const forgeConfig = await getForgeConfig(dir);

await rebuild(
dir,
getElectronVersion(packageJSON),
Expand Down
8 changes: 4 additions & 4 deletions packages/api/core/src/util/forge-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import fs from 'fs-extra';
import path from 'path';
import _template from 'lodash.template';

import readPackageJSON from './read-package-json';
import { readRawPackageJson } from './read-package-json';
import PluginInterface from './plugin-interface';
import runHook from './hook';
import { runMutatingHook } from './hook';

const underscoreCase = (str: string) => str.replace(/(.)([A-Z][a-z]+)/g, '$1_$2').replace(/([a-z0-9])([A-Z])/g, '$1_$2').toUpperCase();

Expand Down Expand Up @@ -70,7 +70,7 @@ export function fromBuildIdentifier<T>(map: { [key: string]: T | undefined }) {
}

export default async (dir: string) => {
const packageJSON = await readPackageJSON(dir);
const packageJSON = await readRawPackageJson(dir);
let forgeConfig: ForgeConfig | string = packageJSON.config.forge;

if (typeof forgeConfig === 'string' && (await fs.pathExists(path.resolve(dir, forgeConfig)) || await fs.pathExists(path.resolve(dir, `${forgeConfig}.js`)))) {
Expand Down Expand Up @@ -109,7 +109,7 @@ export default async (dir: string) => {

forgeConfig.pluginInterface = new PluginInterface(dir, forgeConfig);

await runHook(forgeConfig, 'resolveForgeConfig', forgeConfig);
forgeConfig = await runMutatingHook(forgeConfig, 'resolveForgeConfig', forgeConfig);

return proxify<ForgeConfig>(forgeConfig.buildIdentifier || '', forgeConfig, 'ELECTRON_FORGE');
};
17 changes: 16 additions & 1 deletion packages/api/core/src/util/hook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import debug from 'debug';

const d = debug('electron-forge:hook');

export default async (forgeConfig: ForgeConfig, hookName: string, ...hookArgs: any[]) => {
export const runHook = async (forgeConfig: ForgeConfig, hookName: string, ...hookArgs: any[]) => {
const hooks = forgeConfig.hooks;
if (hooks) {
d(`hook triggered: ${hookName}`);
Expand All @@ -14,3 +14,18 @@ export default async (forgeConfig: ForgeConfig, hookName: string, ...hookArgs: a
}
await forgeConfig.pluginInterface.triggerHook(hookName, hookArgs);
};

export const runMutatingHook = async <T>(forgeConfig: ForgeConfig, hookName: string, item: T): Promise<T> => {
const hooks = forgeConfig.hooks;
if (hooks) {
d(`hook triggered: ${hookName}`);
if (typeof hooks[hookName] === 'function') {
d('calling mutating hook:', hookName, 'with item:', item);
const result = await hooks[hookName](forgeConfig, item);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why pass the forgeConfig into the hook first? This was a bit cryptic to find from the docs.

if (typeof result !== 'undefined') {
item = result;
}
}
}
return await forgeConfig.pluginInterface.triggerMutatingHook(hookName, item);
};
12 changes: 12 additions & 0 deletions packages/api/core/src/util/plugin-interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,18 @@ export default class PluginInterface implements IForgePluginInterface {
}
}

async triggerMutatingHook<T>(hookName: string, item: T) {
for (const plugin of this.plugins) {
if (typeof plugin.getHook === 'function') {
const hook = plugin.getHook(hookName);
if (hook) {
item = await hook(this.config, item);
}
}
}
return item;
}

async overrideStartLogic(opts: StartOptions) {
let newStartFn;
const claimed = [];
Expand Down
8 changes: 7 additions & 1 deletion packages/api/core/src/util/read-package-json.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import { ForgeConfig } from '@electron-forge/shared-types';
import fs from 'fs-extra';
import path from 'path';

export default async (dir: string) =>
import { runMutatingHook } from './hook';

export const readRawPackageJson = async (dir: string) =>
await fs.readJson(path.resolve(dir, 'package.json'));

export const readMutatedPackageJson = async (dir: string, forgeConfig: ForgeConfig) =>
runMutatingHook(forgeConfig, 'readPackageJson', await readRawPackageJson(dir));
9 changes: 7 additions & 2 deletions packages/api/core/src/util/resolve-dir.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import debug from 'debug';
import fs from 'fs-extra';
import path from 'path';
import readPackageJSON from './read-package-json';
import { readRawPackageJson } from './read-package-json';
import getElectronVersion from './electron-version';

const d = debug('electron-forge:project-resolver');

// FIXME: If we want getElectronVersion to be overridable by plugins
// and / or forge config then we need to be able to resolve
// the dir without calling getElectronVersion
export default async (dir: string) => {
let mDir = dir;
let prevDir;
Expand All @@ -14,8 +17,10 @@ export default async (dir: string) => {
const testPath = path.resolve(mDir, 'package.json');
d('searching for project in:', mDir);
if (await fs.pathExists(testPath)) {
const packageJSON = await readPackageJSON(mDir);
const packageJSON = await readRawPackageJson(mDir);

// TODO: Move this check to inside the forge config resolver and use
// mutatedPackageJson reader
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this for a different PR then?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup

const electronVersion = getElectronVersion(packageJSON);
if (electronVersion) {
if (!/[0-9]/.test(electronVersion[0])) {
Expand Down
44 changes: 31 additions & 13 deletions packages/api/core/test/fast/hook_spec.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,46 @@
import { ForgeConfig } from '@electron-forge/shared-types';
import { expect } from 'chai';
import { stub } from 'sinon';
import { stub, SinonStub } from 'sinon';

import runHook from '../../src/util/hook';
import { runHook, runMutatingHook } from '../../src/util/hook';

const fakeConfig = {
pluginInterface: {
triggerHook: async () => false,
triggerMutatingHook: async (_: any, item: any) => item,
},
} as any as ForgeConfig;

describe('runHook', () => {
it('should not error when running non existent hooks', async () => {
await runHook(Object.assign({}, fakeConfig), 'magic');
describe('hooks', () => {
describe('runHook', () => {
it('should not error when running non existent hooks', async () => {
await runHook(Object.assign({}, fakeConfig), 'magic');
});

it('should not error when running a hook that is not a function', async () => {
await runHook(Object.assign({ hooks: { myHook: 'abc' } }, fakeConfig), 'abc');
});

it('should run the hook if it is provided as a function', async () => {
const myStub = stub();
myStub.returns(Promise.resolve());
await runHook(Object.assign({ hooks: { myHook: myStub } }, fakeConfig), 'myHook');
expect(myStub.callCount).to.equal(1);
});
});

it('should not error when running a hook that is not a function', async () => {
await runHook(Object.assign({ hooks: { myHook: 'abc' } }, fakeConfig), 'abc');
});
describe('runMutatingHook', () => {
it('should return the input when running non existent hooks', async () => {
expect(await runMutatingHook(Object.assign({}, fakeConfig), 'magic', 'input')).to.equal('input');
});

it('should run the hook if it is provided as a function', async () => {
const myStub = stub();
myStub.returns(Promise.resolve());
await runHook(Object.assign({ hooks: { myHook: myStub } }, fakeConfig), 'myHook');
expect(myStub.callCount).to.equal(1);
it('should return the mutated input when returned from a hook', async () => {
fakeConfig.pluginInterface.triggerMutatingHook = stub().returnsArg(1);
const myStub = stub();
myStub.returns(Promise.resolve('magneto'));
const output = await runMutatingHook(Object.assign({ hooks: { myHook: myStub } }, fakeConfig), 'myHook', 'input');
expect(output).to.equal('magneto');
expect((fakeConfig.pluginInterface.triggerMutatingHook as SinonStub).firstCall.args[1]).to.equal('magneto');
});
});
});
4 changes: 3 additions & 1 deletion packages/api/core/test/fast/publish_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ describe('publish', () => {
publish = proxyquire.noCallThru().load('../../src/api/publish', {
'./make': async (...args: any[]) => makeStub(...args),
'../util/resolve-dir': async (dir: string) => resolveStub(dir),
'../util/read-package-json': () => Promise.resolve(require('../fixture/dummy_app/package.json')),
'../util/read-package-json': {
readMutatedPackageJson: () => Promise.resolve(require('../fixture/dummy_app/package.json')),
},
'../util/forge-config': async () => {
const config = await (require('../../src/util/forge-config').default(path.resolve(__dirname, '../fixture/dummy_app')));

Expand Down
Loading