Merge pull request #6182 from electron/format-menu-item

Reformat MenuItem class

lib/browser/api/menu-item.js

@@ -31,97 +31,94 @@ const methodInApp = {
   quit: true
 }
 
-const MenuItem = (function () {
-  MenuItem.types = ['normal', 'separator', 'submenu', 'checkbox', 'radio']
-
-  function MenuItem (options) {
-    const {app, Menu} = require('electron')
-
-    const click = options.click
-    this.selector = options.selector
-    this.type = options.type
-    this.role = options.role
-    this.label = options.label
-    this.sublabel = options.sublabel
-    this.accelerator = options.accelerator
-    this.icon = options.icon
-    this.enabled = options.enabled
-    this.visible = options.visible
-    this.checked = options.checked
-    this.submenu = options.submenu
-    if ((this.submenu != null) && this.submenu.constructor !== Menu) {
-      this.submenu = Menu.buildFromTemplate(this.submenu)
-    }
-    if ((this.type == null) && (this.submenu != null)) {
-      this.type = 'submenu'
-    }
-    if (this.type === 'submenu' && (this.submenu != null ? this.submenu.constructor : void 0) !== Menu) {
-      throw new Error('Invalid submenu')
-    }
-    this.overrideReadOnlyProperty('type', 'normal')
-    this.overrideReadOnlyProperty('role')
-    this.overrideReadOnlyProperty('accelerator')
-    this.overrideReadOnlyProperty('icon')
-    this.overrideReadOnlyProperty('submenu')
-    this.overrideProperty('label', '')
-    this.overrideProperty('sublabel', '')
-    this.overrideProperty('enabled', true)
-    this.overrideProperty('visible', true)
-    this.overrideProperty('checked', false)
-    if (MenuItem.types.indexOf(this.type) === -1) {
-      throw new Error('Unknown menu type ' + this.type)
-    }
-    this.commandId = ++nextCommandId
-    this.click = (event, focusedWindow) => {
-      // Manually flip the checked flags when clicked.
-      if (this.type === 'checkbox' || this.type === 'radio') {
-        this.checked = !this.checked
-      }
+const MenuItem = function (options) {
+  const {app, Menu} = require('electron')
 
-      if (this.role && rolesMap[this.role] && process.platform !== 'darwin' && (focusedWindow != null)) {
-        const methodName = rolesMap[this.role]
-        if (methodInApp[methodName]) {
-          return app[methodName]()
-        } else if (typeof methodInBrowserWindow[methodName] === 'function') {
-          return methodInBrowserWindow[methodName](focusedWindow)
-        } else if (methodInBrowserWindow[methodName]) {
-          return focusedWindow[methodName]()
-        } else {
-          const {webContents} = focusedWindow
-          return webContents != null ? webContents[methodName]() : void 0
-        }
-      } else if (typeof click === 'function') {
-        return click(this, focusedWindow, event)
-      } else if (typeof this.selector === 'string' && process.platform === 'darwin') {
-        return Menu.sendActionToFirstResponder(this.selector)
-      }
-    }
+  this.selector = options.selector
+  this.type = options.type
+  this.role = options.role
+  this.label = options.label
+  this.sublabel = options.sublabel
+  this.accelerator = options.accelerator
+  this.icon = options.icon
+  this.enabled = options.enabled
+  this.visible = options.visible
+  this.checked = options.checked
+
+  this.submenu = options.submenu
+  if (this.submenu != null && this.submenu.constructor !== Menu) {
+    this.submenu = Menu.buildFromTemplate(this.submenu)
+  }
+  if (this.type == null && this.submenu != null) {
+    this.type = 'submenu'
+  }
+  if (this.type === 'submenu' && (this.submenu == null || this.submenu.constructor !== Menu)) {
+    throw new Error('Invalid submenu')
   }
 
-  MenuItem.prototype.overrideProperty = function (name, defaultValue) {
-    if (defaultValue == null) {
-      defaultValue = null
-    }
-    this[name] != null ? this[name] : this[name] = defaultValue
+  this.overrideReadOnlyProperty('type', 'normal')
+  this.overrideReadOnlyProperty('role')
+  this.overrideReadOnlyProperty('accelerator')
+  this.overrideReadOnlyProperty('icon')
+  this.overrideReadOnlyProperty('submenu')
 
-    return this[name]
+  this.overrideProperty('label', '')
+  this.overrideProperty('sublabel', '')
+  this.overrideProperty('enabled', true)
+  this.overrideProperty('visible', true)
+  this.overrideProperty('checked', false)
+
+  if (!MenuItem.types.includes(this.type)) {
+    throw new Error(`Unknown menu item type: ${this.type}`)
   }
 
-  MenuItem.prototype.overrideReadOnlyProperty = function (name, defaultValue) {
-    if (defaultValue == null) {
-      defaultValue = null
+  this.commandId = ++nextCommandId
+
+  const click = options.click
+  this.click = (event, focusedWindow) => {
+    // Manually flip the checked flags when clicked.
+    if (this.type === 'checkbox' || this.type === 'radio') {
+      this.checked = !this.checked
     }
-    if (this[name] == null) {
-      this[name] = defaultValue
+
+    if (this.role && rolesMap[this.role] && process.platform !== 'darwin' && focusedWindow != null) {
+      const methodName = rolesMap[this.role]
+      if (methodInApp[methodName]) {
+        return app[methodName]()
+      } else if (typeof methodInBrowserWindow[methodName] === 'function') {
+        return methodInBrowserWindow[methodName](focusedWindow)
+      } else if (methodInBrowserWindow[methodName]) {
+        return focusedWindow[methodName]()
+      } else {
+        const {webContents} = focusedWindow
+        return webContents != null ? webContents[methodName]() : void 0
+      }
+    } else if (typeof click === 'function') {
+      return click(this, focusedWindow, event)
+    } else if (typeof this.selector === 'string' && process.platform === 'darwin') {
+      return Menu.sendActionToFirstResponder(this.selector)
     }
-    return Object.defineProperty(this, name, {
-      enumerable: true,
-      writable: false,
-      value: this[name]
-    })
   }
+}
+
+MenuItem.types = ['normal', 'separator', 'submenu', 'checkbox', 'radio']
 
-  return MenuItem
-})()
+MenuItem.prototype.overrideProperty = function (name, defaultValue) {
+  if (defaultValue == null) {
+    defaultValue = null
+  }
+  if (this[name] == null) {
+    this[name] = defaultValue
+  }
+}
+
+MenuItem.prototype.overrideReadOnlyProperty = function (name, defaultValue) {
+  this.overrideProperty(name, defaultValue)
+  Object.defineProperty(this, name, {
+    enumerable: true,
+    writable: false,
+    value: this[name]
+  })
+}
 
 module.exports = MenuItem

spec/api-menu-spec.js

@@ -1,10 +1,7 @@
 const assert = require('assert')
 
-const remote = require('electron').remote
-const ipcRenderer = require('electron').ipcRenderer
-
-const Menu = remote.require('electron').Menu
-const MenuItem = remote.require('electron').MenuItem
+const {ipcRenderer, remote} = require('electron')
+const {Menu, MenuItem} = remote
 
 describe('menu module', function () {
   describe('Menu.buildFromTemplate', function () {
@@ -359,4 +356,30 @@ describe('menu module', function () {
       }
     })
   })
+
+  describe('MenuItem with invalid type', function () {
+    it('throws an exception', function () {
+      assert.throws(function () {
+        var menu = Menu.buildFromTemplate([
+          {
+            label: 'text',
+            type: 'not-a-type'
+          }
+        ])
+      }, /Unknown menu item type: not-a-type/)
+    })
+  })
+
+  describe('MenuItem with submenu type and missing submenu', function () {
+    it('throws an exception', function () {
+      assert.throws(function () {
+        var menu = Menu.buildFromTemplate([
+          {
+            label: 'text',
+            type: 'submenu'
+          }
+        ])
+      }, /Invalid submenu/)
+    })
+  })
 })