Lusito / webextension-polyfill-ts

This is a TypeScript ready "wrapper" for the WebExtension browser API Polyfill by Mozilla
zlib License
393 stars 23 forks source link

Add generics to `browser.runtime.sendMessage` #86

Open marcelreppi opened 1 year ago

marcelreppi commented 1 year ago

Hey,

I would love to use browser.runtime.sendMessage the following way so that I can specify the type of the message that I am sending.

  browser.runtime.sendMessage<LogMessage>({
    command: "log",
    logData,
  })

My current workaround is this but generics would be much nicer

  browser.runtime.sendMessage({
    command: "log",
    logData,
  } as LogMessage)

Also for browser.runtime.onMessage.addListener it would be useful to be able to specify the type of the callback argument.

browser.runtime.onMessage.addListener<LogMessage>(async message => {
  // It would be nice to have auto-completion for message.logData
})

Would it be possible to add this?

Thanks a lot!

Lusito commented 1 year ago

Interesting idea, but I'm not sure, how much work it would be to integrate that into the code generator. I'll check when I have some time.

For the time being, I would advise to not use the as LogMessage, as it is not fully type-safe. Try using the satisfies keyword from TypeScript 4.9 if you can: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-9.html

marcelreppi commented 1 year ago

The idea originally comes from me using this web-ext-types package which used generics for exactly this use case. I had my code written this way but when I switched to your more up-to-date type package, I had to remove the generics and adapt my implementation.

I think it would be great to provide this possibility because right now the message is of type any and it would be nice to be more precise about the message without using additional keywords such as as or satisfies.

Thanks a lot for your great tip! I will use it to improve my workaround for now 👍🏼

bnn1 commented 11 months ago

browser.runtime.onMessage.addListener callback's 3rd argument sendResponse should also be typed. Rn it's signature is

sendResponse: () => void

but the correct signature would be

sendResponse: <T>(data: T) => void

More info: https://github.com/wxt-dev/wxt/issues/299

Lusito commented 6 months ago

I'll add the changes for sendMessage and sendNativeMessage. That part seems to be straight forward.

However, onMessage is a different story. After all, it's not just about sendResponse, but also about the return-type and the message paramter as well. Currently, I have no neat typescript way to do this with the existing API due to the nature of the Events.Event type. Feel free to offer ideas.

I can offer you a workaround for that though:

// Add this somewhere in your code:
function typedEventListener<TMessage, TResponse>(listener: (
                message: TMessage,
                sender: MessageSender,
                sendResponse: (message: TResponse) => void,
            ) => Promise<TResponse> | true | void) {
    return listener as (
                message: unknown,
                sender: MessageSender,
                sendResponse: (message: unknown) => void,
            ) => Promise<unknown> | true | void;
};

// Then wrap your listeners like this:
browser.onMessage.addListener(typedEventListener<{foo: string}, { bar: string }>((message, sender, sendResponse) => {
    sendResponse({ bar: "true" });
    return true;
}));

// Or promise-based:
x.onMessage.addListener(typedEventListener<{foo: string}, { bar: string }>((message, sender, sendResponse) => {
    return Promise.resolve({ bar: "true" });
}));
marcelreppi commented 6 months ago

Cool! I really appreciate the effort and your suggested workaround!

namukang commented 5 days ago

I'll add the changes for sendMessage and sendNativeMessage. That part seems to be straight forward.

However, onMessage is a different story. After all, it's not just about sendResponse, but also about the return-type and the message paramter as well. Currently, I have no neat typescript way to do this with the existing API due to the nature of the Events.Event type. Feel free to offer ideas.

I can offer you a workaround for that though:

// Add this somewhere in your code:
function typedEventListener<TMessage, TResponse>(listener: (
                message: TMessage,
                sender: MessageSender,
                sendResponse: (message: TResponse) => void,
            ) => Promise<TResponse> | true | void) {
    return listener as (
                message: unknown,
                sender: MessageSender,
                sendResponse: (message: unknown) => void,
            ) => Promise<unknown> | true | void;
};

// Then wrap your listeners like this:
browser.onMessage.addListener(typedEventListener<{foo: string}, { bar: string }>((message, sender, sendResponse) => {
    sendResponse({ bar: "true" });
    return true;
}));

// Or promise-based:
x.onMessage.addListener(typedEventListener<{foo: string}, { bar: string }>((message, sender, sendResponse) => {
    return Promise.resolve({ bar: "true" });
}));

@Lusito Is there a reason why in the library, the type for the listener passed into browser.runtime.onMessage.addListener has to return undefined rather than void?

https://github.com/Lusito/webextension-polyfill-ts/blob/75f8b24b455cf0634561b0e9c63bf0b2c609adc6/out/namespaces/runtime.d.ts#L583

I was expecting it to allow returning void like in your comment but the library requires undefined so I've needed to explicitly return undefined in my listeners to avoid type errors.

Upon further investigation, this looks like a regression so I filed a new issue for it here: https://github.com/Lusito/webextension-polyfill-ts/issues/109