Skip to content

Use browser extension Port objects instead of hacky comlink string channel

Warren Gifford requested to merge comlink-no-string-channel into master

Created by: felixfbecker

Comlink relies on creating MessagePorts for each proxied object and transferring those with postMessage(). But browser extension Port objects don't support transferring MessagePorts (or anything really).

Because of this, we used a hacky adapter that channeled every MessagePort through a single Port using string IDs (and serializing everything to a string, although that's not required).

This string message adapter was a hacky addition to comlink v3 that "should have never made it to master" according to its creator, and is missing in v4. There is an experimental one in v4, but it is broken, and actually broke our browser extension (I thought I had tested this, but I must have made a mistake, maybe I had a prod browser extension running? 🤷‍♂️ Really wish we had tests for this in CI).

Because I couldn't get it fixed, I instead rethought our approach and worked on a solution that doesn't use a string wrapper. While Ports don't support transferables, it is possible to open multiple connections. How it works with this PR:

  • We capture all incoming Port connections and map them by their name.
  • Whenever we see a MessagePort in a message to be sent, we open a Port to the other side with a UUID name.
  • We include in the message sent over the Port the UUID and path at which the MessagePort appeared.
  • When we receive a message with UUID port references we take (or wait for) a connection with the UUID.
  • We hook that Port up to a MessagePort, and splice that MessagePort back into the message at the path it appeared.

@lguychard I hope I am not forgetting a reason why we didn't do this in the first place (besides saving effort) 😅

Also would like to source the swarm intelligence to figure out where we need to add ensure resources are cleaned up and not leaked. Although I'm relatively sure we were leaking before - we're only using the new v4 [releaseProxy]() as of this PR.

Tested:

  • Chrome extension
  • Firefox extension
  • Web app
  • Native integration: Bitbucket server

Merge request reports