Skip to content

Commit

Permalink
Use addEventListener instead of onmessage in WebWorkerPostMessage…
Browse files Browse the repository at this point in the history
…Stream (#83)

* Use addEventListener instead of onmessage in WebWorkerPostMessageStream constructor

The use of addEventListener allows proper handling of multiple event listeners. onmesssage also does not work with LavaMoat.

* Remove test for throwing error when not run in a WebWorker

This test is now unnecessary.

* Refactor WebWorkerPostMessageStream test using mocked self

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.
  • Loading branch information
Mrtenz authored Mar 31, 2023
1 parent d7558fa commit f3bcf14
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 13 deletions.
24 changes: 15 additions & 9 deletions src/WebWorker/WebWorker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,26 +84,26 @@ describe('WebWorker Streams', () => {
);
});

it('throws if not run in a WebWorker (self not an instance of WorkerGlobalScope)', () => {
(globalThis as any).self = originalSelf;
expect(() => new WebWorkerPostMessageStream()).toThrow(
'WorkerGlobalScope not found. This class should only be instantiated in a WebWorker.',
);
});

it('can be destroyed', () => {
(globalThis as any).self = originalSelf;
const stream = new WebWorkerPostMessageStream();
expect(stream.destroy()).toBeUndefined();
});

it('forwards valid messages', () => {
(globalThis as any).self = originalSelf;
const addEventListenerSpy = jest.spyOn(globalThis, 'addEventListener');
const stream = new WebWorkerPostMessageStream();

const onDataSpy = jest
.spyOn(stream, '_onData' as any)
.mockImplementation();

expect(addEventListenerSpy).toHaveBeenCalledTimes(1);
const listener = addEventListenerSpy.mock.calls[0][1] as EventListener;

// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
self.onmessage!(
listener(
new MessageEvent('foo', {
data: { data: 'bar', target: DEDICATED_WORKER_NAME },
}),
Expand All @@ -114,11 +114,17 @@ describe('WebWorker Streams', () => {
});

it('ignores invalid messages', () => {
(globalThis as any).self = originalSelf;
const addEventListenerSpy = jest.spyOn(globalThis, 'addEventListener');
const stream = new WebWorkerPostMessageStream();

const onDataSpy = jest
.spyOn(stream, '_onData' as any)
.mockImplementation();

expect(addEventListenerSpy).toHaveBeenCalledTimes(2);
const listener = addEventListenerSpy.mock.calls[0][1] as EventListener;

(
[
{ data: 'bar' },
Expand All @@ -127,7 +133,7 @@ describe('WebWorker Streams', () => {
] as const
).forEach((invalidMessage) => {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
self.onmessage!(new MessageEvent<unknown>('foo', invalidMessage));
listener(new MessageEvent<unknown>('foo', invalidMessage));

expect(onDataSpy).not.toHaveBeenCalled();
});
Expand Down
6 changes: 2 additions & 4 deletions src/WebWorker/WebWorkerPostMessageStream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ export class WebWorkerPostMessageStream extends BasePostMessageStream {
if (
typeof self === 'undefined' ||
// @ts-expect-error: No types for WorkerGlobalScope
typeof WorkerGlobalScope === 'undefined' ||
// @ts-expect-error: No types for WorkerGlobalScope
!(self instanceof WorkerGlobalScope)
typeof WorkerGlobalScope === 'undefined'
) {
throw new Error(
'WorkerGlobalScope not found. This class should only be instantiated in a WebWorker.',
Expand All @@ -34,7 +32,7 @@ export class WebWorkerPostMessageStream extends BasePostMessageStream {
super();

this._name = DEDICATED_WORKER_NAME;
self.onmessage = this._onMessage.bind(this) as any;
self.addEventListener('message', this._onMessage.bind(this) as any);

this._handshake();
}
Expand Down

0 comments on commit f3bcf14

Please sign in to comment.