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

check for message-port-like methods should reach lower common denominator #16

Open
achou11 opened this issue Sep 11, 2023 · 1 comment

Comments

@achou11
Copy link
Member

achou11 commented Sep 11, 2023

Looking at this check:

typeof value.on === 'function' &&
typeof value.off === 'function' &&
typeof value.postMessage === 'function'

I think this should be doing addEventListener and removeEventListener to align with the EventTarget interface, which MessagePort should extend. I think on and off are more strictly related to Node's implementation and are usually just aliases of add/remove event listener.

Example of additional work needed when integrating rpc-reflector is somewhat describe here, where the channel in question didn't have on and off methods and so a custom class had to be created to emulate the checked interface (digidem/comapeo-mobile#8):

separate implementations of MessagePortLike. this is because the nodejs-mobile channel extends different behaving classes based on the environment (Node's EventEmitter in the nodejs-mobile process but React Native's EventEmitter in the React Native runtime). Due to the different APIs and behaviors between the two, creating a shared implementation is not trivial and more overhead than necessary.

@achou11
Copy link
Member Author

achou11 commented Sep 11, 2023

turns out node's event emitter doesn't implement addEventListener and removeEventListener 🤦‍♂️ Still, maybe at least check for addListener and removeListener

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

No branches or pull requests

1 participant