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

@@ -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();
   }

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) {

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`;

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<string, string>;
+    ) as unknown as Record<string, string>;
     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<string, string>;
+      ) as unknown as Record<string, string>;
       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<string, string>;
+    ) as unknown as Record<string, string>;
     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<string, string>;
+      ) as unknown as Record<string, string>;
       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<string, boolean>;
     expect(defaultArgs[arg]).toBe(false);
 
     const args = parseArgs(
       initArgs(['https://google.com', `--${arg}`]),
-    ) as Record<string, boolean>;
+    ) as unknown as Record<string, boolean>;
     expect(args[arg]).toBe(true);
     if (shortArg) {
       const argsShort = parseArgs(
         initArgs(['https://google.com', `-${shortArg}`]),
-      ) as Record<string, boolean>;
+      ) as unknown as Record<string, boolean>;
       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<string, boolean>;
       expect(defaultArgs[arg]).toBe(false);
       expect(defaultArgs[inverse]).toBe(true);
 
       const args = parseArgs(
         initArgs(['https://google.com', `--${arg}`]),
-      ) as Record<string, boolean>;
+      ) as unknown as Record<string, boolean>;
       expect(args[arg]).toBe(true);
       expect(args[inverse]).toBe(false);
 
       if (shortArg) {
         const argsShort = parseArgs(
           initArgs(['https://google.com', `-${shortArg}`]),
-        ) as Record<string, boolean>;
+        ) as unknown as Record<string, boolean>;
         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<string, number>;
+    ) as unknown as Record<string, number>;
     expect(args[arg]).toBe(value);
 
     const badArgs = parseArgs(
       initArgs(['https://google.com', `--${arg}`, 'abcd']),
-    ) as Record<string, number>;
+    ) as unknown as Record<string, number>;
     expect(badArgs[arg]).toBeNaN();
 
     if (shortArg) {
       const shortArgs = parseArgs(
         initArgs(['https://google.com', `-${shortArg}`, `${value}`]),
-      ) as Record<string, number>;
+      ) as unknown as Record<string, number>;
       expect(shortArgs[arg]).toBe(value);
 
       const badShortArgs = parseArgs(
         initArgs(['https://google.com', `-${shortArg}`, 'abcd']),
-      ) as Record<string, number>;
+      ) as unknown as Record<string, number>;
       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<string, number>;
+    ) as unknown as Record<string, number>;
     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');
   });
 });

src/cli.ts

@@ -233,7 +233,7 @@ export function initArgs(argv: string[]): yargs.Argv<RawOptions> {
       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<RawOptions> {
   // Do this now to go ahead and get any errors out of the way
   args.argv;
 
-  return args;
+  return args as yargs.Argv<RawOptions>;
 }
 
 function decorateYargOptionGroup(value: string): string {

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<void> {
   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);

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,

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;

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(

src/options/optionsMain.ts

@@ -100,7 +100,7 @@ export async function getOptions(rawOptions: RawOptions): Promise<AppOptions> {
       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,