Skip to content

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.

3 participants