From 8ce28a88e4d8d084781cee11f8db82d0c9ebe0f0 Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Wed, 2 Oct 2024 13:37:31 +0100 Subject: [PATCH] Simplify Chrome API wrapper Since the migration to Manifest V3 is now complete, all of the `chrome` APIs support returning promises, so we don't need to wrap the older callback-returning APIs to return promises. This simplifies the code and also removes the need for a TypeScript workaround when resolving the type of wrapped functions. --- src/background/chrome-api.ts | 81 ++++++----------------------- tests/background/chrome-api-test.js | 15 +++--- 2 files changed, 24 insertions(+), 72 deletions(-) diff --git a/src/background/chrome-api.ts b/src/background/chrome-api.ts index 4b1025ae..e24cbeec 100644 --- a/src/background/chrome-api.ts +++ b/src/background/chrome-api.ts @@ -8,8 +8,6 @@ * - Provide a seam that can be easily mocked in tests */ -type Callback = (r: Result) => void; - /** * Wrap the browser APIs exposed via the `chrome` object to return promises. * @@ -24,72 +22,23 @@ export function getChromeAPI(chrome = globalThis.chrome) { return null as never; } - /** - * Cache of Promise-ified APIs. This is used so that APIs which are looked up - * on-demand, eg. because they are optional and may not exist when `getChromeAPI` - * is first called, are only created once. - */ - const cache = new Map<() => void, any>(); - - /** - * Convert an async callback-accepting Chrome API to a Promise-returning version. - * - * TypeScript may complain if the API has a Manifest V3-only overload that - * returns a Promise. Use {@link promisifyAlt} as a workaround. - * - * This wrapper can be removed once the extension becomes Manifest V3-only. - * - * @param fn - The original Chrome API that accepts a callback. - * @return Wrapped API that doesn't take a callback but returns a Promise instead - */ - function promisify( - fn: (...args: [...Args, Callback]) => void, - ): (...args: Args) => Promise { - const cached = cache.get(fn); - if (cached) { - return cached; - } - - return (...args) => { - return new Promise((resolve, reject) => { - fn(...args, (result: Result) => { - const lastError = chrome.runtime.lastError; - if (lastError) { - reject(lastError); - } else { - resolve(result); - } - }); - }); - }; - } - - function promisifyAlt( - fn: (...args: Args) => Promise, - ): (...args: Args) => Promise { - // @ts-expect-error - return promisify(fn); - } - const browserAction = chrome.browserAction ?? chrome.action; return { browserAction: { onClicked: browserAction.onClicked, - setBadgeBackgroundColor: promisify(browserAction.setBadgeBackgroundColor), - setBadgeText: promisify(browserAction.setBadgeText), - setIcon: promisify(browserAction.setIcon), - setTitle: promisify(browserAction.setTitle), + setBadgeBackgroundColor: browserAction.setBadgeBackgroundColor, + setBadgeText: browserAction.setBadgeText, + setIcon: browserAction.setIcon, + setTitle: browserAction.setTitle, }, extension: { - isAllowedFileSchemeAccess: promisify( - chrome.extension.isAllowedFileSchemeAccess, - ), + isAllowedFileSchemeAccess: chrome.extension.isAllowedFileSchemeAccess, }, management: { - getSelf: promisify(chrome.management.getSelf), + getSelf: chrome.management.getSelf, }, runtime: { @@ -103,24 +52,24 @@ export function getChromeAPI(chrome = globalThis.chrome) { // Firefox (as of v92) does not support `requestUpdateCheck`. requestUpdateCheck: chrome.runtime.requestUpdateCheck - ? promisify(chrome.runtime.requestUpdateCheck) + ? chrome.runtime.requestUpdateCheck : null, }, permissions: { - getAll: promisify(chrome.permissions.getAll), - request: promisify(chrome.permissions.request), + getAll: chrome.permissions.getAll, + request: chrome.permissions.request, }, tabs: { - create: promisify(chrome.tabs.create), - get: promisifyAlt(chrome.tabs.get), + create: chrome.tabs.create, + get: chrome.tabs.get, onCreated: chrome.tabs.onCreated, onReplaced: chrome.tabs.onReplaced, onRemoved: chrome.tabs.onRemoved, onUpdated: chrome.tabs.onUpdated, - query: promisifyAlt(chrome.tabs.query), - update: promisify(chrome.tabs.update), + query: chrome.tabs.query, + update: chrome.tabs.update, }, scripting: { @@ -131,7 +80,7 @@ export function getChromeAPI(chrome = globalThis.chrome) { // Methods of storage areas (sync, local, managed) need to be bound. // Standalone functions in Chrome API namespaces do not. sync: { - get: promisify(chrome.storage.sync.get.bind(chrome.storage.sync)), + get: chrome.storage.sync.get.bind(chrome.storage.sync), }, }, @@ -145,7 +94,7 @@ export function getChromeAPI(chrome = globalThis.chrome) { get webNavigation() { return { - getAllFrames: promisifyAlt(chrome.webNavigation.getAllFrames), + getAllFrames: chrome.webNavigation.getAllFrames, }; }, }; diff --git a/tests/background/chrome-api-test.js b/tests/background/chrome-api-test.js index 30b646a1..a2efde1a 100644 --- a/tests/background/chrome-api-test.js +++ b/tests/background/chrome-api-test.js @@ -68,13 +68,16 @@ describe('chrome-api', () => { continue; } + const expectedResult = {}; + if (!syncAPIs.has(method)) { + method.resolves(expectedResult); + } + const arg = {}; let result = chromeAPI[namespace][methodName](arg); assert.calledWith(method, arg); if (!syncAPIs.has(method)) { - const expectedResult = {}; - method.yield(expectedResult); assert.equal(await result, expectedResult); } } @@ -84,8 +87,8 @@ describe('chrome-api', () => { it('wrapped methods reject if an error occurs', async () => { const chromeAPI = getChromeAPI(fakeChrome); - fakeChrome.runtime.lastError = new Error('Something went wrong'); - fakeChrome.tabs.get.yields(null); + const expectedError = new Error('Something went wrong'); + fakeChrome.tabs.get.rejects(expectedError); let error; try { @@ -94,7 +97,7 @@ describe('chrome-api', () => { error = e; } - assert.equal(error, fakeChrome.runtime.lastError); + assert.equal(error, expectedError); }); describe('APIs that require optional permissions', () => { @@ -119,7 +122,7 @@ describe('chrome-api', () => { // Simulate the "webNavigation" permission being granted, which will // make the `chrome.webNavigation` property accessible. fakeChrome.webNavigation = { - getAllFrames: sinon.stub().yields(frames), + getAllFrames: sinon.stub().resolves(frames), }; const actualFrames = await chromeAPI.webNavigation.getAllFrames();