Fix message ports not being closed when proxy is relased

[achim-k]

Changelog

Fix message ports not being closed when proxy is relased

Docs

None

Description

Fixes a regression introduced #3 that caused the

requestResponseMessage(ep, pendingListeners || new Map(), {
    type: MessageType.RELEASE,
  })

promise to never settle which in turn prevented the endpoint from being closed.

[defunctzombie]

Is the issue that clearing the listeners here is premature because the releaseEndpoint call needs to access the listeners to resolve? If that's the case it looks like releaseEndpoint itself returns a promise so you could do a .finally(() => pendingListeners.clear()) here instead of passing it to releaseEndpoint?

Or is it important to pass pendingListeners so it can be passed to requestResponseMessage ?

[achim-k]

Or is it important to pass pendingListeners so it can be passed to requestResponseMessage ?

Yes, that's the important bit. If pendingListeners are not passed, the resolver function is not found and the requestResponseMessage promise will never settle.

[defunctzombie]

So why do we allow pendingListeners to be undefined? In what situation is that ok?

[achim-k]

This was necessary when releasing the endpoint when the proxy is garbage collected. However, thinking about this more, we should also have access to the pendingListeners map as it is tightly coupled to the endpoint now.

[achim-k]

I have added a commit that changes the endpoint and pendingListeners to be passed around in a single object (EndpointWithPendingListeners). There is probably a better name for that type/object...

[defunctzombie]

Not sure why it is ok for pendingListeners to be false if we need it to invoke releaseEndpoint correctly - some comments on expected behaviors and implementation details would go a long way towards making this easier to maintain

[defunctzombie]

Suggested change

	const newCount = (proxyCounter.get(epWithPendingListeners) || 0) + 1;
	const newCount = (proxyCounter.get(epWithPendingListeners) ?? 0) + 1;

[achim-k]

I'm leaving this as is as the logical OR operator works with much older browser versions. The nullish coalescing operator was introduced much later.

[defunctzombie]

fwiw - that is handled during the transpilation step of ts -> js. When you specify a specific ES version as the target it will produce the code for that ES version so you can write your ts using modern styles.