Page MenuHomePhabricator

[web-db] implement basic app-worker connection types

Authored by kamil on Tue, Mar 7, 9:00 AM.
Referenced Files
Unknown Object (File)
Tue, Mar 21, 11:49 PM
Unknown Object (File)
Mon, Mar 20, 9:10 PM
Unknown Object (File)
Fri, Mar 17, 1:42 PM
Unknown Object (File)
Wed, Mar 8, 4:25 PM
Unknown Object (File)
Wed, Mar 8, 4:24 PM
Unknown Object (File)
Wed, Mar 8, 11:54 AM



Adding basic types and concept of sharing information between worker and app. Current type (PING/PONG) will be used probably only for development, will add more concrete ones in next diffs.

Depends on D6990

Test Plan

Run this code:

const webDbWorker = new SharedWorker('/worker/webDatabase');
webDbWorker.port.onmessage = function (e) {
  console.log('message from worker: ',;
const message: WorkerRequestMessage = {
  type: workerRequestMessageTypes.PING,
  text: 'hi from app',

and check results in browser, desktop and shared worker consoles.

Check error flow.

Diff Detail

rCOMM Comm
Lint Not Applicable
Tests Not Applicable

Event Timeline

kamil held this revision as a draft.
40 ↗(On Diff #23509)

That's a problematic line but unfortunately is typed in flow as mixed and there isn't a better way to cast this to our exact type.

We can not override MessageEvent because even if we override all types there is a place where there will be a need to cast via any (there always will be a difference between what we have and flow's SharedWorker definition).

I think this is the safest place because we will receive data from the other side of the pipe here

kamil published this revision for review.Tue, Mar 7, 10:06 AM
40 ↗(On Diff #23509)

This is fine, and I think you're doing it well

40 ↗(On Diff #23509)

Thanks for confirmation

tomek added inline comments.
24 ↗(On Diff #23509)

At this point it looks like we can make this mandatory, but maybe that will change in the future

46–48 ↗(On Diff #23509)

I think it might be a good idea to include testing this error scenario in the test plan

This revision is now accepted and ready to land.Wed, Mar 15, 3:40 AM
24 ↗(On Diff #23509)

Yes, it will, there will be messages (like INIT) for which resolving/rejecting promises will be the only information. We could avoid sending additional response data between app<->worker with success status when promise could do the same with fewer data transmitted.

I know it's not logically consistent for this diff but the following diff in the stack (D6993) shows why this should not be mandatory.

46–48 ↗(On Diff #23509)

Oops, I test that but didn't include it in the test plan, fixing