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 only accepts 2 arguments #54

Closed
eugbyte opened this issue Apr 24, 2021 · 10 comments
Closed

browser.runtime.onMessage.addListener only accepts 2 arguments #54

eugbyte opened this issue Apr 24, 2021 · 10 comments
Labels
invalid This doesn't seem right wontfix This will not be worked on

Comments

@eugbyte
Copy link

eugbyte commented Apr 24, 2021

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

The actual browser.runtime.onMessage.addListener accepts 3 arguments:, message, sender, sendResponse. However, the current implementation only accepts 2 arguments: message, sender

@eugbyte
Copy link
Author

eugbyte commented Apr 25, 2021

created pull request here:
#55

@brnbs
Copy link

brnbs commented Apr 26, 2021

Please merge this ASAP

@Lusito
Copy link
Owner

Lusito commented Apr 26, 2021

As you can see in the page you linked: "Returning a Promise is now preferred, as sendResponse() will be removed from the W3C spec."

Please use a promise as return value instead of using sendResponse.

I'm going to keep this open, as this pops up regularly, as people don't seem to search before opening new tickets: #53, #46, #45, #31, #7

@Lusito Lusito added invalid This doesn't seem right wontfix This will not be worked on labels Apr 26, 2021
@brnbs
Copy link

brnbs commented Apr 27, 2021

I landed here from Chrome Developer website where I found no mention about returning a Promise, however I gave it a try and just wanted to confirm that this works the same as calling sendResponse:

browser.runtime.onMessage.addListener((message: string) => {
    if (message == "whatever") {
        // Do something
        return Promise.resolve("response");
    }
});

@Lusito
Copy link
Owner

Lusito commented Apr 28, 2021

Chrome documentation is not about the W3C standard. They don't care about the standard at all. If you try to target multiple browsers, I suggest you use the MDN documentation. Of course, there will be things you need to check if you want chrome-only features and for that you'll need to use the chrome documentation.

@linonetwo
Copy link

This makes it not easy to pass Observable from backgroundScript to contentScript, an Observable need to send multiple times, after subscribe.

I need something like

    const ovserver = {
      next: (value) => sender.send(subscriptionId, { type: ResponseType.Next, value }),
      error: (error: Error) => sender.send(subscriptionId, { type: ResponseType.Error, error: Errio.stringify(error) }),
      complete: () => sender.send(subscriptionId, { type: ResponseType.Complete }),
    };

To achieve something like https://github.com/frankwallis/electron-ipc-proxy

@fregante
Copy link

fregante commented Nov 8, 2021

@linonetwo That's not how sendResponse works.

If you call sendReponse multiple times, all calls past the first one will just be ignored. There's no such sender.send function.

sendResponse is defined by its name, it sends a response, not a generic message.

@joshsmith
Copy link

As seen in mozilla/webextension-polyfill#386 (comment) sendResponse should now be supported by this package.

See also updated MDN documentation: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/runtime/onMessage

@lhk
Copy link

lhk commented Jan 6, 2023

@Lusito I think the post from joshsmith is important, wouldn't it make sense to update the specification?

@Lusito Lusito closed this as completed in b803c68 Jan 8, 2023
@Lusito
Copy link
Owner

Lusito commented Jan 8, 2023

Will be fixed in the next version.

For the record: I don't think it's a good idea, because people easily misunderstand this and think they can call it multiple times.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

7 participants