refactor: convert browser window state fns to props (#18618)

atom/browser/api/atom_api_system_preferences.cc

@@ -102,9 +102,9 @@ void SystemPreferences::BuildPrototype(
                  &SystemPreferences::IsSwipeTrackingFromScrollEventsEnabled)
       .SetMethod("getEffectiveAppearance",
                  &SystemPreferences::GetEffectiveAppearance)
-      .SetMethod("getAppLevelAppearance",
+      .SetMethod("_getAppLevelAppearance",
                  &SystemPreferences::GetAppLevelAppearance)
-      .SetMethod("setAppLevelAppearance",
+      .SetMethod("_setAppLevelAppearance",
                  &SystemPreferences::SetAppLevelAppearance)
       .SetProperty("appLevelAppearance",
                    &SystemPreferences::GetAppLevelAppearance,

atom/browser/api/atom_api_top_level_window.cc

@@ -1070,19 +1070,31 @@ void TopLevelWindow::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("setMaximumSize", &TopLevelWindow::SetMaximumSize)
       .SetMethod("getMaximumSize", &TopLevelWindow::GetMaximumSize)
       .SetMethod("setSheetOffset", &TopLevelWindow::SetSheetOffset)
-      .SetMethod("setResizable", &TopLevelWindow::SetResizable)
-      .SetMethod("isResizable", &TopLevelWindow::IsResizable)
-      .SetMethod("setMovable", &TopLevelWindow::SetMovable)
       .SetMethod("moveTop", &TopLevelWindow::MoveTop)
-      .SetMethod("isMovable", &TopLevelWindow::IsMovable)
+      .SetMethod("_setResizable", &TopLevelWindow::SetResizable)
-      .SetMethod("setMinimizable", &TopLevelWindow::SetMinimizable)
+      .SetMethod("_isResizable", &TopLevelWindow::IsResizable)
-      .SetMethod("isMinimizable", &TopLevelWindow::IsMinimizable)
+      .SetProperty("resizable", &TopLevelWindow::IsResizable,
-      .SetMethod("setMaximizable", &TopLevelWindow::SetMaximizable)
+                   &TopLevelWindow::SetResizable)
-      .SetMethod("isMaximizable", &TopLevelWindow::IsMaximizable)
+      .SetMethod("_setMovable", &TopLevelWindow::SetMovable)
-      .SetMethod("setFullScreenable", &TopLevelWindow::SetFullScreenable)
+      .SetMethod("_isMovable", &TopLevelWindow::IsMovable)
-      .SetMethod("isFullScreenable", &TopLevelWindow::IsFullScreenable)
+      .SetProperty("movable", &TopLevelWindow::IsMovable,
-      .SetMethod("setClosable", &TopLevelWindow::SetClosable)
+                   &TopLevelWindow::SetMovable)
-      .SetMethod("isClosable", &TopLevelWindow::IsClosable)
+      .SetMethod("_setMinimizable", &TopLevelWindow::SetMinimizable)
+      .SetMethod("_isMinimizable", &TopLevelWindow::IsMinimizable)
+      .SetProperty("minimizable", &TopLevelWindow::IsMinimizable,
+                   &TopLevelWindow::SetMinimizable)
+      .SetMethod("_setMaximizable", &TopLevelWindow::SetMaximizable)
+      .SetMethod("_isMaximizable", &TopLevelWindow::IsMaximizable)
+      .SetProperty("maximizable", &TopLevelWindow::IsMaximizable,
+                   &TopLevelWindow::SetMaximizable)
+      .SetMethod("_setFullScreenable", &TopLevelWindow::SetFullScreenable)
+      .SetMethod("_isFullScreenable", &TopLevelWindow::IsFullScreenable)
+      .SetProperty("fullScreenable", &TopLevelWindow::IsFullScreenable,
+                   &TopLevelWindow::SetFullScreenable)
+      .SetMethod("_setClosable", &TopLevelWindow::SetClosable)
+      .SetMethod("_isClosable", &TopLevelWindow::IsClosable)
+      .SetProperty("closable", &TopLevelWindow::IsClosable,
+                   &TopLevelWindow::SetClosable)
       .SetMethod("setAlwaysOnTop", &TopLevelWindow::SetAlwaysOnTop)
       .SetMethod("isAlwaysOnTop", &TopLevelWindow::IsAlwaysOnTop)
       .SetMethod("center", &TopLevelWindow::Center)

docs/api/browser-window.md

@@ -1009,22 +1009,30 @@ Returns `Integer[]` - Contains the window's maximum width and height.
 
 Sets whether the window can be manually resized by user.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `win.isResizable()`
 
 Returns `Boolean` - Whether the window can be manually resized by user.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `win.setMovable(movable)` _macOS_ _Windows_
 
 * `movable` Boolean
 
 Sets whether the window can be moved by user. On Linux does nothing.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `win.isMovable()` _macOS_ _Windows_
 
 Returns `Boolean` - Whether the window can be moved by user.
 
 On Linux always returns `true`.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `win.setMinimizable(minimizable)` _macOS_ _Windows_
 
 * `minimizable` Boolean
@@ -1032,12 +1040,16 @@ On Linux always returns `true`.
 Sets whether the window can be manually minimized by user. On Linux does
 nothing.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `win.isMinimizable()` _macOS_ _Windows_
 
 Returns `Boolean` - Whether the window can be manually minimized by user
 
 On Linux always returns `true`.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `win.setMaximizable(maximizable)` _macOS_ _Windows_
 
 * `maximizable` Boolean
@@ -1045,12 +1057,16 @@ On Linux always returns `true`.
 Sets whether the window can be manually maximized by user. On Linux does
 nothing.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `win.isMaximizable()` _macOS_ _Windows_
 
 Returns `Boolean` - Whether the window can be manually maximized by user.
 
 On Linux always returns `true`.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `win.setFullScreenable(fullscreenable)`
 
 * `fullscreenable` Boolean
@@ -1058,23 +1074,31 @@ On Linux always returns `true`.
 Sets whether the maximize/zoom window button toggles fullscreen mode or
 maximizes the window.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `win.isFullScreenable()`
 
 Returns `Boolean` - Whether the maximize/zoom window button toggles fullscreen mode or
 maximizes the window.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `win.setClosable(closable)` _macOS_ _Windows_
 
 * `closable` Boolean
 
 Sets whether the window can be manually closed by user. On Linux does nothing.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `win.isClosable()` _macOS_ _Windows_
 
 Returns `Boolean` - Whether the window can be manually closed by user.
 
 On Linux always returns `true`.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `win.setAlwaysOnTop(flag[, level][, relativeLevel])`
 
 * `flag` Boolean
@@ -1653,6 +1677,39 @@ A `Boolean` property that determines whether the window menu bar should hide its
 If the menu bar is already visible, setting this property to `true` won't
 hide it immediately.
 
+#### `win.minimizable`
+
+A `Boolean` property that determines whether the window can be manually minimized by user.
+
+On Linux the setter is a no-op, although the getter returns `true`.
+
+#### `win.maximizable`
+
+A `Boolean` property that determines whether the window can be manually maximized by user.
+
+On Linux the setter is a no-op, although the getter returns `true`.
+
+#### `win.fullScreenable`
+
+A `Boolean` property that determines whether the maximize/zoom window button toggles fullscreen mode or
+maximizes the window.
+
+#### `win.resizable`
+
+A `Boolean` property that determines whether the window can be manually resized by user.
+
+#### `win.closable`
+
+A `Boolean` property that determines whether the window can be manually closed by user.
+
+On Linux the setter is a no-op, although the getter returns `true`.
+
+#### `win.movable`
+
+A `Boolean` property that determines Whether the window can be moved by user.
+
+On Linux the setter is a no-op, although the getter returns `true`.
+
 #### `win.excludedFromShownWindowsMenu` _macOS_
 
 A `Boolean` property that determines whether the window is excluded from the application’s Windows menu. `false` by default.

docs/api/modernization/property-updates.md

@@ -12,12 +12,6 @@ The Electron team is currently undergoing an initiative to convert separate gett
 * `BrowserWindow`
   * `fullscreen`
   * `simpleFullscreen`
-  * `movable`
-  * `resizable`
-  * `maximizable`
-  * `minimizable`
-  * `fullscreenable`
-  * `closable`
   * `alwaysOnTop`
   * `title`
   * `documentEdited`
@@ -55,6 +49,12 @@ The Electron team is currently undergoing an initiative to convert separate gett
   * `name`
 * `BrowserWindow` module
   * `autohideMenuBar`
+  * `resizable`
+  * `maximizable`
+  * `minimizable`
+  * `fullscreenable`
+  * `movable`
+  * `closable`
 * `NativeImage`
   * `isMacTemplateImage`
 * `SystemPreferences` module

lib/browser/api/app.ts

@@ -77,9 +77,9 @@ for (const name of events) {
 }
 
 // Property Deprecations
-deprecate.fnToProperty(app, 'accessibilitySupportEnabled', '_isAccessibilitySupportEnabled', '_setAccessibilitySupportEnabled')
+deprecate.fnToProperty(App.prototype, 'accessibilitySupportEnabled', '_isAccessibilitySupportEnabled', '_setAccessibilitySupportEnabled')
-deprecate.fnToProperty(app, 'badgeCount', '_getBadgeCount', '_setBadgeCount')
+deprecate.fnToProperty(App.prototype, 'badgeCount', '_getBadgeCount', '_setBadgeCount')
-deprecate.fnToProperty(app, 'name', '_getName', '_setName')
+deprecate.fnToProperty(App.prototype, 'name', '_getName', '_setName')
 
 // Wrappers for native classes.
 const { DownloadItem } = process.electronBinding('download_item')

lib/browser/api/browser-window.js

@@ -193,5 +193,11 @@ Object.assign(BrowserWindow.prototype, {
 
 // Deprecations
 deprecate.fnToProperty(BrowserWindow.prototype, 'autoHideMenuBar', '_isMenuBarAutoHide', '_setAutoHideMenuBar')
+deprecate.fnToProperty(BrowserWindow.prototype, 'minimizable', '_isMinimizable', '_setMinimizable')
+deprecate.fnToProperty(BrowserWindow.prototype, 'maximizable', '_isMaximizable', '_setMaximizable')
+deprecate.fnToProperty(BrowserWindow.prototype, 'resizable', '_isResizable', '_setResizable')
+deprecate.fnToProperty(BrowserWindow.prototype, 'fullScreenable', '_isFullScreenable', '_setFullScreenable')
+deprecate.fnToProperty(BrowserWindow.prototype, 'closable', '_isClosable', '_setClosable')
+deprecate.fnToProperty(BrowserWindow.prototype, 'movable', '_isMovable', '_setMovable')
 
 module.exports = BrowserWindow

lib/browser/api/system-preferences.js

@@ -9,7 +9,7 @@ Object.setPrototypeOf(SystemPreferences.prototype, EventEmitter.prototype)
 EventEmitter.call(systemPreferences)
 
 if ('appLevelAppearance' in systemPreferences) {
-  deprecate.fnToProperty(systemPreferences, 'appLevelAppearance', '_getAppLevelAppearance', '_setAppLevelAppearance')
+  deprecate.fnToProperty(SystemPreferences.prototype, 'appLevelAppearance', '_getAppLevelAppearance', '_setAppLevelAppearance')
 }
 
 module.exports = systemPreferences

lib/common/api/deprecate.ts

@@ -57,17 +57,18 @@ const deprecate: ElectronInternal.DeprecationUtil = {
   },
 
   // deprecate a getter/setter function pair in favor of a property
-  fnToProperty: (module: any, prop: string, getter: string, setter: string) => {
+  fnToProperty: (prototype: any, prop: string, getter: string, setter: string) => {
-    const withWarnOnce = (obj: any, key: any, oldName: string, newName: string) => {
+    const withWarnOnce = function (obj: any, key: any, oldName: string, newName: string) {
       const warn = warnOnce(oldName, newName)
-      return (...args: any) => {
+      const method = obj[key]
+      return function (this: any, ...args: any) {
         warn()
-        return obj[key](...args)
+        return method.apply(this, args)
       }
     }
 
-    module[getter.substr(1)] = withWarnOnce(module, getter, `${getter.substr(1)} function`, `${prop} property`)
+    prototype[getter.substr(1)] = withWarnOnce(prototype, getter, `${getter.substr(1)} function`, `${prop} property`)
-    module[setter.substr(1)] = withWarnOnce(module, setter, `${setter.substr(1)} function`, `${prop} property`)
+    prototype[setter.substr(1)] = withWarnOnce(prototype, setter, `${setter.substr(1)} function`, `${prop} property`)
   },
 
   // remove a property with no replacement

spec/api-browser-window-spec.js

@@ -2336,24 +2336,24 @@ describe('BrowserWindow module', () => {
         show: false
       })
 
-      w.setMinimizable(false)
+      w.minimizable = false
-      w.setMinimizable(true)
+      w.minimizable = true
       expect(w.getSize()).to.deep.equal([300, 200])
 
-      w.setResizable(false)
+      w.resizable = false
-      w.setResizable(true)
+      w.resizable = true
       expect(w.getSize()).to.deep.equal([300, 200])
 
-      w.setMaximizable(false)
+      w.maximizable = false
-      w.setMaximizable(true)
+      w.maximizable = true
       expect(w.getSize()).to.deep.equal([300, 200])
 
-      w.setFullScreenable(false)
+      w.fullScreenable = false
-      w.setFullScreenable(true)
+      w.fullScreenable = true
       expect(w.getSize()).to.deep.equal([300, 200])
 
-      w.setClosable(false)
+      w.closable = false
-      w.setClosable(true)
+      w.closable = true
       expect(w.getSize()).to.deep.equal([300, 200])
     })
 
@@ -2361,13 +2361,14 @@ describe('BrowserWindow module', () => {
       it('can be changed with resizable option', () => {
         w.destroy()
         w = new BrowserWindow({ show: false, resizable: false })
-        expect(w.isResizable()).to.be.false()
+        expect(w.resizable).to.be.false()
 
         if (process.platform === 'darwin') {
-          expect(w.isMaximizable()).to.to.true()
+          expect(w.maximizable).to.to.true()
         }
       })
 
+      // TODO(codebytere): remove when propertyification is complete
       it('can be changed with setResizable method', () => {
         expect(w.isResizable()).to.be.true()
         w.setResizable(false)
@@ -2376,15 +2377,23 @@ describe('BrowserWindow module', () => {
         expect(w.isResizable()).to.be.true()
       })
 
+      it('can be changed with resizable property', () => {
+        expect(w.resizable).to.be.true()
+        w.resizable = false
+        expect(w.resizable).to.be.false()
+        w.resizable = true
+        expect(w.resizable).to.be.true()
+      })
+
       it('works for a frameless window', () => {
         w.destroy()
         w = new BrowserWindow({ show: false, frame: false })
-        expect(w.isResizable()).to.be.true()
+        expect(w.resizable).to.be.true()
 
         if (process.platform === 'win32') {
           w.destroy()
           w = new BrowserWindow({ show: false, thickFrame: false })
-          expect(w.isResizable()).to.be.false()
+          expect(w.resizable).to.be.false()
         }
       })
 
@@ -2456,7 +2465,23 @@ describe('BrowserWindow module', () => {
       return
     }
 
-    describe('movable state', () => {
+    describe('movable state (property)', () => {
+      it('can be changed with movable option', () => {
+        w.destroy()
+        w = new BrowserWindow({ show: false, movable: false })
+        expect(w.movable).to.be.false()
+      })
+      it('can be changed with movable property', () => {
+        expect(w.movable).to.be.true()
+        w.movable = false
+        expect(w.movable).to.be.false()
+        w.movable = true
+        expect(w.movable).to.be.true()
+      })
+    })
+
+    // TODO(codebytere): remove when propertyification is complete
+    describe('movable state (methods)', () => {
       it('can be changed with movable option', () => {
         w.destroy()
         w = new BrowserWindow({ show: false, movable: false })
@@ -2471,7 +2496,24 @@ describe('BrowserWindow module', () => {
       })
     })
 
-    describe('minimizable state', () => {
+    describe('minimizable state (property)', () => {
+      it('can be changed with minimizable option', () => {
+        w.destroy()
+        w = new BrowserWindow({ show: false, minimizable: false })
+        expect(w.minimizable).to.be.false()
+      })
+
+      it('can be changed with minimizable property', () => {
+        expect(w.minimizable).to.be.true()
+        w.minimizable = false
+        expect(w.minimizable).to.be.false()
+        w.minimizable = true
+        expect(w.minimizable).to.be.true()
+      })
+    })
+
+    // TODO(codebytere): remove when propertyification is complete
+    describe('minimizable state (methods)', () => {
       it('can be changed with minimizable option', () => {
         w.destroy()
         w = new BrowserWindow({ show: false, minimizable: false })
@@ -2487,7 +2529,40 @@ describe('BrowserWindow module', () => {
       })
     })
 
-    describe('maximizable state', () => {
+    describe('maximizable state (property)', () => {
+      it('can be changed with maximizable option', () => {
+        w.destroy()
+        w = new BrowserWindow({ show: false, maximizable: false })
+        expect(w.maximizable).to.be.false()
+      })
+
+      it('can be changed with maximizable property', () => {
+        expect(w.maximizable).to.be.true()
+        w.maximizable = false
+        expect(w.maximizable).to.be.false()
+        w.maximizable = true
+        expect(w.maximizable).to.be.true()
+      })
+
+      it('is not affected when changing other states', () => {
+        w.maximizable = false
+        expect(w.maximizable).to.be.false()
+        w.minimizable = false
+        expect(w.maximizable).to.be.false()
+        w.closable = false
+        expect(w.maximizable).to.be.false()
+
+        w.maximizable = true
+        expect(w.maximizable).to.be.true()
+        w.closable = true
+        expect(w.maximizable).to.be.true()
+        w.fullScreenable = false
+        expect(w.maximizable).to.be.true()
+      })
+    })
+
+    // TODO(codebytere): remove when propertyification is complete
+    describe('maximizable state (methods)', () => {
       it('can be changed with maximizable option', () => {
         w.destroy()
         w = new BrowserWindow({ show: false, maximizable: false })
@@ -2523,6 +2598,23 @@ describe('BrowserWindow module', () => {
       // Only implemented on windows.
       if (process.platform !== 'win32') return
 
+      it('is reset to its former state', () => {
+        w.maximizable = false
+        w.resizable = false
+        w.resizable = true
+        expect(w.maximizable).to.be.false()
+        w.maximizable = true
+        w.resizable = false
+        w.resizable = true
+        expect(w.maximizable).to.be.true()
+      })
+    })
+
+    // TODO(codebytere): remove when propertyification is complete
+    describe('maximizable state (Windows only) (methods)', () => {
+      // Only implemented on windows.
+      if (process.platform !== 'win32') return
+
       it('is reset to its former state', () => {
         w.setMaximizable(false)
         w.setResizable(false)
@@ -2535,12 +2627,30 @@ describe('BrowserWindow module', () => {
       })
     })
 
-    describe('fullscreenable state', () => {
+    describe('fullscreenable state (property)', () => {
       before(function () {
-        // Only implemented on macOS.
+        if (process.platform !== 'darwin') this.skip()
-        if (process.platform !== 'darwin') {
+      })
-          this.skip()
+
-        }
+      it('can be changed with fullscreenable option', () => {
+        w.destroy()
+        w = new BrowserWindow({ show: false, fullscreenable: false })
+        expect(w.fullScreenable).to.be.false()
+      })
+
+      it('can be changed with fullScreenable property', () => {
+        expect(w.fullScreenable).to.be.true()
+        w.fullScreenable = false
+        expect(w.fullScreenable).to.be.false()
+        w.fullScreenable = true
+        expect(w.fullScreenable).to.be.true()
+      })
+    })
+
+    // TODO(codebytere): remove when propertyification is complete
+    describe('fullscreenable state (methods)', () => {
+      before(function () {
+        if (process.platform !== 'darwin') this.skip()
       })
 
       it('can be changed with fullscreenable option', () => {
@@ -2584,21 +2694,18 @@ describe('BrowserWindow module', () => {
 
     describe('fullscreen state with resizable set', () => {
       before(function () {
-        // Only implemented on macOS.
+        if (process.platform !== 'darwin') this.skip()
-        if (process.platform !== 'darwin') {
-          this.skip()
-        }
       })
 
       it('resizable flag should be set to true and restored', (done) => {
         w.destroy()
         w = new BrowserWindow({ resizable: false })
         w.once('enter-full-screen', () => {
-          expect(w.isResizable()).to.be.true()
+          expect(w.resizable).to.be.true()
           w.setFullScreen(false)
         })
         w.once('leave-full-screen', () => {
-          expect(w.isResizable()).to.be.false()
+          expect(w.resizable).to.be.false()
           done()
         })
         w.setFullScreen(true)
@@ -2645,7 +2752,24 @@ describe('BrowserWindow module', () => {
       })
     })
 
-    describe('closable state', () => {
+    describe('closable state (property)', () => {
+      it('can be changed with closable option', () => {
+        w.destroy()
+        w = new BrowserWindow({ show: false, closable: false })
+        expect(w.closable).to.be.false()
+      })
+
+      it('can be changed with setClosable method', () => {
+        expect(w.closable).to.be.true()
+        w.closable = false
+        expect(w.closable).to.be.false()
+        w.closable = true
+        expect(w.closable).to.be.true()
+      })
+    })
+
+    // TODO(codebytere): remove when propertyification is complete
+    describe('closable state (methods)', () => {
       it('can be changed with closable option', () => {
         w.destroy()
         w = new BrowserWindow({ show: false, closable: false })

spec/api-system-preferences-spec.js

@@ -160,8 +160,8 @@ describe('systemPreferences module', () => {
       expect(systemPreferences).to.have.a.property('appLevelAppearance')
 
       // TODO(codebytere): remove when propertyification is complete
-      expect(systemPreferences.getAppLevelAppearance).to.be.a('function')
       expect(systemPreferences.setAppLevelAppearance).to.be.a('function')
+      expect(() => { systemPreferences.getAppLevelAppearance() }).to.not.throw()
     })
   })