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

Use addEventListener instead of onmessage in WebWorkerPostMessageStream #83

Merged
merged 3 commits into from
Mar 31, 2023

Conversation

Mrtenz
Copy link
Member

@Mrtenz Mrtenz commented Mar 31, 2023

Description

This pull request updates the WebWorkerPostMessageStream constructor to use the addEventListener method instead
of onmessage. The use of addEventListener allows for proper handling of multiple event listeners and making the
event handling process more robust. It also ensures that the code is compatible with LavaMoat.

The WebWorkerPostMessageStream is used in our project for communication between web workers in our application. The
stream constructor uses postMessage and onmessage to emit and receive messages, respectively. The onmessage
method is used to handle incoming messages from multiple listeners. However, this method doesn't provide easy
management of event listeners. In this pull request, we change the constructor code to use addEventListener which
is more versatile and makes the code more maintainable.

This change will not have a significant impact on the performance of the application, but it makes the code more
readable and safer to maintain.

Changes

  1. Update WebWorkerPostMessageStream constructor to use addEventListener instead of onmessage.

…am constructor

The use of addEventListener allows proper handling of multiple event listeners. onmesssage also does not work with LavaMoat.
@Mrtenz Mrtenz changed the title Use addEventListener instead of onmessage in WebWorkerPostMessageStrea Use addEventListener instead of onmessage in WebWorkerPostMessageStream Mar 31, 2023
The test was trying to use globalThis.self variable, but it is not always defined in the test
environment. To fix the issue, the variable can be created as a mock object before
each test, and restored to the original value after it finishes.
Copy link
Member

@rekmarks rekmarks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Mrtenz Mrtenz merged commit f3bcf14 into main Mar 31, 2023
@Mrtenz Mrtenz deleted the mrtenz/fix-webworker-stream branch March 31, 2023 17:32
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

Successfully merging this pull request may close these issues.

2 participants