From 737e3de3fa34acd9b9222a898de44cf233ca9575 Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Mon, 9 Oct 2023 01:55:16 +0200 Subject: [PATCH] chore: restore deprecate as an internal module (#40124) * Revert "refactor: don't expose deprecate as an internal module (#35311)" This reverts commit 8424779906ae33222764db9ed837f55115143e71. * check crashed event warnings --- filenames.auto.gni | 5 ++- lib/browser/api/app.ts | 3 +- lib/browser/api/crash-reporter.ts | 3 +- lib/browser/api/web-contents.ts | 3 +- lib/common/{ => api}/deprecate.ts | 41 ++++++++++++++++------- lib/common/api/module-list.ts | 4 ++- lib/common/define-properties.ts | 2 +- lib/sandboxed_renderer/api/module-list.ts | 6 ++++ spec/api-app-spec.ts | 16 ++++++--- spec/api-web-contents-spec.ts | 19 ++++++++++- spec/deprecate-spec.ts | 2 +- typings/internal-electron.d.ts | 29 ++++++++++++++++ 12 files changed, 106 insertions(+), 27 deletions(-) rename lib/common/{ => api}/deprecate.ts (74%) diff --git a/filenames.auto.gni b/filenames.auto.gni index 1d89fc05e329..02837fff6d6a 100644 --- a/filenames.auto.gni +++ b/filenames.auto.gni @@ -142,6 +142,7 @@ auto_filenames = { ] sandbox_bundle_deps = [ + "lib/common/api/deprecate.ts", "lib/common/api/native-image.ts", "lib/common/define-properties.ts", "lib/common/ipc-messages.ts", @@ -247,11 +248,11 @@ auto_filenames = { "lib/browser/parse-features-string.ts", "lib/browser/rpc-server.ts", "lib/browser/web-view-events.ts", + "lib/common/api/deprecate.ts", "lib/common/api/module-list.ts", "lib/common/api/native-image.ts", "lib/common/api/shell.ts", "lib/common/define-properties.ts", - "lib/common/deprecate.ts", "lib/common/init.ts", "lib/common/ipc-messages.ts", "lib/common/reset-search-paths.ts", @@ -265,6 +266,7 @@ auto_filenames = { ] renderer_bundle_deps = [ + "lib/common/api/deprecate.ts", "lib/common/api/module-list.ts", "lib/common/api/native-image.ts", "lib/common/api/shell.ts", @@ -303,6 +305,7 @@ auto_filenames = { ] worker_bundle_deps = [ + "lib/common/api/deprecate.ts", "lib/common/api/module-list.ts", "lib/common/api/native-image.ts", "lib/common/api/shell.ts", diff --git a/lib/browser/api/app.ts b/lib/browser/api/app.ts index 5d7ce57880b7..0fe34383c9d4 100644 --- a/lib/browser/api/app.ts +++ b/lib/browser/api/app.ts @@ -1,7 +1,6 @@ import * as fs from 'fs'; -import { Menu } from 'electron/main'; -import * as deprecate from '@electron/internal/common/deprecate'; +import { Menu, deprecate } from 'electron/main'; const bindings = process._linkedBinding('electron_browser_app'); const commandLine = process._linkedBinding('electron_common_command_line'); diff --git a/lib/browser/api/crash-reporter.ts b/lib/browser/api/crash-reporter.ts index 729d9fe53db1..16a9b1b5cb0f 100644 --- a/lib/browser/api/crash-reporter.ts +++ b/lib/browser/api/crash-reporter.ts @@ -1,5 +1,4 @@ -import { app } from 'electron/main'; -import * as deprecate from '@electron/internal/common/deprecate'; +import { app, deprecate } from 'electron/main'; const binding = process._linkedBinding('electron_browser_crash_reporter'); diff --git a/lib/browser/api/web-contents.ts b/lib/browser/api/web-contents.ts index 7c876d5317cf..307530b41ca5 100644 --- a/lib/browser/api/web-contents.ts +++ b/lib/browser/api/web-contents.ts @@ -1,4 +1,4 @@ -import { app, ipcMain, session, webFrameMain } from 'electron/main'; +import { app, ipcMain, session, webFrameMain, deprecate } from 'electron/main'; import type { BrowserWindowConstructorOptions, LoadURLOptions } from 'electron/main'; import * as url from 'url'; @@ -10,7 +10,6 @@ import * as ipcMainUtils from '@electron/internal/browser/ipc-main-internal-util import { MessagePortMain } from '@electron/internal/browser/message-port-main'; import { IPC_MESSAGES } from '@electron/internal/common/ipc-messages'; import { IpcMainImpl } from '@electron/internal/browser/ipc-main-impl'; -import * as deprecate from '@electron/internal/common/deprecate'; // session is not used here, the purpose is to make sure session is initialized // before the webContents module. diff --git a/lib/common/deprecate.ts b/lib/common/api/deprecate.ts similarity index 74% rename from lib/common/deprecate.ts rename to lib/common/api/deprecate.ts index dc4e9d80ca86..12dc31122c18 100644 --- a/lib/common/deprecate.ts +++ b/lib/common/api/deprecate.ts @@ -2,13 +2,13 @@ type DeprecationHandler = (message: string) => void; let deprecationHandler: DeprecationHandler | null = null; -export function warnOnce (oldName: string, newName?: string) { +function warnOnce (oldName: string, newName?: string) { return warnOnceMessage(newName ? `'${oldName}' is deprecated and will be removed. Please use '${newName}' instead.` : `'${oldName}' is deprecated and will be removed.`); } -export function warnOnceMessage (msg: string) { +function warnOnceMessage (msg: string) { let warned = false; return () => { if (!warned && !process.noDeprecation) { @@ -18,21 +18,21 @@ export function warnOnceMessage (msg: string) { }; } -export function setHandler (handler: DeprecationHandler | null): void { +function setHandler (handler: DeprecationHandler | null): void { deprecationHandler = handler; } -export function getHandler (): DeprecationHandler | null { +function getHandler (): DeprecationHandler | null { return deprecationHandler; } -export function warn (oldName: string, newName: string): void { +function warn (oldName: string, newName: string): void { if (!process.noDeprecation) { log(`'${oldName}' is deprecated. Use '${newName}' instead.`); } } -export function log (message: string): void { +function log (message: string): void { if (typeof deprecationHandler === 'function') { deprecationHandler(message); } else if (process.throwDeprecation) { @@ -45,7 +45,7 @@ export function log (message: string): void { } // remove a function with no replacement -export function removeFunction (fn: T, removedName: string): T { +function removeFunction (fn: T, removedName: string): T { if (!fn) { throw new Error(`'${removedName} function' is invalid or does not exist.`); } // wrap the deprecated function to warn user @@ -57,7 +57,7 @@ export function removeFunction (fn: T, removedName: string): } // change the name of a function -export function renameFunction (fn: T, newName: string): T { +function renameFunction (fn: T, newName: string): T { const warn = warnOnce(`${fn.name} function`, `${newName} function`); return function (this: any) { warn(); @@ -66,7 +66,7 @@ export function renameFunction (fn: T, newName: string): T { } // change the name of an event -export function event (emitter: NodeJS.EventEmitter, oldName: string, newName: string, transformer: (...args: any[]) => any[] | undefined = (...args) => args) { +function event (emitter: NodeJS.EventEmitter, oldName: string, newName: string, transformer: (...args: any[]) => any[] | undefined = (...args) => args) { const warn = newName.startsWith('-') /* internal event */ ? warnOnce(`${oldName} event`) : warnOnce(`${oldName} event`, `${newName} event`); @@ -82,7 +82,7 @@ export function event (emitter: NodeJS.EventEmitter, oldName: string, newName: s } // remove a property with no replacement -export function removeProperty(object: T, removedName: K, onlyForValues?: any[]): T { +function removeProperty(object: T, removedName: K, onlyForValues?: any[]): T { // if the property's already been removed, warn about it // eslint-disable-next-line no-proto const info = Object.getOwnPropertyDescriptor((object as any).__proto__, removedName); @@ -113,7 +113,7 @@ export function removeProperty(object: T, remov } // change the name of a property -export function renameProperty(object: T, oldName: string, newName: K): T { +function renameProperty(object: T, oldName: string, newName: K): T { const warn = warnOnce(oldName, newName); // if the new property isn't there yet, @@ -137,10 +137,27 @@ export function renameProperty(o }); } -export function moveAPI (fn: T, oldUsage: string, newUsage: string): T { +function moveAPI (fn: T, oldUsage: string, newUsage: string): T { const warn = warnOnce(oldUsage, newUsage); return function (this: any) { warn(); return fn.apply(this, arguments); } as unknown as typeof fn; } + +const deprecate: ElectronInternal.DeprecationUtil = { + warnOnce, + warnOnceMessage, + setHandler, + getHandler, + warn, + log, + removeFunction, + renameFunction, + event, + removeProperty, + renameProperty, + moveAPI +}; + +export default deprecate; diff --git a/lib/common/api/module-list.ts b/lib/common/api/module-list.ts index 6383c33cd8c9..202c38b3a7c5 100644 --- a/lib/common/api/module-list.ts +++ b/lib/common/api/module-list.ts @@ -1,5 +1,7 @@ // Common modules, please sort alphabetically export const commonModuleList: ElectronInternal.ModuleEntry[] = [ { name: 'nativeImage', loader: () => require('./native-image') }, - { name: 'shell', loader: () => require('./shell') } + { name: 'shell', loader: () => require('./shell') }, + // The internal modules, invisible unless you know their names. + { name: 'deprecate', loader: () => require('./deprecate'), private: true } ]; diff --git a/lib/common/define-properties.ts b/lib/common/define-properties.ts index ad9e08d4fd95..add72cd84665 100644 --- a/lib/common/define-properties.ts +++ b/lib/common/define-properties.ts @@ -9,7 +9,7 @@ export function defineProperties (targetExports: Object, moduleList: ElectronInt const descriptors: PropertyDescriptorMap = {}; for (const module of moduleList) { descriptors[module.name] = { - enumerable: true, + enumerable: !module.private, get: handleESModule(module.loader) }; } diff --git a/lib/sandboxed_renderer/api/module-list.ts b/lib/sandboxed_renderer/api/module-list.ts index 09772fd72e1d..91d075eebc1c 100644 --- a/lib/sandboxed_renderer/api/module-list.ts +++ b/lib/sandboxed_renderer/api/module-list.ts @@ -18,5 +18,11 @@ export const moduleList: ElectronInternal.ModuleEntry[] = [ { name: 'webFrame', loader: () => require('@electron/internal/renderer/api/web-frame') + }, + // The internal modules, invisible unless you know their names. + { + name: 'deprecate', + loader: () => require('@electron/internal/common/api/deprecate'), + private: true } ]; diff --git a/spec/api-app-spec.ts b/spec/api-app-spec.ts index f77b73b56acc..fabd5b3dc5de 100644 --- a/spec/api-app-spec.ts +++ b/spec/api-app-spec.ts @@ -6,7 +6,7 @@ import * as net from 'node:net'; import * as fs from 'fs-extra'; import * as path from 'node:path'; import { promisify } from 'node:util'; -import { app, BrowserWindow, Menu, session, net as electronNet, WebContents } from 'electron/main'; +import { app, BrowserWindow, Menu, session, net as electronNet, WebContents, deprecate } from 'electron/main'; import { closeWindow, closeAllWindows } from './lib/window-helpers'; import { ifdescribe, ifit, listen, waitUntil } from './lib/spec-helpers'; import { once } from 'node:events'; @@ -492,7 +492,10 @@ describe('app module', () => { describe('BrowserWindow events', () => { let w: BrowserWindow = null as any; - afterEach(() => closeWindow(w).then(() => { w = null as any; })); + afterEach(() => { + deprecate.setHandler(null); + closeWindow(w).then(() => { w = null as any; }); + }); it('should emit browser-window-focus event when window is focused', async () => { const emitted = once(app, 'browser-window-focus') as Promise<[any, BrowserWindow]>; @@ -535,11 +538,16 @@ describe('app module', () => { }); await w.loadURL('about:blank'); - const emitted = once(app, 'renderer-process-crashed') as Promise<[any, WebContents]>; + const messages: string[] = []; + deprecate.setHandler(message => messages.push(message)); + + const emitted = once(app, 'renderer-process-crashed') as Promise<[any, WebContents, boolean]>; w.webContents.executeJavaScript('process.crash()'); - const [, webContents] = await emitted; + const [, webContents, killed] = await emitted; expect(webContents).to.equal(w.webContents); + expect(killed).to.be.false(); + expect(messages).to.deep.equal(['\'renderer-process-crashed event\' is deprecated and will be removed. Please use \'render-process-gone event\' instead.']); }); // FIXME: re-enable this test on win32. diff --git a/spec/api-web-contents-spec.ts b/spec/api-web-contents-spec.ts index 8f8fc0439757..9b59f9bce33f 100644 --- a/spec/api-web-contents-spec.ts +++ b/spec/api-web-contents-spec.ts @@ -3,7 +3,7 @@ import { AddressInfo } from 'node:net'; import * as path from 'node:path'; import * as fs from 'node:fs'; import * as http from 'node:http'; -import { BrowserWindow, ipcMain, webContents, session, app, BrowserView, WebContents } from 'electron/main'; +import { BrowserWindow, ipcMain, webContents, session, app, BrowserView, WebContents, deprecate } from 'electron/main'; import { closeAllWindows } from './lib/window-helpers'; import { ifdescribe, defer, waitUntil, listen, ifit } from './lib/spec-helpers'; import { once } from 'node:events'; @@ -2331,6 +2331,8 @@ describe('webContents module', () => { }); describe('crashed event', () => { + afterEach(() => deprecate.setHandler(null)); + it('does not crash main process when destroying WebContents in it', async () => { const contents = (webContents as typeof ElectronInternal.WebContents).create({ nodeIntegration: true }); const crashEvent = once(contents, 'render-process-gone'); @@ -2339,6 +2341,21 @@ describe('webContents module', () => { await crashEvent; contents.destroy(); }); + + it('logs a warning', async () => { + const contents = (webContents as typeof ElectronInternal.WebContents).create({ nodeIntegration: true }); + await contents.loadURL('about:blank'); + + const messages: string[] = []; + deprecate.setHandler(message => messages.push(message)); + + const crashEvent = once(contents, 'crashed'); + contents.forcefullyCrashRenderer(); + const [, killed] = await crashEvent; + + expect(killed).to.be.a('boolean'); + expect(messages).to.deep.equal(['\'crashed event\' is deprecated and will be removed. Please use \'render-process-gone event\' instead.']); + }); }); describe('context-menu event', () => { diff --git a/spec/deprecate-spec.ts b/spec/deprecate-spec.ts index b8e3ae9b1d85..c45c338e26c6 100644 --- a/spec/deprecate-spec.ts +++ b/spec/deprecate-spec.ts @@ -1,5 +1,5 @@ import { expect } from 'chai'; -import * as deprecate from '../lib/common/deprecate'; +import { deprecate } from 'electron/main'; describe('deprecate', () => { let throwing: boolean; diff --git a/typings/internal-electron.d.ts b/typings/internal-electron.d.ts index caffd8d56348..2ccdecd86964 100644 --- a/typings/internal-electron.d.ts +++ b/typings/internal-electron.d.ts @@ -155,6 +155,18 @@ declare namespace Electron { _replyChannel: ReplyChannel; } + namespace Common { + const deprecate: ElectronInternal.DeprecationUtil; + } + + namespace Main { + const deprecate: ElectronInternal.DeprecationUtil; + } + + namespace Renderer { + const deprecate: ElectronInternal.DeprecationUtil; + } + class View {} // Experimental views API @@ -200,6 +212,22 @@ declare namespace Electron { } declare namespace ElectronInternal { + type DeprecationHandler = (message: string) => void; + interface DeprecationUtil { + warnOnce(oldName: string, newName?: string): () => void; + warnOnceMessage(msg: string): () => void; + setHandler(handler: DeprecationHandler | null): void; + getHandler(): DeprecationHandler | null; + warn(oldName: string, newName: string): void; + log(message: string): void; + removeFunction(fn: T, removedName: string): T; + renameFunction(fn: T, newName: string): T; + event(emitter: NodeJS.EventEmitter, oldName: string, newName: string, transformer?: (...args: any[]) => any[] | undefined): void; + removeProperty(object: T, propertyName: K, onlyForValues?: any[]): T; + renameProperty(object: T, oldName: string, newName: K): T; + moveAPI(fn: T, oldUsage: string, newUsage: string): T; + } + interface DesktopCapturer { startHandling(captureWindow: boolean, captureScreen: boolean, thumbnailSize: Electron.Size, fetchWindowIcons: boolean): void; _onerror?: (error: string) => void; @@ -264,6 +292,7 @@ declare namespace ElectronInternal { interface ModuleEntry { name: string; loader: ModuleLoader; + private?: boolean; } interface UtilityProcessWrapper extends NodeJS.EventEmitter {