wxt-dev / wxt

⚡ Next-gen Web Extension Framework
https://wxt.dev
MIT License
7.25k stars 330 forks source link

Add support for Firefox-specific APIs #1652

Open XFox111 opened 1 month ago

XFox111 commented 1 month ago

Feature Request

Add types for browser-specific APIs and make them separate. For example:

// This API is cross-browser compatible, so it can be accessed via `browser`
await browser.tabs.create({ url: "https://github.com" });

// This is Chrome-specific API, which is not accessible on Firefox
await chrome.sidePanel.open({ windowId: 123 });

// This is Firefox-specific API, which is not accessible on Chrome
await firefox.sidebarAction.open();

Right now, since 0.20.0, browser implements only Chrome API types, which makes Firefox-specific API harder to access.

Ideally, if possible, it would be also a good idea to setup some linting for that, in case an extension tries to access browser-specific API without first checking which browser it is currently running on.

Is your feature request related to a bug?

1651

What are the alternatives?

Bring back webextension-polyfill. That way we can use browser for cross-compatible/Firefox-specific APIs, and chrome for Chrome-specific ones.

Additional context

Chrome and Firefox have quite a big set of incompatible APIs, and it would be nice to have access to them as well

XFox111 commented 1 month ago

Ideally, if possible, it would be also a good idea to setup some linting for that, in case an extension tries to access browser-specific API without first checking which browser it is currently running on.

This could be achieved by setting firefox and chrome to null based on the browser it was compiled for. That way, instead of doing this:

if (import.meta.env.FIREFOX)
    firefox.sidebarAction.open();
else
    chrome.sidePanel.open({...});

Which isn't type-safe, we could do this:

if (firefox)
    firefox.sidebarAction.open();
else if (chrome)
    chrome.sidePanel.open({...});

Which doesn't look too good either, but at least it provides type safety

honwhy commented 2 weeks ago

should wxt do dynamic dispatch ?

browser.sidePanel.open()

call chrome.sidePanel.open when in chrome, call browser.sidebarAction.open when in firefox

aklinker1 commented 2 weeks ago

So what are the main concerns here, and what are we trying to solve?

See https://github.com/wxt-dev/wxt/pull/1661#issuecomment-2949782376 for context. I'd like to get an understanding of what people want so we can build the correct solution.

I would prefer we build everything into the single browser variable, and you should use feature detection, NOT environment variables or if (firefox) to decide if an API exists... because what about Safari? Or any other future browser that provides custom extension APIs in the future. Or what about new manifest versions with new APIs? There's lots of different variables outside which browser the code is running on that determine if an API exists.

Let's look at the example from the original issue description:

// This is Chrome-specific API, which is not accessible on Firefox
await chrome.sidePanel.open({ windowId: 123 });

// This is Firefox-specific API, which is not accessible on Chrome
await firefox.sidebarAction.open();

I would prefer my code to look like this:

await browser.sidePanel?.open({ windowId: 123 });
await browser.sidebarAction?.open();

Each project is going to run in different browsers, in different manifest versions, etc., and I don't think it's feasible to provide 100% true type-safety when accessing the APIs. In the case above, I know that my hypothetical extension runs on Safari, Firefox, and Chrome. I know that browser.sidePanel won't exist on Firefox or Safari because they're MV2, so I add the optional chaining to both the APIs so everything runs safely.

Since there will always be some responsibility on the dev to know where their code is running and which APIs are available where, I prefer to take the same approach as @types/chrome and just say everything always exists, relying on the developer to know if it actually exists or not at runtime. Honestly, as basically the sole contributor to WXT, this also appeals to me from a maintainability standpoint.


TLDR:

SimYunSup commented 2 weeks ago

I didn't properly consider type-safety. As you said, it seems like we need Feature Detection. I'll also look for other ways to implement it.

SimYunSup commented 2 weeks ago

@aklinker1 Following your advice, I've implemented a system that encourages feature detection( #1661 updated ). Using ts-morph, I automatically generate TypeScript definitions based on data from @types/chrome and @types/firefox-webext-browser. If an API is not implemented in all major browsers(detect via @mdn/browser-compat-data), it's marked as an optional property, otherwise, it remains a required property.

XFox111 commented 2 weeks ago

should wxt do dynamic dispatch ?

