From dc0e6cb68ffa05613073b9cc85646e02c5477606 Mon Sep 17 00:00:00 2001 From: Adam Weeden Date: Fri, 25 Jun 2021 16:03:23 -0400 Subject: [PATCH] Fix app keeping running in the background after closed (--tray false regression) (PR #1242) By tightening up typing and logic around tray options. --- app/src/helpers/windowHelpers.ts | 4 +-- src/build/buildIcon.ts | 2 +- src/build/buildNativefierApp.ts | 2 +- src/cli.test.ts | 45 ++++++++++++++----------------- src/cli.ts | 4 +-- src/integration-test.ts | 23 +++++++++------- src/options/fields/fields.test.ts | 2 +- src/options/model.ts | 6 +++-- src/options/optionsMain.test.ts | 10 ++++--- src/options/optionsMain.ts | 2 +- 10 files changed, 51 insertions(+), 49 deletions(-) diff --git a/app/src/helpers/windowHelpers.ts b/app/src/helpers/windowHelpers.ts index d8193d5..695ce1e 100644 --- a/app/src/helpers/windowHelpers.ts +++ b/app/src/helpers/windowHelpers.ts @@ -179,13 +179,13 @@ export function hideWindow( window: BrowserWindow, event: IpcMainEvent, fastQuit: boolean, - tray, + tray: 'true' | 'false' | 'start-in-tray', ): void { if (isOSX() && !fastQuit) { // this is called when exiting from clicking the cross button on the window event.preventDefault(); window.hide(); - } else if (!fastQuit && tray) { + } else if (!fastQuit && tray !== 'false') { event.preventDefault(); window.hide(); } diff --git a/src/build/buildIcon.ts b/src/build/buildIcon.ts index ad99c4c..fcc7de3 100644 --- a/src/build/buildIcon.ts +++ b/src/build/buildIcon.ts @@ -87,7 +87,7 @@ export function convertIconIfNecessary(options: AppOptions): void { const iconPath = convertToIcns(options.packager.icon); options.packager.icon = iconPath; } - if (options.nativefier.tray) { + if (options.nativefier.tray !== 'false') { convertToTrayIcon(options.packager.icon); } } catch (err: unknown) { diff --git a/src/build/buildNativefierApp.ts b/src/build/buildNativefierApp.ts index dc40b14..ba5fb50 100644 --- a/src/build/buildNativefierApp.ts +++ b/src/build/buildNativefierApp.ts @@ -44,7 +44,7 @@ async function copyIconsIfNecessary( options.packager.platform === 'darwin' || options.packager.platform === 'mas' ) { - if (options.nativefier.tray) { + if (options.nativefier.tray !== 'false') { //tray icon needs to be .png log.debug('Copying icon for tray application'); const trayIconFileName = `tray-icon.png`; diff --git a/src/cli.test.ts b/src/cli.test.ts index a0b0ba2..1751c52 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -141,7 +141,7 @@ describe('initArgs + parseArgs', () => { ])('test string arg %s', ({ arg, shortArg, value, isJsonString }) => { const args = parseArgs( initArgs(['https://google.com', `--${arg}`, value]), - ) as Record; + ) as unknown as Record; if (!isJsonString) { expect(args[arg]).toBe(value); } else { @@ -151,7 +151,7 @@ describe('initArgs + parseArgs', () => { if (shortArg) { const argsShort = parseArgs( initArgs(['https://google.com', `-${shortArg}`, value]), - ) as Record; + ) as unknown as Record; if (!isJsonString) { expect(argsShort[arg]).toBe(value); } else { @@ -172,7 +172,7 @@ describe('initArgs + parseArgs', () => { ])('limited choice arg %s', ({ arg, shortArg, value, badValue }) => { const args = parseArgs( initArgs(['https://google.com', `--${arg}`, value]), - ) as Record; + ) as unknown as Record; expect(args[arg]).toBe(value); // Mock console.error to not pollute the log with the yargs help text @@ -186,7 +186,7 @@ describe('initArgs + parseArgs', () => { if (shortArg) { const argsShort = parseArgs( initArgs(['https://google.com', `-${shortArg}`, value]), - ) as Record; + ) as unknown as Record; expect(argsShort[arg]).toBe(value); initArgs(['https://google.com', `-${shortArg}`, badValue]); @@ -224,20 +224,19 @@ describe('initArgs + parseArgs', () => { { arg: 'verbose', shortArg: '' }, { arg: 'widevine', shortArg: '' }, ])('test boolean arg %s', ({ arg, shortArg }) => { - const defaultArgs = parseArgs(initArgs(['https://google.com'])) as Record< - string, - boolean - >; + const defaultArgs = parseArgs( + initArgs(['https://google.com']), + ) as unknown as Record; expect(defaultArgs[arg]).toBe(false); const args = parseArgs( initArgs(['https://google.com', `--${arg}`]), - ) as Record; + ) as unknown as Record; expect(args[arg]).toBe(true); if (shortArg) { const argsShort = parseArgs( initArgs(['https://google.com', `-${shortArg}`]), - ) as Record; + ) as unknown as Record; expect(argsShort[arg]).toBe(true); } }); @@ -247,23 +246,22 @@ describe('initArgs + parseArgs', () => { ({ arg, shortArg }) => { const inverse = arg.startsWith('no-') ? arg.substr(3) : `no-${arg}`; - const defaultArgs = parseArgs(initArgs(['https://google.com'])) as Record< - string, - boolean - >; + const defaultArgs = parseArgs( + initArgs(['https://google.com']), + ) as unknown as Record; expect(defaultArgs[arg]).toBe(false); expect(defaultArgs[inverse]).toBe(true); const args = parseArgs( initArgs(['https://google.com', `--${arg}`]), - ) as Record; + ) as unknown as Record; expect(args[arg]).toBe(true); expect(args[inverse]).toBe(false); if (shortArg) { const argsShort = parseArgs( initArgs(['https://google.com', `-${shortArg}`]), - ) as Record; + ) as unknown as Record; expect(argsShort[arg]).toBe(true); expect(argsShort[inverse]).toBe(true); } @@ -283,23 +281,23 @@ describe('initArgs + parseArgs', () => { ])('test numeric arg %s', ({ arg, shortArg, value }) => { const args = parseArgs( initArgs(['https://google.com', `--${arg}`, `${value}`]), - ) as Record; + ) as unknown as Record; expect(args[arg]).toBe(value); const badArgs = parseArgs( initArgs(['https://google.com', `--${arg}`, 'abcd']), - ) as Record; + ) as unknown as Record; expect(badArgs[arg]).toBeNaN(); if (shortArg) { const shortArgs = parseArgs( initArgs(['https://google.com', `-${shortArg}`, `${value}`]), - ) as Record; + ) as unknown as Record; expect(shortArgs[arg]).toBe(value); const badShortArgs = parseArgs( initArgs(['https://google.com', `-${shortArg}`, 'abcd']), - ) as Record; + ) as unknown as Record; expect(badShortArgs[arg]).toBeNaN(); } }); @@ -312,7 +310,7 @@ describe('initArgs + parseArgs', () => { ])('test tray valyue %s', ({ arg, value }) => { const args = parseArgs( initArgs(['https://google.com', `--${arg}`, `${value}`]), - ) as Record; + ) as unknown as Record; if (value !== '') { expect(args[arg]).toBe(value); } else { @@ -321,10 +319,7 @@ describe('initArgs + parseArgs', () => { }); test('test tray value defaults to false', () => { - const args = parseArgs(initArgs(['https://google.com'])) as Record< - string, - number - >; + const args = parseArgs(initArgs(['https://google.com'])); expect(args.tray).toBe('false'); }); }); diff --git a/src/cli.ts b/src/cli.ts index 4f88bf0..f804bd3 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -233,7 +233,7 @@ export function initArgs(argv: string[]): yargs.Argv { default: 'false', description: "allow app to stay in system tray. If 'start-in-tray' is set as argument, don't show main window on first start", - type: 'string', + choices: ['true', 'false', 'start-in-tray'], }) .option('width', { defaultDescription: '1280', @@ -519,7 +519,7 @@ export function initArgs(argv: string[]): yargs.Argv { // Do this now to go ahead and get any errors out of the way args.argv; - return args; + return args as yargs.Argv; } function decorateYargOptionGroup(value: string): string { diff --git a/src/integration-test.ts b/src/integration-test.ts index 61d965f..323d34f 100644 --- a/src/integration-test.ts +++ b/src/integration-test.ts @@ -10,12 +10,12 @@ import { getLatestSafariVersion } from './infer/browsers/inferSafariVersion'; import { inferArch } from './infer/inferOs'; import { buildNativefierApp } from './main'; import { userAgent } from './options/fields/userAgent'; -import { NativefierOptions } from './options/model'; +import { NativefierOptions, RawOptions } from './options/model'; import { parseJson } from './utils/parseUtils'; async function checkApp( appRoot: string, - inputOptions: NativefierOptions, + inputOptions: RawOptions, ): Promise { const arch = inputOptions.arch ? (inputOptions.arch as string) : inferArch(); if (inputOptions.out !== undefined) { @@ -102,12 +102,13 @@ describe('Nativefier', () => { 'builds a Nativefier app for platform %s', async (platform) => { const tempDirectory = getTempDir('integtest'); - const options = { - platform, - targetUrl: 'https://google.com/', + const options: RawOptions = { + lang: 'en-US', out: tempDirectory, overwrite: true, - lang: 'en-US', + platform, + targetUrl: 'https://google.com/', + tray: 'false', }; const appPath = await buildNativefierApp(options); expect(appPath).not.toBeUndefined(); @@ -134,20 +135,22 @@ describe('Nativefier upgrade', () => { 'can upgrade a Nativefier app for platform/arch: %s', async (baseAppOptions) => { const tempDirectory = getTempDir('integtestUpgrade1'); - const options = { - targetUrl: 'https://google.com/', + const options: RawOptions = { + electronVersion: '11.2.3', out: tempDirectory, overwrite: true, - electronVersion: '11.2.3', + targetUrl: 'https://google.com/', + tray: 'false', ...baseAppOptions, }; const appPath = await buildNativefierApp(options); expect(appPath).not.toBeUndefined(); await checkApp(appPath as string, options); - const upgradeOptions = { + const upgradeOptions: RawOptions = { upgrade: appPath, overwrite: true, + tray: 'false', }; const upgradeAppPath = await buildNativefierApp(upgradeOptions); diff --git a/src/options/fields/fields.test.ts b/src/options/fields/fields.test.ts index b1e9a10..c720d4a 100644 --- a/src/options/fields/fields.test.ts +++ b/src/options/fields/fields.test.ts @@ -47,7 +47,7 @@ describe('fields', () => { showMenuBar: false, singleInstance: false, titleBarStyle: undefined, - tray: false, + tray: 'false', userAgent: undefined, userAgentHonest: false, verbose: false, diff --git a/src/options/model.ts b/src/options/model.ts index a6d4f44..f6495a0 100644 --- a/src/options/model.ts +++ b/src/options/model.ts @@ -1,6 +1,8 @@ import { CreateOptions } from 'asar'; import * as electronPackager from 'electron-packager'; +export type TrayValue = 'true' | 'false' | 'start-in-tray'; + export interface ElectronPackagerOptions extends electronPackager.Options { portable: boolean; platform?: string; @@ -50,7 +52,7 @@ export interface AppOptions { showMenuBar: boolean; singleInstance: boolean; titleBarStyle?: string; - tray: string | boolean; + tray: TrayValue; userAgent?: string; userAgentHonest: boolean; verbose: boolean; @@ -149,7 +151,7 @@ export type RawOptions = { singleInstance?: boolean; targetUrl?: string; titleBarStyle?: string; - tray?: string | boolean; + tray: TrayValue; upgrade?: string | boolean; upgradeFrom?: string; userAgent?: string; diff --git a/src/options/optionsMain.test.ts b/src/options/optionsMain.test.ts index c76eb26..533a200 100644 --- a/src/options/optionsMain.test.ts +++ b/src/options/optionsMain.test.ts @@ -1,7 +1,7 @@ import { getOptions, normalizePlatform } from './optionsMain'; import * as asyncConfig from './asyncConfig'; import { inferPlatform } from '../infer/inferOs'; -import { AppOptions } from './model'; +import { AppOptions, RawOptions } from './model'; let asyncConfigMock: jest.SpyInstance; const mockedAsyncConfig: AppOptions = { @@ -47,7 +47,7 @@ const mockedAsyncConfig: AppOptions = { showMenuBar: false, singleInstance: false, titleBarStyle: undefined, - tray: false, + tray: 'false', userAgent: undefined, userAgentHonest: false, verbose: false, @@ -74,8 +74,9 @@ beforeAll(() => { }); test('it should call the async config', async () => { - const params = { + const params: RawOptions = { targetUrl: 'https://example.com/', + tray: 'false', }; const result = await getOptions(params); expect(asyncConfigMock).toHaveBeenCalledWith( @@ -88,8 +89,9 @@ test('it should call the async config', async () => { }); test('it should set the accessibility prompt option to true by default', async () => { - const params = { + const params: RawOptions = { targetUrl: 'https://example.com/', + tray: 'false', }; const result = await getOptions(params); expect(asyncConfigMock).toHaveBeenCalledWith( diff --git a/src/options/optionsMain.ts b/src/options/optionsMain.ts index ad7b13e..67134c1 100644 --- a/src/options/optionsMain.ts +++ b/src/options/optionsMain.ts @@ -100,7 +100,7 @@ export async function getOptions(rawOptions: RawOptions): Promise { showMenuBar: rawOptions.showMenuBar ?? false, singleInstance: rawOptions.singleInstance ?? false, titleBarStyle: rawOptions.titleBarStyle, - tray: rawOptions.tray ?? false, + tray: rawOptions.tray ?? 'false', userAgent: rawOptions.userAgent, userAgentHonest: rawOptions.userAgentHonest ?? false, verbose: rawOptions.verbose ?? false,