Page MenuHomePhabricator
Differential D6991 Diff 23509 web/database/worker/web-db-worker.js

Changeset View

Standalone View

View Options

web/database/worker/web-db-worker.js

// @flow // @flow
import localforage from 'localforage'; import localforage from 'localforage';
import { type SharedWorkerMessageEvent } from '../../types/worker-types.js'; import {
type SharedWorkerMessageEvent,
type WorkerRequestMessage,
type WorkerResponseMessage,
workerRequestMessageTypes,
workerResponseMessageTypes,
} from '../../types/worker-types.js';
const localforageConfig: PartialConfig = { const localforageConfig: PartialConfig = {
driver: localforage.INDEXEDDB, driver: localforage.INDEXEDDB,
name: 'comm', name: 'comm',
storeName: 'commStorage', storeName: 'commStorage',
description: 'Comm encrypted database storage', description: 'Comm encrypted database storage',
version: '1.0', version: '1.0',
}; };
localforage.config(localforageConfig); localforage.config(localforageConfig);
function processAppRequest(
message: WorkerRequestMessage,
): ?WorkerResponseMessage {
tomekUnsubmitted
Not Done
Inline Actions
message: WorkerRequestMessage,
- ): ?WorkerResponseMessage {
+ ): WorkerResponseMessage {
if (message.type === workerRequestMessageTypes.PING) {

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

tomek: At this point it looks like we can make this mandatory, but maybe that will change in the future
kamilAuthorUnsubmitted
Done
Inline Actions

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.

kamil: Yes, it will, there will be messages (like `INIT`) for which resolving/rejecting promises will…
if (message.type === workerRequestMessageTypes.PING) {
return {
type: workerResponseMessageTypes.PONG,
text: 'PONG',
};
}
throw new Error('Request type not supported');
}
function connectHandler(event: SharedWorkerMessageEvent) { function connectHandler(event: SharedWorkerMessageEvent) {
const port: MessagePort = event.ports[0]; const port: MessagePort = event.ports[0];
console.log('Web database worker alive!'); console.log('Web database worker alive!');
port.onmessage = async function (messageEvent: MessageEvent) { port.onmessage = async function (messageEvent: MessageEvent) {
console.log('message from main thread: ', messageEvent.data); const message: WorkerRequestMessage = (messageEvent.data: any);
kamilAuthorUnsubmitted
Done
Inline Actions

That's a problematic line but unfortunately messageEvent.data 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: That's a problematic line but unfortunately `messageEvent.data` is typed in flow as `mixed` and…
ashoatUnsubmitted
Not Done
Inline Actions

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

ashoat: This is fine, and I think you're doing it well
kamilAuthorUnsubmitted
Done
Inline Actions

Thanks for confirmation

kamil: Thanks for confirmation
port.postMessage({ message: 'response from worker' }); try {
const result = processAppRequest(message);
port.postMessage(result);
} catch (e) {
port.postMessage({
error: e,
});
tomekUnsubmitted
Not Done
Inline Actions

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

tomek: I think it might be a good idea to include testing this error scenario in the test plan
kamilAuthorUnsubmitted
Done
Inline Actions

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

kamil: Oops, I test that but didn't include it in the test plan, fixing
}
}; };
} }
self.addEventListener('connect', connectHandler); self.addEventListener('connect', connectHandler);