call chrome.sidePanel.open when in chrome, call browser.sidebarAction.open when in firefox

I don't think that this is a good idea, since both of these APIs have different signatures. And this is true for most platform-specific APIs

XFox111 commented 2 weeks ago

@aklinker1 yeah, I agree. On the second though, chrome/firefox approach doesn't seem like the best idea.

In this case, it makes sense to get all of these APIs under browser and just make those which are platform-specific explicitly nullable.

Though I see several issues with this approach:

  1. browser type definition should not rely on 3rd party implementations to keep the API consistent. That means that this should be a homegrown implementation of browser which will require maintenance to keep its definitions up to date. 1.1. Documentation for the API should be slightly modified in order to inform developers that a specific API is platform dependent.
  2. Combining APIs like this, may lead to confusing signatures. For example, browser.tabs.captureVisibleTab and chrome.tabs.captureVisibleTab have slightly different options argument which, in this case, can be circumvented by adding compatibility notice to the docs, but I'm pretty sure that there're other more severe instances of this issue

Since there will always be some responsibility on the dev to know where their code is running and which APIs are available where, I prefer to take the same approach as @types/chrome and just say everything always exists, relying on the developer to know if it actually exists or not at runtime. Honestly, as basically the sole contributor to WXT, this also appeals to me from a maintainability standpoint.

I somewhat disagree. Imo, the approach should be more like "We know that thing X should be there, but we don't know if it actually is". Which implies that API parts should be nullable and prompt developer either to make sure that they are there (like this: await browser.sidePanel?.open({ windowId: 123 });) or to just tank it with await browser.sidePanel!.open({ windowId: 123 });. In either case, it's now 100% developer's fault if something goes wrong because they have been warned.

To be honest, I think that the best approach could be to create a new API library that would provide a layer of unified APIs and leave developers an option to use @types/chrome or webextension-polyfill instead, if they want. This does require quite a lot of work though.

aklinker1 commented 2 weeks ago

I somewhat disagree. Imo, the approach should be more like "We know that thing X should be there, but we don't know if it actually is". Which implies that API parts should be nullable and prompt developer either to make sure that they are there (like this: await browser.sidePanel?.open({ windowId: 123 });) or to just tank it with await browser.sidePanel!.open({ windowId: 123 });. In either case, it's now 100% developer's fault if something goes wrong because they have been warned.

@XFox111 Hmm, OK, you've almost convinced me to make these APIs optional. Kinda inconvenient if you're making an extension for a single browser, but WXT goal is making extensions that work on all browsers, so that makes sense.

There's one last problem... permissions. Even if we use an implementation like @SimYunSup's PR, or DIY it like you suggest, it won't be type-safe if we assume all the APIs exist, like browser.storage. Even some API signatures change, like the Tab type, change when the tabs permission is requested.

To be honest, I think that the best approach could be to create a new API library that would provide a layer of unified APIs and leave developers an option to use @types/chrome or webextension-polyfill instead, if they want. This does require quite a lot of work though.

Yeah, If we decide to go down this road, I would pull this package out into it's own repo in that case, so issues and stuff can be tracked separately.

aklinker1 commented 2 weeks ago

This only thing I can think of to handle permissions is to build a factory function that returns a dynamic type based on the permissions used. At which point... why not also provide a list of targets (chrome-mv3, firefox-mv2, etc) that can also be used to determine if an API is available...

export function defineBrowser<TPermissions extends Permission[], TTargets extends Target[]>(): Browser<TPermissions, TTargets> {
  return browser as any.
}

I'm not sure I want to commit time to maintaining something like this. Plus, I don't think it'd be possible to use namespaces like @types/chrome... I need to checkout @SimYunSup PR today to see how he merged things together.

XFox111 commented 2 weeks ago

That's a good point.

In theory, we can also use WXT's type generator. This way it should go something like this:

  1. WXT has a typedef template for all existing APIs (something like @types/chrome)
  2. Developer specifies in wxt.config.ts their permissions and target browsers
  3. When hit with wxt prepare, the framework generates api.d.ts or something, based on the template and active browsers/permissions and exports it globally
XFox111 commented 2 weeks ago

I can try and build a prototype for the type generator. Though it will take me a week or two, since I have some other stuff going on right now.