From 8d83518f9ad4008e29f0c31ead7acbab28975eab Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Tue, 30 Apr 2019 13:55:33 -0700 Subject: [PATCH] refactor: make name a prop on app (#17701) Update app.name to be a property on app. --- atom/browser/api/atom_api_app.cc | 6 ++++-- default_app/main.ts | 4 ++-- docs/api/app.md | 13 +++++++++++++ docs/api/browser-window.md | 2 +- docs/api/crash-reporter.md | 2 +- docs/api/menu.md | 2 +- docs/api/modernization/property-updates.md | 2 +- lib/browser/api/app.ts | 5 +++-- lib/browser/api/menu-item-roles.js | 8 ++++---- lib/browser/crash-reporter-init.js | 2 +- lib/browser/init.ts | 6 +++--- spec-main/api-app-spec.ts | 18 +++++++++++++++++- spec/api-crash-reporter-spec.js | 2 +- spec/api-menu-item-spec.js | 6 +++--- spec/api-notification-dbus-spec.js | 4 ++-- 15 files changed, 57 insertions(+), 25 deletions(-) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index c7b8cc98da6b..7eb1fa8ce81e 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -1344,8 +1344,8 @@ void App::BuildPrototype(v8::Isolate* isolate, base::BindRepeating(&Browser::GetVersion, browser)) .SetMethod("setVersion", base::BindRepeating(&Browser::SetVersion, browser)) - .SetMethod("getName", base::BindRepeating(&Browser::GetName, browser)) - .SetMethod("setName", base::BindRepeating(&Browser::SetName, browser)) + .SetMethod("_getName", base::BindRepeating(&Browser::GetName, browser)) + .SetMethod("_setName", base::BindRepeating(&Browser::SetName, browser)) .SetMethod("isReady", base::BindRepeating(&Browser::is_ready, browser)) .SetMethod("whenReady", base::BindRepeating(&Browser::WhenReady, browser)) .SetMethod("addRecentDocument", @@ -1375,6 +1375,8 @@ void App::BuildPrototype(v8::Isolate* isolate, .SetProperty("badgeCount", base::BindRepeating(&Browser::GetBadgeCount, browser), base::BindRepeating(&Browser::SetBadgeCount, browser)) + .SetProperty("name", base::BindRepeating(&Browser::GetName, browser), + base::BindRepeating(&Browser::SetName, browser)) #if defined(OS_MACOSX) .SetMethod("hide", base::BindRepeating(&Browser::Hide, browser)) .SetMethod("show", base::BindRepeating(&Browser::Show, browser)) diff --git a/default_app/main.ts b/default_app/main.ts index 597f47ddd8b4..6d2ec90c045e 100644 --- a/default_app/main.ts +++ b/default_app/main.ts @@ -98,9 +98,9 @@ function loadApplicationPackage (packagePath: string) { app.setVersion(packageJson.version) } if (packageJson.productName) { - app.setName(packageJson.productName) + app.name = packageJson.productName } else if (packageJson.name) { - app.setName(packageJson.name) + app.name = packageJson.name } app._setDefaultAppPaths(packagePath) } diff --git a/docs/api/app.md b/docs/api/app.md index 289b52443777..ad65c58df178 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -661,12 +661,16 @@ to the npm modules spec. You should usually also specify a `productName` field, which is your application's full capitalized name, and which will be preferred over `name` by Electron. +**[Deprecated Soon](modernization/property-updates.md)** + ### `app.setName(name)` * `name` String Overrides the current application's name. +**[Deprecated Soon](modernization/property-updates.md)** + ### `app.getLocale()` Returns `String` - The current application locale. Possible return values are documented [here](locales.md). @@ -1366,3 +1370,12 @@ A `Boolean` property that returns `true` if the app is packaged, `false` otherw [Squirrel-Windows]: https://github.com/Squirrel/Squirrel.Windows [JumpListBeginListMSDN]: https://msdn.microsoft.com/en-us/library/windows/desktop/dd378398(v=vs.85).aspx [about-panel-options]: https://developer.apple.com/reference/appkit/nsapplication/1428479-orderfrontstandardaboutpanelwith?language=objc + +### `app.name` + +A `String` property that indicates the current application's name, which is the name in the application's `package.json` file. + +Usually the `name` field of `package.json` is a short lowercased name, according +to the npm modules spec. You should usually also specify a `productName` +field, which is your application's full capitalized name, and which will be +preferred over `name` by Electron. diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index c304e12c9526..775f18155dce 100644 --- a/docs/api/browser-window.md +++ b/docs/api/browser-window.md @@ -1318,7 +1318,7 @@ Change to indeterminate mode when progress > 1. On Linux platform, only supports Unity desktop environment, you need to specify the `*.desktop` file name to `desktopName` field in `package.json`. By default, -it will assume `app.getName().desktop`. +it will assume `{app.name}.desktop`. On Windows, a mode can be passed. Accepted values are `none`, `normal`, `indeterminate`, `error`, and `paused`. If you call `setProgressBar` without a diff --git a/docs/api/crash-reporter.md b/docs/api/crash-reporter.md index 4591771363b7..9f229864c806 100644 --- a/docs/api/crash-reporter.md +++ b/docs/api/crash-reporter.md @@ -44,7 +44,7 @@ The `crashReporter` module has the following methods: * `options` Object * `companyName` String * `submitURL` String - URL that crash reports will be sent to as POST. - * `productName` String (optional) - Defaults to `app.getName()`. + * `productName` String (optional) - Defaults to `app.name`. * `uploadToServer` Boolean (optional) - Whether crash reports should be sent to the server Default is `true`. * `ignoreSystemCrashHandler` Boolean (optional) - Default is `false`. diff --git a/docs/api/menu.md b/docs/api/menu.md index 6a2e3c513e86..0727d1476eef 100644 --- a/docs/api/menu.md +++ b/docs/api/menu.md @@ -155,7 +155,7 @@ const { app, Menu } = require('electron') const template = [ // { role: 'appMenu' } ...(process.platform === 'darwin' ? [{ - label: app.getName(), + label: app.name, submenu: [ { role: 'about' }, { type: 'separator' }, diff --git a/docs/api/modernization/property-updates.md b/docs/api/modernization/property-updates.md index 51f519805fde..5a83d9dca686 100644 --- a/docs/api/modernization/property-updates.md +++ b/docs/api/modernization/property-updates.md @@ -5,7 +5,6 @@ The Electron team is currently undergoing an initiative to convert separate gett ## Candidates * `app` module - * `name` * `dock` * `badge` * `autoUpdater` module @@ -58,3 +57,4 @@ The Electron team is currently undergoing an initiative to convert separate gett * `accessibilitySupport` * `applicationMenu` * `badgeCount` + * `name` diff --git a/lib/browser/api/app.ts b/lib/browser/api/app.ts index 17843bbacd9b..a3c8ea3eac27 100644 --- a/lib/browser/api/app.ts +++ b/lib/browser/api/app.ts @@ -56,8 +56,8 @@ app.isPackaged = (() => { app._setDefaultAppPaths = (packagePath) => { // Set the user path according to application's name. - app.setPath('userData', path.join(app.getPath('appData'), app.getName())) - app.setPath('userCache', path.join(app.getPath('cache'), app.getName())) + app.setPath('userData', path.join(app.getPath('appData'), app.name!)) + app.setPath('userCache', path.join(app.getPath('cache'), app.name!)) app.setAppPath(packagePath) // Add support for --user-data-dir= @@ -89,6 +89,7 @@ for (const name of events) { // Property Deprecations deprecate.fnToProperty(app, 'accessibilitySupportEnabled', '_isAccessibilitySupportEnabled', '_setAccessibilitySupportEnabled') deprecate.fnToProperty(app, 'badgeCount', '_getBadgeCount', '_setBadgeCount') +deprecate.fnToProperty(app, 'name', '_getName', '_setName') // Wrappers for native classes. const { DownloadItem } = process.electronBinding('download_item') diff --git a/lib/browser/api/menu-item-roles.js b/lib/browser/api/menu-item-roles.js index 22949b324cdb..447edd8ed164 100644 --- a/lib/browser/api/menu-item-roles.js +++ b/lib/browser/api/menu-item-roles.js @@ -9,7 +9,7 @@ const isLinux = process.platform === 'linux' const roles = { about: { get label () { - return isLinux ? 'About' : `About ${app.getName()}` + return isLinux ? 'About' : `About ${app.name}` } }, close: { @@ -49,7 +49,7 @@ const roles = { }, hide: { get label () { - return `Hide ${app.getName()}` + return `Hide ${app.name}` }, accelerator: 'Command+H' }, @@ -77,7 +77,7 @@ const roles = { quit: { get label () { switch (process.platform) { - case 'darwin': return `Quit ${app.getName()}` + case 'darwin': return `Quit ${app.name}` case 'win32': return 'Exit' default: return 'Quit' } @@ -172,7 +172,7 @@ const roles = { // App submenu should be used for Mac only appmenu: { get label () { - return app.getName() + return app.name }, submenu: [ { role: 'about' }, diff --git a/lib/browser/crash-reporter-init.js b/lib/browser/crash-reporter-init.js index ed03cc214510..f8175bd9749c 100644 --- a/lib/browser/crash-reporter-init.js +++ b/lib/browser/crash-reporter-init.js @@ -14,7 +14,7 @@ const getTempDirectory = function () { } exports.crashReporterInit = function (options) { - const productName = options.productName || app.getName() + const productName = options.productName || app.name const crashesDirectory = path.join(getTempDirectory(), `${productName} Crashes`) let crashServicePid diff --git a/lib/browser/init.ts b/lib/browser/init.ts index 473433a51196..12647fb45aca 100644 --- a/lib/browser/init.ts +++ b/lib/browser/init.ts @@ -134,16 +134,16 @@ if (packageJson.version != null) { // Set application's name. if (packageJson.productName != null) { - app.setName(`${packageJson.productName}`.trim()) + app.name = `${packageJson.productName}`.trim() } else if (packageJson.name != null) { - app.setName(`${packageJson.name}`.trim()) + app.name = `${packageJson.name}`.trim() } // Set application's desktop name. if (packageJson.desktopName != null) { app.setDesktopName(packageJson.desktopName) } else { - app.setDesktopName((app.getName()) + '.desktop') + app.setDesktopName(`${app.name}.desktop`) } // Set v8 flags diff --git a/spec-main/api-app-spec.ts b/spec-main/api-app-spec.ts index 3fa13871e0e4..2af44ec58f9b 100644 --- a/spec-main/api-app-spec.ts +++ b/spec-main/api-app-spec.ts @@ -84,19 +84,35 @@ describe('app module', () => { }) }) + describe('app.name', () => { + it('returns the name field of package.json', () => { + expect(app.name).to.equal('Electron Test Main') + }) + + it('overrides the name', () => { + expect(app.name).to.equal('Electron Test Main') + app.name = 'test-name' + + expect(app.name).to.equal('test-name') + app.name = 'Electron Test Main' + }) + }) + + // TODO(codebytere): remove when propertyification is complete describe('app.getName()', () => { it('returns the name field of package.json', () => { expect(app.getName()).to.equal('Electron Test Main') }) }) + // TODO(codebytere): remove when propertyification is complete describe('app.setName(name)', () => { it('overrides the name', () => { expect(app.getName()).to.equal('Electron Test Main') app.setName('test-name') expect(app.getName()).to.equal('test-name') - app.setName('Electron Test') + app.setName('Electron Test Main') }) }) diff --git a/spec/api-crash-reporter-spec.js b/spec/api-crash-reporter-spec.js index bd9928a3308f..6b8881a9ee54 100644 --- a/spec/api-crash-reporter-spec.js +++ b/spec/api-crash-reporter-spec.js @@ -79,7 +79,7 @@ describe('crashReporter module', () => { stopServer = startServer({ callback (port) { - const crashesDir = path.join(app.getPath('temp'), `${process.platform === 'win32' ? 'Zombies' : app.getName()} Crashes`) + const crashesDir = path.join(app.getPath('temp'), `${process.platform === 'win32' ? 'Zombies' : app.name} Crashes`) const version = app.getVersion() const crashPath = path.join(fixtures, 'module', 'crash.js') diff --git a/spec/api-menu-item-spec.js b/spec/api-menu-item-spec.js index e44570e4352f..bdf94ae9c734 100644 --- a/spec/api-menu-item-spec.js +++ b/spec/api-menu-item-spec.js @@ -246,7 +246,7 @@ describe('MenuItems', () => { 'minimize': 'Minimize', 'paste': 'Paste', 'pasteandmatchstyle': 'Paste and Match Style', - 'quit': (process.platform === 'darwin') ? `Quit ${app.getName()}` : (process.platform === 'win32') ? 'Exit' : 'Quit', + 'quit': (process.platform === 'darwin') ? `Quit ${app.name}` : (process.platform === 'win32') ? 'Exit' : 'Quit', 'redo': 'Redo', 'reload': 'Reload', 'resetzoom': 'Actual Size', @@ -316,7 +316,7 @@ describe('MenuItems', () => { it('includes a default submenu layout when submenu is empty', () => { const item = new MenuItem({ role: 'appMenu' }) - expect(item.label).to.equal(app.getName()) + expect(item.label).to.equal(app.name) expect(item.submenu.items[0].role).to.equal('about') expect(item.submenu.items[1].type).to.equal('separator') expect(item.submenu.items[2].role).to.equal('services') @@ -335,7 +335,7 @@ describe('MenuItems', () => { role: 'close' }] }) - expect(item.label).to.equal(app.getName()) + expect(item.label).to.equal(app.name) expect(item.submenu.items[0].role).to.equal('close') }) }) diff --git a/spec/api-notification-dbus-spec.js b/spec/api-notification-dbus-spec.js index 3c6193e0a326..e93adf4feea8 100644 --- a/spec/api-notification-dbus-spec.js +++ b/spec/api-notification-dbus-spec.js @@ -20,14 +20,14 @@ const skip = process.platform !== 'linux' || (skip ? describe.skip : describe)('Notification module (dbus)', () => { let mock, Notification, getCalls, reset - const realAppName = app.getName() + const realAppName = app.name const realAppVersion = app.getVersion() const appName = 'api-notification-dbus-spec' const serviceName = 'org.freedesktop.Notifications' before(async () => { // init app - app.setName(appName) + app.name = appName app.setDesktopName(`${appName}.desktop`) // init dbus const path = '/org/freedesktop/Notifications'