Browse Source

refactor: allow requiring modules with no side effects (#17496)

Shelley Vohr 6 years ago
parent
commit
7b55ee9d36

+ 5 - 4
atom/browser/api/atom_api_power_monitor.cc

@@ -101,9 +101,10 @@ int PowerMonitor::GetSystemIdleTime() {
 // static
 v8::Local<v8::Value> PowerMonitor::Create(v8::Isolate* isolate) {
   if (!Browser::Get()->is_ready()) {
-    isolate->ThrowException(v8::Exception::Error(mate::StringToV8(
-        isolate,
-        "Cannot require \"powerMonitor\" module before app is ready")));
+    isolate->ThrowException(v8::Exception::Error(
+        mate::StringToV8(isolate,
+                         "The 'powerMonitor' module can't be used before the "
+                         "app 'ready' event")));
     return v8::Null(isolate);
   }
 
@@ -139,7 +140,7 @@ void Initialize(v8::Local<v8::Object> exports,
                 void* priv) {
   v8::Isolate* isolate = context->GetIsolate();
   mate::Dictionary dict(isolate, exports);
-  dict.Set("powerMonitor", PowerMonitor::Create(isolate));
+  dict.Set("createPowerMonitor", base::Bind(&PowerMonitor::Create, isolate));
   dict.Set("PowerMonitor", PowerMonitor::GetConstructor(isolate)
                                ->GetFunction(context)
                                .ToLocalChecked());

+ 3 - 2
atom/browser/api/atom_api_screen.cc

@@ -116,7 +116,8 @@ void Screen::OnDisplayMetricsChanged(const display::Display& display,
 v8::Local<v8::Value> Screen::Create(v8::Isolate* isolate) {
   if (!Browser::Get()->is_ready()) {
     isolate->ThrowException(v8::Exception::Error(mate::StringToV8(
-        isolate, "Cannot require \"screen\" module before app is ready")));
+        isolate,
+        "The 'screen' module can't be used before the app 'ready' event")));
     return v8::Null(isolate);
   }
 
@@ -162,7 +163,7 @@ void Initialize(v8::Local<v8::Object> exports,
                 void* priv) {
   v8::Isolate* isolate = context->GetIsolate();
   mate::Dictionary dict(isolate, exports);
-  dict.Set("screen", Screen::Create(isolate));
+  dict.Set("createScreen", base::Bind(&Screen::Create, isolate));
   dict.Set(
       "Screen",
       Screen::GetConstructor(isolate)->GetFunction(context).ToLocalChecked());

+ 8 - 9
docs/api/power-monitor.md

@@ -4,17 +4,17 @@
 
 Process: [Main](../glossary.md#main-process)
 
-You cannot require or use this module until the `ready` event of the `app`
+
+This module cannot be used until the `ready` event of the `app`
 module is emitted.
 
 For example:
 
 ```javascript
-const electron = require('electron')
-const { app } = electron
+const { app, powerMonitor } = require('electron')
 
 app.on('ready', () => {
-  electron.powerMonitor.on('suspend', () => {
+  powerMonitor.on('suspend', () => {
     console.log('The system is going to sleep')
   })
 })
@@ -59,7 +59,7 @@ Emitted as soon as the systems screen is unlocked.
 
 The `powerMonitor` module has the following methods:
 
-#### `powerMonitor.querySystemIdleState(idleThreshold, callback)` _(Deprecated)_
+### `powerMonitor.querySystemIdleState(idleThreshold, callback)` _(Deprecated)_
 
 * `idleThreshold` Integer
 * `callback` Function
@@ -70,14 +70,14 @@ before considered idle. `callback` will be called synchronously on some systems
 and with an `idleState` argument that describes the system's state. `locked` is
 available on supported systems only.
 
-#### `powerMonitor.querySystemIdleTime(callback)` _(Deprecated)_
+### `powerMonitor.querySystemIdleTime(callback)` _(Deprecated)_
 
 * `callback` Function
   * `idleTime` Integer - Idle time in seconds
 
 Calculate system idle time in seconds.
 
-#### `powerMonitor.getSystemIdleState(idleThreshold)`
+### `powerMonitor.getSystemIdleState(idleThreshold)`
 
 * `idleThreshold` Integer
 
@@ -86,9 +86,8 @@ Returns `String` - The system's current state. Can be `active`, `idle`, `locked`
 Calculate the system idle state. `idleThreshold` is the amount of time (in seconds)
 before considered idle.  `locked` is available on supported systems only.
 
-#### `powerMonitor.getSystemIdleTime()`
+### `powerMonitor.getSystemIdleTime()`
 
 Returns `Integer` - Idle time in seconds
 
 Calculate system idle time in seconds.
-

+ 5 - 8
docs/api/screen.md

@@ -4,7 +4,7 @@
 
 Process: [Main](../glossary.md#main-process)
 
-You cannot require or use this module until the `ready` event of the `app`
+This module cannot be used until the `ready` event of the `app`
 module is emitted.
 
 `screen` is an [EventEmitter](https://nodejs.org/api/events.html#events_class_eventemitter).
@@ -15,13 +15,11 @@ property, so writing `let { screen } = require('electron')` will not work.
 An example of creating a window that fills the whole screen:
 
 ```javascript
-const electron = require('electron')
-const { app, BrowserWindow } = electron
+const { app, BrowserWindow, screen } = require('electron')
 
 let win
-
 app.on('ready', () => {
-  const { width, height } = electron.screen.getPrimaryDisplay().workAreaSize
+  const { width, height } = screen.getPrimaryDisplay().workAreaSize
   win = new BrowserWindow({ width, height })
   win.loadURL('https://github.com')
 })
@@ -30,13 +28,12 @@ app.on('ready', () => {
 Another example of creating a window in the external display:
 
 ```javascript
-const electron = require('electron')
-const { app, BrowserWindow } = require('electron')
+const { app, BrowserWindow, screen } = require('electron')
 
 let win
 
 app.on('ready', () => {
-  let displays = electron.screen.getAllDisplays()
+  let displays = screen.getAllDisplays()
   let externalDisplay = displays.find((display) => {
     return display.bounds.x !== 0 || display.bounds.y !== 0
   })

+ 3 - 2
filenames.gni

@@ -22,10 +22,10 @@ filenames = {
     "lib/browser/api/net.js",
     "lib/browser/api/net-log.js",
     "lib/browser/api/notification.js",
-    "lib/browser/api/power-monitor.js",
+    "lib/browser/api/power-monitor.ts",
     "lib/browser/api/power-save-blocker.js",
     "lib/browser/api/protocol.ts",
-    "lib/browser/api/screen.js",
+    "lib/browser/api/screen.ts",
     "lib/browser/api/session.js",
     "lib/browser/api/system-preferences.js",
     "lib/browser/api/top-level-window.js",
@@ -46,6 +46,7 @@ filenames = {
     "lib/browser/navigation-controller.js",
     "lib/browser/objects-registry.js",
     "lib/browser/rpc-server.js",
+    "lib/browser/utils.ts",
     "lib/common/api/clipboard.js",
     "lib/common/api/deprecate.ts",
     "lib/common/api/deprecations.js",

+ 12 - 18
lib/browser/api/power-monitor.js → lib/browser/api/power-monitor.ts

@@ -1,22 +1,25 @@
 'use strict'
 
+import { createLazyInstance } from '../utils'
+
 const { EventEmitter } = require('events')
-const { powerMonitor, PowerMonitor } = process.electronBinding('power_monitor')
+const { createPowerMonitor, PowerMonitor } = process.electronBinding('power_monitor')
 const { deprecate } = require('electron')
 
 // PowerMonitor is an EventEmitter.
 Object.setPrototypeOf(PowerMonitor.prototype, EventEmitter.prototype)
-EventEmitter.call(powerMonitor)
+
+const powerMonitor = createLazyInstance(createPowerMonitor, PowerMonitor, true)
 
 // On Linux we need to call blockShutdown() to subscribe to shutdown event.
 if (process.platform === 'linux') {
-  powerMonitor.on('newListener', (event) => {
+  powerMonitor.on('newListener', (event:string) => {
     if (event === 'shutdown' && powerMonitor.listenerCount('shutdown') === 0) {
       powerMonitor.blockShutdown()
     }
   })
 
-  powerMonitor.on('removeListener', (event) => {
+  powerMonitor.on('removeListener', (event: string) => {
     if (event === 'shutdown' && powerMonitor.listenerCount('shutdown') === 0) {
       powerMonitor.unblockShutdown()
     }
@@ -24,30 +27,21 @@ if (process.platform === 'linux') {
 }
 
 // TODO(nitsakh): Remove in 7.0
-powerMonitor.querySystemIdleState = function (threshold, callback) {
+powerMonitor.querySystemIdleState = function (threshold: number, callback: Function) {
   deprecate.warn('powerMonitor.querySystemIdleState', 'powerMonitor.getSystemIdleState')
-  if (typeof threshold !== 'number') {
-    throw new Error('Must pass threshold as a number')
-  }
-
-  if (typeof callback !== 'function') {
-    throw new Error('Must pass callback as a function argument')
-  }
+  if (typeof threshold !== 'number') throw new Error('Must pass threshold as a number')
+  if (typeof callback !== 'function') throw new Error('Must pass callback as a function argument')
 
   const idleState = this.getSystemIdleState(threshold)
-
   process.nextTick(() => callback(idleState))
 }
 
 // TODO(nitsakh): Remove in 7.0
-powerMonitor.querySystemIdleTime = function (callback) {
+powerMonitor.querySystemIdleTime = function (callback: Function) {
   deprecate.warn('powerMonitor.querySystemIdleTime', 'powerMonitor.getSystemIdleTime')
-  if (typeof callback !== 'function') {
-    throw new Error('Must pass function as an argument')
-  }
+  if (typeof callback !== 'function') throw new Error('Must pass function as an argument')
 
   const idleTime = this.getSystemIdleTime()
-
   process.nextTick(() => callback(idleTime))
 }
 

+ 0 - 10
lib/browser/api/screen.js

@@ -1,10 +0,0 @@
-'use strict'
-
-const { EventEmitter } = require('events')
-const { screen, Screen } = process.electronBinding('screen')
-
-// Screen is an EventEmitter.
-Object.setPrototypeOf(Screen.prototype, EventEmitter.prototype)
-EventEmitter.call(screen)
-
-module.exports = screen

+ 10 - 0
lib/browser/api/screen.ts

@@ -0,0 +1,10 @@
+'use strict'
+
+import { createLazyInstance } from '../utils'
+const { EventEmitter } = require('events')
+const { Screen, createScreen } = process.electronBinding('screen')
+
+// Screen is an EventEmitter.
+Object.setPrototypeOf(Screen.prototype, EventEmitter.prototype)
+
+module.exports = createLazyInstance(createScreen, Screen, true)

+ 38 - 0
lib/browser/utils.ts

@@ -0,0 +1,38 @@
+import { EventEmitter } from 'events'
+
+/**
+* Creates a lazy instance of modules that can't be required before the
+* app 'ready' event by returning a proxy object to mitigate side effects
+* on 'require'
+*
+* @param {Function} creator - Function that creates a new module instance
+* @param {Object} holder - the object holding the module prototype
+* @param {Boolean} isEventEmitter - whether or not the module is an EventEmitter
+* @returns {Object} - a proxy object for the
+*/
+
+export function createLazyInstance (
+  creator: Function,
+  holder: Object,
+  isEventEmitter: Boolean
+) {
+  let lazyModule: Object
+  const module: any = {}
+  for (const method in (holder as any).prototype) {
+    module[method] = (...args: any) => {
+      // create new instance of module at runtime if none exists
+      if (!lazyModule) {
+        lazyModule = creator()
+        if (isEventEmitter) EventEmitter.call(lazyModule as any)
+      }
+
+      // check for properties on the prototype chain that aren't functions
+      if (typeof (lazyModule as any)[method] !== 'function') {
+        return (lazyModule as any)[method]
+      }
+
+      return (lazyModule as any)[method](...args)
+    }
+  }
+  return module
+}

+ 1 - 1
spec-main/events-helpers.ts

@@ -1,4 +1,4 @@
-import { EventEmitter } from "electron";
+import { EventEmitter } from 'electron';
 
 /**
  * @fileoverview A set of helper functions to make it easier to work