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

Abstract chrome.runtime.onMessage #449

Open
mkobayashime opened this issue Nov 4, 2022 · 3 comments
Open

Abstract chrome.runtime.onMessage #449

mkobayashime opened this issue Nov 4, 2022 · 3 comments

Comments

@mkobayashime
Copy link
Member

Wrap chrome.runtime.onMessage and provide reasonable types.

@yudukikun5120
Copy link
Collaborator

@mkobayashime Could I hear the motivation for wrapping chrome.runtime.onMessage? This wrapping seems to be similar to chrome.storage and I thought this is because of the restrictions of the first parameter (e.g. channel name) (, and this is a common problem for developers who struggle with IPC).

@mkobayashime
Copy link
Member Author

@yudukikun5120
We generally should wrap this kind of external interfaces so that we can deal with any changes/updates to the interface with minimal changes to the codebase (hopefully only on the wrapper layer). Abstraction like this in chrome.storage will help us in migration to MV3 actually.
Additionally, chrome.runtime.onMessage recieves all messages emitted from anywhere, one intended to download report template file and another one intended to open options page. The onMessage listener you added in #416 actually duplicates with existing one (here) listening for clicks of the link for the options page, so we have to pass props like action: 'openOptionsPage' | 'downloadReportTemplate' and detect the intention of the message in onMessage. Using chrome.runtime APIs directly makes it harder to avoid this kind of mistakes.

@yudukikun5120
Copy link
Collaborator

@mkobayashime I'm glad to know the intention of the wrapping. I understand it in the context of the namespace of the onMessage and will pass it through the wrapper on the later coding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants