From de7682bba8a83b148d82fc014f16aa2cb27d0669 Mon Sep 17 00:00:00 2001 From: Greg Tatum Date: Wed, 23 Oct 2019 13:49:27 +0000 Subject: [PATCH] Bug 1588192 - Fix the TypeScript require function in the initializers; r=julienw Differential Revision: https://phabricator.services.mozilla.com/D49012 --HG-- extra : moz-landing-system : lando --- .../client/performance-new/initializer.js | 31 +++++++++++++----- .../performance-new/popup/background.jsm.js | 5 +++ .../performance-new/popup/initializer.js | 32 ++++++++++++------- devtools/client/performance-new/typescript.md | 29 +++++++++++++++++ 4 files changed, 78 insertions(+), 19 deletions(-) create mode 100644 devtools/client/performance-new/typescript.md diff --git a/devtools/client/performance-new/initializer.js b/devtools/client/performance-new/initializer.js index fa3f99951823..03e86b0121bf 100644 --- a/devtools/client/performance-new/initializer.js +++ b/devtools/client/performance-new/initializer.js @@ -3,21 +3,36 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ // @ts-check /* exported gInit, gDestroy, loader */ -"use strict"; /** * @typedef {import("./@types/perf").PerfFront} PerfFront * @typedef {import("./@types/perf").PreferenceFront} PreferenceFront * @typedef {import("./@types/perf").RecordingStateFromPreferences} RecordingStateFromPreferences */ +"use strict"; + +{ + // Create the browser loader, but take care not to conflict with + // TypeScript. See devtools/client/performance-new/typescript.md and + // the section on "Do not overload require" for more information. + + const { BrowserLoader } = ChromeUtils.import( + "resource://devtools/client/shared/browser-loader.js" + ); + const browserLoader = BrowserLoader({ + baseURI: "resource://devtools/client/performance-new/", + window, + }); + + /** + * @type {any} - Coerce the current scope into an `any`, and assign the + * loaders to the scope. They can then be used freely below. + */ + const scope = this; + scope.require = browserLoader.require; + scope.loader = browserLoader.loader; +} -const { BrowserLoader } = ChromeUtils.import( - "resource://devtools/client/shared/browser-loader.js" -); -const { require, loader } = BrowserLoader({ - baseURI: "resource://devtools/client/performance-new/", - window, -}); const Perf = require("devtools/client/performance-new/components/Perf"); const ReactDOM = require("devtools/client/shared/vendor/react-dom"); const React = require("devtools/client/shared/vendor/react"); diff --git a/devtools/client/performance-new/popup/background.jsm.js b/devtools/client/performance-new/popup/background.jsm.js index 0efa96887aab..f4dad1e339eb 100644 --- a/devtools/client/performance-new/popup/background.jsm.js +++ b/devtools/client/performance-new/popup/background.jsm.js @@ -98,6 +98,11 @@ const lazyPreferenceManagement = requireLazy(() => { * @type {Map} */ const symbolCache = new Map(); + +/** + * @param {string} debugName + * @param {string} breakpadId + */ async function getSymbolsFromThisBrowser(debugName, breakpadId) { if (symbolCache.size === 0) { // Prime the symbols cache. diff --git a/devtools/client/performance-new/popup/initializer.js b/devtools/client/performance-new/popup/initializer.js index 0831d8a364eb..246b347b6263 100644 --- a/devtools/client/performance-new/popup/initializer.js +++ b/devtools/client/performance-new/popup/initializer.js @@ -17,17 +17,27 @@ * Tools -> Web Developer -> Enable Profiler Toolbar Icon */ -/** - * Initialize the require function through a BrowserLoader. This loader ensures - * that the popup can use require and has access to the window object. - */ -const { BrowserLoader } = ChromeUtils.import( - "resource://devtools/client/shared/browser-loader.js" -); -const { require } = BrowserLoader({ - baseURI: "resource://devtools/client/performance-new/popup/", - window, -}); +{ + // Create the browser loader, but take care not to conflict with + // TypeScript. See devtools/client/performance-new/typescript.md and + // the section on "Do not overload require" for more information. + + const { BrowserLoader } = ChromeUtils.import( + "resource://devtools/client/shared/browser-loader.js" + ); + const browserLoader = BrowserLoader({ + baseURI: "resource://devtools/client/performance-new/popup", + window, + }); + + /** + * @type {any} - Coerce the current scope into an `any`, and assign the + * loaders to the scope. They can then be used freely below. + */ + const scope = this; + scope.require = browserLoader.require; + scope.loader = browserLoader.loader; +} /** * The background.jsm.js manages the profiler state, and can be loaded multiple time diff --git a/devtools/client/performance-new/typescript.md b/devtools/client/performance-new/typescript.md new file mode 100644 index 000000000000..26f6773c603d --- /dev/null +++ b/devtools/client/performance-new/typescript.md @@ -0,0 +1,29 @@ +# TypeScript Experiment + +This folder contains an experiment to add TypeScript to Gecko. The type checking is not integrated into continuous integration as of yet, and can be run manually via: + +``` +cd devtools/client/performance-new +yarn install +yarn test +``` + +Also, the types should work with editor integration. VS Code works with TypeScript by default, and should pick up the types here. + +## Do not overload require + +Anytime that our code creates the `require` function through a BrowserLoader, it can conflict with the TypeScript type system. For example: + +``` +const { require } = BrowserLoader(...); +``` + +TypeScript treats `require` as a special keyword. If the variable is defined on the page, then it shadow's TypeScript's keyword, and the require machinery will be improperly typed as an `any`. Care needs to be taken to get around this. Here is a solution of hiding the `require` function from TypeScript: + +``` +const browserLoader = BrowserLoader(...); + +/** @type {any} - */ +const scope = this; +scope.require = browserLoader.require; +```