Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

browser.runtime.onMessage.addListener callback's 3rd argument sendResponse has incorrect type #299

Open
1 task done
bnn1 opened this issue Dec 25, 2023 · 4 comments
Open
1 task done
Labels
bug Something isn't working upstream Issue related to an upstream library
Milestone

Comments

@bnn1
Copy link

bnn1 commented Dec 25, 2023

Describe the bug

The third callback argument sendResponse of browser.runtime.onMessage.addListener has incorrect signature:

sendResponse: () => void

To Reproduce

Reproduction

Steps to reproduce the behavior:

  1. Open entrypoints>background.ts
  2. See TS error "Expected 0 arguments, but got 1.ts(2554)"

OR

  1. add onMessage listener:
browser.runtime.onMessage.addListener((message, sender, sendResponse) => {
  console.log("Message!:", message)
  sendResponse("Sup brother?")
})
  1. See TS error "Expected 0 arguments, but got 1.ts(2554)"
  2. But response is actually send back

Expected behavior

sendMessage should accept any, unknown or generic data type

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

Screenshots

image
image
image

Environment

  System:
    OS: Linux 6.6 EndeavourOS
    CPU: (12) x64 AMD Ryzen 5 4600H with Radeon Graphics
    Memory: 17.76 GB / 30.73 GB
    Container: Yes
    Shell: 5.9 - /usr/bin/zsh
  Binaries:
    Node: 18.14.2 - ~/.nvm/versions/node/v18.14.2/bin/node
    Yarn: 1.22.21 - ~/.nvm/versions/node/v18.14.2/bin/yarn
    npm: 9.6.0 - ~/.nvm/versions/node/v18.14.2/bin/npm
    pnpm: 8.12.1 - ~/.nvm/versions/node/v18.14.2/bin/pnpm
    bun: 1.0.0 - ~/.bun/bin/bun
  npmPackages:
    wxt: ^0.12.0 => 0.12.5 

Contribution

  • I would like to fix this BUG via a PR
@bnn1 bnn1 added the triage Bug or problem that needs to be looked into label Dec 25, 2023
@aklinker1
Copy link
Collaborator

aklinker1 commented Dec 25, 2023

Thanks for the report @bnn1! This is likely an upstream issue with @types/webextension-polyfill.

In the meantime, you could use the promise API without sendResponse.

Screenshot_20231225-133710.png

https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/runtime/sendMessage#examples

browser.runtime.onMessage.addListener(() => {
  return Promise.resolve("greetings");
});

// or use an async callback

browser.runtime.onMessage.addListener(async () => {
  return "greetings";
});

@aklinker1 aklinker1 added bug Something isn't working and removed triage Bug or problem that needs to be looked into labels Dec 25, 2023
@aklinker1 aklinker1 added the upstream Issue related to an upstream library label Dec 25, 2023
@dvlden
Copy link
Contributor

dvlden commented Dec 26, 2023

Can someone explain to me why message listener can return something?
On Chrome API I clearly see that it can return only boolean or undefined but on FF it says return Promise...

What is the return for?

@aklinker1
Copy link
Collaborator

aklinker1 commented Dec 26, 2023

@dvlden maybe you want to make a fetch request from a content script, but it's blocked by CORS. To get around this, you can send a message to the background and perform the fetch request there instead, where CORS is disabled. So the background, the message receiver, needs to be able to send the response back to the content script.

If you need to do async work, like a fetch request, then:

  • On Chrome, you return true from the message listener, informing the browser a response will be sent. Then you use callbacks to call sendResponse once the async work has finished
  • On Firefox, you just return a promise of the response

The polyfill supports both ways of sending a response on both browsers. I think promises are simpler, so I use the Firefox approach in my code.

That said, if you're doing anything complex, I would recommend using a library over doing it yourself. WXT will include a messaging library in the future, simplifying all this.

@aklinker1
Copy link
Collaborator

aklinker1 commented Jan 8, 2024

Was gonna open a PR to webextension-polyfill-ts, but this was already taken care of, see Lusito/webextension-polyfill-ts#96. Just hasn't been released to @types/webextension-polyfill yet.

Pinged the maintainer and offered to help

@aklinker1 aklinker1 added this to the v1.0 milestone Mar 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working upstream Issue related to an upstream library
Projects
None yet
Development

No branches or pull requests

3 participants