Skip to content

Commit

Permalink
Simplify Chrome API wrapper
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
robertknight committed Oct 2, 2024
1 parent f756458 commit 8ce28a8
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 72 deletions.
81 changes: 15 additions & 66 deletions src/background/chrome-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
* - Provide a seam that can be easily mocked in tests
*/

type Callback<Result> = (r: Result) => void;

/**
* Wrap the browser APIs exposed via the `chrome` object to return promises.
*
Expand All @@ -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<Args extends any[], Result>(
fn: (...args: [...Args, Callback<Result>]) => void,
): (...args: Args) => Promise<Result> {
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<Args extends any[], Result>(
fn: (...args: Args) => Promise<Result>,
): (...args: Args) => Promise<Result> {
// @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: {
Expand All @@ -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: {
Expand All @@ -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),
},
},

Expand All @@ -145,7 +94,7 @@ export function getChromeAPI(chrome = globalThis.chrome) {

get webNavigation() {
return {
getAllFrames: promisifyAlt(chrome.webNavigation.getAllFrames),
getAllFrames: chrome.webNavigation.getAllFrames,
};
},
};
Expand Down
15 changes: 9 additions & 6 deletions tests/background/chrome-api-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand All @@ -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 {
Expand All @@ -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', () => {
Expand All @@ -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();
Expand Down

0 comments on commit 8ce28a8

Please sign in to comment.