diff --git a/src/builders.ts b/src/builders.ts index fbe6446cc..4bfc3995a 100644 --- a/src/builders.ts +++ b/src/builders.ts @@ -1,6 +1,6 @@ -import { Builder } from './types/package'; +import { Builders } from './types/package'; -const builders: Builder = { +const builders: Builders = { src: { alias: 's', describe: 'Source file', diff --git a/src/cli.ts b/src/cli.ts index 73116f746..37641d001 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -1,6 +1,7 @@ import yargs from 'yargs/yargs'; import { SnapsCliGlobals } from './types/package'; -import { assignGlobals, sanitizeInputs } from './utils'; +import { applyConfig, sanitizeInputs, setSnapGlobals } from './utils'; +import builders from './builders'; declare global { // eslint-disable-next-line @typescript-eslint/no-namespace @@ -25,26 +26,21 @@ export function cli(argv: string[], commands: any): void { .command(commands) - .option('verboseErrors', { - alias: ['v', 'verboseErrors'], - type: 'boolean', - describe: 'Display original errors', - required: false, - default: false, - }) + .option('verboseErrors', builders.verboseErrors) - .option('suppressWarnings', { - alias: ['sw', 'suppressWarnings'], - type: 'boolean', - describe: 'Suppress warnings', - required: false, - default: false, - }) + .option('suppressWarnings', builders.suppressWarnings) .strict() - .middleware((yargsArgv) => { - assignGlobals(yargsArgv); + .middleware(async (yargsArgv) => { + // Some globals affect error logging, and applyConfig may error. + // Therefore, we set globals from the yargs argv object before applying + // defaults from the config, and then set the globals again after, to + // ensure that inline options are preferred over any config files. + // This is ugly, but cheap. + setSnapGlobals(yargsArgv); + await applyConfig(yargsArgv); + setSnapGlobals(yargsArgv); sanitizeInputs(yargsArgv); }) diff --git a/src/main.ts b/src/main.ts index 5a7791857..770908b60 100755 --- a/src/main.ts +++ b/src/main.ts @@ -1,6 +1,5 @@ #!/usr/bin/env node import { cli } from './cli'; -import { applyConfig } from './utils'; import commands from './cmds'; global.snaps = { @@ -12,7 +11,6 @@ global.snaps = { main(); // application -async function main(): Promise { - await applyConfig(); +function main(): void { cli(process.argv, commands); } diff --git a/src/types/package.d.ts b/src/types/package.d.ts index e5da05af2..e95222066 100644 --- a/src/types/package.d.ts +++ b/src/types/package.d.ts @@ -19,20 +19,19 @@ export interface NodePackageManifest { web3Wallet?: ManifestWalletProperty; } -export interface Builder { - [key: string]: Options; - src: Options; - dist: Options; - bundle: Options; - root: Options; - port: Options; - sourceMaps: Options; - stripComments: Options; - outfileName: Options; - manifest: Options; - populate: Options; - eval: Options; - verboseErrors: Options; - suppressWarnings: Options; - environment: Options; +export interface Builders { + readonly src: Readonly; + readonly dist: Readonly; + readonly bundle: Readonly; + readonly root: Readonly; + readonly port: Readonly; + readonly sourceMaps: Readonly; + readonly stripComments: Readonly; + readonly outfileName: Readonly; + readonly manifest: Readonly; + readonly populate: Readonly; + readonly eval: Readonly; + readonly verboseErrors: Readonly; + readonly suppressWarnings: Readonly; + readonly environment: Readonly; } diff --git a/src/utils/misc.ts b/src/utils/misc.ts index 0dbab07c1..205f55205 100644 --- a/src/utils/misc.ts +++ b/src/utils/misc.ts @@ -1,4 +1,4 @@ -import yargs from 'yargs'; +import { Arguments } from 'yargs'; export const permRequestKeys = [ '@context', @@ -20,10 +20,9 @@ export const CONFIG_PATHS = [ * * @param {Argument} argv - arguments as an object generated by yargs */ -export function assignGlobals(argv: yargs.Arguments<{ - verboseErrors: boolean; -} & { - suppressWarnings: boolean; +export function setSnapGlobals(argv: Arguments<{ + verboseErrors: unknown; + suppressWarnings: unknown; }>) { if (['w', 'watch'].includes(argv._[0] as string)) { global.snaps.isWatching = true; @@ -43,11 +42,7 @@ export function assignGlobals(argv: yargs.Arguments<{ * * @param {Argument} argv - arguments as an object generated by yargs */ -export function sanitizeInputs(argv: yargs.Arguments<{ - verboseErrors: boolean; -} & { - suppressWarnings: boolean; -}>) { +export function sanitizeInputs(argv: Arguments) { Object.keys(argv).forEach((key) => { if (typeof argv[key] === 'string') { if (argv[key] === './') { diff --git a/src/utils/snap-config.ts b/src/utils/snap-config.ts index b03db57f5..e5460d01a 100644 --- a/src/utils/snap-config.ts +++ b/src/utils/snap-config.ts @@ -1,4 +1,5 @@ import { promises as fs } from 'fs'; +import { Arguments } from 'yargs'; import builders from '../builders'; import { logError } from './misc'; import { CONFIG_PATHS } from '.'; @@ -9,7 +10,7 @@ const INVALID_CONFIG_FILE = 'Invalid config file.'; * Attempts to read the config file and apply the config to * globals. */ -export async function applyConfig(): Promise { +export async function applyConfig(argv: Arguments): Promise { let pkg: any; // first, attempt to read and apply config from package.json @@ -17,21 +18,21 @@ export async function applyConfig(): Promise { pkg = JSON.parse(await fs.readFile('package.json', 'utf8')); if (pkg.main) { - builders.src.default = pkg.main; + argv.src = pkg.main; } if (pkg.web3Wallet) { const { bundle } = pkg.web3Wallet; if (bundle?.local) { const { local: bundlePath } = bundle; - builders.bundle.default = bundlePath; + argv.bundle = bundlePath; let dist: string; if (bundlePath.indexOf('/') === -1) { dist = '.'; } else { dist = bundlePath.substr(0, bundlePath.indexOf('/') + 1); } - builders.dist.default = dist; + argv.dist = dist; } } } catch (err) { @@ -67,7 +68,7 @@ export async function applyConfig(): Promise { if (cfg && typeof cfg === 'object' && !Array.isArray(cfg)) { for (const key of Object.keys(cfg)) { if (Object.hasOwnProperty.call(builders, key)) { - builders[key].default = cfg[key]; + argv[key] = cfg[key]; } else { logError( `Error: Encountered unrecognized config file property "${key}" in config file "${usedConfigPath as string}".`, diff --git a/test/utils/misc.test.js b/test/utils/misc.test.js index 3721f4a99..43c88f147 100644 --- a/test/utils/misc.test.js +++ b/test/utils/misc.test.js @@ -1,4 +1,4 @@ -const { trimPathString, logError, logWarning, sanitizeInputs, assignGlobals } = require('../../dist/src/utils/misc'); +const { trimPathString, logError, logWarning, sanitizeInputs, setSnapGlobals } = require('../../dist/src/utils/misc'); describe('misc', () => { global.snaps = { @@ -115,16 +115,16 @@ describe('misc', () => { delete global.snaps; }); - describe('assignGlobals', () => { + describe('setSnapGlobals', () => { it('sets global variables correctly', () => { - assignGlobals(exampleArgv); + setSnapGlobals(exampleArgv); expect(global.snaps.isWatching).toStrictEqual(true); expect(global.snaps.verboseErrors).toStrictEqual(true); expect(global.snaps.suppressWarnings).toStrictEqual(true); }); it('doesnt set global variables incorrectly', () => { - assignGlobals(defaultArgv); + setSnapGlobals(defaultArgv); expect(global.snaps.isWatching).toStrictEqual(false); expect(global.snaps.verboseErrors).toStrictEqual(false); expect(global.snaps.suppressWarnings).toStrictEqual(false); diff --git a/test/utils/snap-config.test.js b/test/utils/snap-config.test.js index 6171434aa..50d3682f7 100644 --- a/test/utils/snap-config.test.js +++ b/test/utils/snap-config.test.js @@ -3,11 +3,6 @@ const { default: builders } = require('../../dist/src/builders'); const { applyConfig } = require('../../dist/src/utils/snap-config'); const misc = require('../../dist/src/utils/misc'); -const originalBuilders = Object.keys(builders).reduce((snapshot, key) => { - snapshot[key] = builders[key]; - return builders[key]; -}, {}); - const getDefaultWeb3Wallet = () => { return { 'bundle': { @@ -37,6 +32,22 @@ const getDefaultSnapConfig = () => { }; }; +const getArgv = ({ + bundle = builders.bundle.default, + dist = builders.dist.default, + outfileName = builders.outfileName.default, + port = builders.port.default, + src = builders.src.default, +} = {}) => { + return { + bundle, + dist, + outfileName, + port, + src, + }; +}; + describe('snap-config', () => { let fsMock; @@ -59,9 +70,6 @@ describe('snap-config', () => { }); afterEach(() => { - Object.keys(originalBuilders).forEach((key) => { - builders[key] = originalBuilders[key]; - }); fsMock.mockRestore(); fsMock = null; delete global.snaps; @@ -75,10 +83,12 @@ describe('snap-config', () => { return {}; }); - await applyConfig(); - expect(builders.src.default).toStrictEqual('index.js'); - expect(builders.bundle.default).toStrictEqual('dist/foo.js'); - expect(builders.dist.default).toStrictEqual('dist/'); + const argv = getArgv(); + await applyConfig(argv); + + expect(argv.src).toStrictEqual(builders.src.default); + expect(argv.bundle).toStrictEqual('dist/foo.js'); + expect(argv.dist).toStrictEqual('dist/'); }); it('sets web3wallet with no local property correctly', async () => { @@ -98,10 +108,12 @@ describe('snap-config', () => { return {}; }); - await applyConfig(); - expect(builders.src.default).toStrictEqual(main); - expect(builders.bundle.default).toStrictEqual(builders.bundle.default); - expect(builders.dist.default).toStrictEqual(builders.dist.default); + const argv = getArgv(); + await applyConfig(argv); + + expect(argv.src).toStrictEqual(main); + expect(argv.bundle).toStrictEqual(builders.bundle.default); + expect(argv.dist).toStrictEqual(builders.dist.default); }); it('sets dist field correctly in edge case', async () => { @@ -122,10 +134,12 @@ describe('snap-config', () => { return {}; }); - await applyConfig(); - expect(builders.src.default).toStrictEqual(main); - expect(builders.bundle.default).toStrictEqual(web3Wallet.bundle.local); - expect(builders.dist.default).toStrictEqual('.'); + const argv = getArgv(); + await applyConfig(argv); + + expect(argv.src).toStrictEqual(main); + expect(argv.bundle).toStrictEqual(web3Wallet.bundle.local); + expect(argv.dist).toStrictEqual('.'); }); it('package.json read error is handled correctly', async () => { @@ -137,7 +151,7 @@ describe('snap-config', () => { throw new Error('foo'); }); - await applyConfig(); + await applyConfig(getArgv()); expect(mockLogError).toHaveBeenCalled(); expect(process.exit).toHaveBeenCalledWith(1); }); @@ -149,9 +163,11 @@ describe('snap-config', () => { .mockImplementationOnce(async () => getPackageJson()) .mockImplementationOnce(async () => getDefaultSnapConfig()); - await applyConfig(); - expect(builders.port.default).toStrictEqual(8084); - expect(builders.outfileName.default).toStrictEqual('test.js'); + const argv = getArgv(); + await applyConfig(argv); + + expect(argv.port).toStrictEqual(8084); + expect(argv.outfileName).toStrictEqual('test.js'); }); it('config file read error is handled correctly', async () => { @@ -164,7 +180,7 @@ describe('snap-config', () => { throw new Error('foo'); }); - await applyConfig(); + await applyConfig(getArgv()); expect(mockLogError).toHaveBeenCalled(); expect(process.exit).toHaveBeenCalledWith(1); }); @@ -177,7 +193,7 @@ describe('snap-config', () => { .mockImplementationOnce(async () => getPackageJson()) .mockImplementationOnce(async () => 'foo'); - await applyConfig(); + await applyConfig(getArgv()); expect(mockLogError).toHaveBeenCalled(); expect(process.exit).toHaveBeenCalledWith(1); }); @@ -192,37 +208,9 @@ describe('snap-config', () => { return { foo: 'bar' }; }); - await applyConfig(); + await applyConfig(getArgv()); expect(mockLogError).toHaveBeenCalled(); expect(process.exit).toHaveBeenCalledWith(1); }); }); - - describe('applyConfig', () => { - it('sets default values correctly', async () => { - fsMock = jest.spyOn(fs, 'readFile') - .mockImplementationOnce(async () => getPackageJson()) - .mockImplementationOnce(async () => { - return {}; - }); - - await applyConfig(); - expect(builders.src.default).toStrictEqual('index.js'); - expect(builders.bundle.default).toStrictEqual('dist/foo.js'); - expect(builders.dist.default).toStrictEqual('dist/'); - }); - - it('config file overrides package.json fields', async () => { - const customConfig = { - 'src': 'custom.js', - }; - - fsMock = jest.spyOn(fs, 'readFile') - .mockImplementationOnce(async () => getPackageJson()) - .mockImplementationOnce(async () => customConfig); - - await applyConfig(); - expect(builders.src.default).toStrictEqual('custom.js'); - }); - }); });