-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
polyfill Worker
in chrome runtime
#190
Conversation
managed to discover some lies within the chrome runtime api about the offscreen lifecycle, so i have to work around that, but this is nearly ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
presently a draft, uploaded to show wip
"background": { | ||
"service_worker": "service-worker.js" | ||
"service_worker": "service-worker.js", | ||
"type": "module" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change is necessary due to some webpack reconfiguration.
apps/extension/src/service-worker.ts
Outdated
import { OffscreenWorker } from '@repo/chrome-offscreen-worker/worker'; | ||
|
||
globalThis.Worker = OffscreenWorker; | ||
OffscreenWorker.configure( | ||
chrome.runtime.getURL( | ||
'offscreen.html', | ||
) as `chrome-extension://${typeof chrome.runtime.id}/${'offscreen.html'}`, | ||
); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the offscreen worker polyfill is applied to the global here. any packages bundled into this entry calling new Worker
will actually be calling new OffscreenWorker
@@ -31,6 +31,7 @@ | |||
"@bufbuild/protobuf": "^1.10.0", | |||
"@connectrpc/connect": "^1.4.0", | |||
"@connectrpc/connect-web": "^1.4.0", | |||
"@tsconfig/strictest": "^2.0.5", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import { OffscreenWorkerPort } from './messages/worker-event.js'; | ||
import { OffscreenRootPort, WorkerConstructorParamsPrimitive } from './messages/root-control.js'; | ||
|
||
const attempts: string[] = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
todo: remove debug
const attempts: string[] = []; |
export type WorkerConstructorParamsPrimitive = | ||
Required<ConstructorParameters<typeof Worker>> extends [infer S, infer O] | ||
? [S extends string ? S : never, { [k in keyof O]: O[k] & string }] | ||
: never; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mostly this just excludes URL
from the worker path url parameter, but it also types the WorkerOptions
structure as primitives to indicate jsonifiability
this.internalTarget.addEventListener('error', this.parentListener); | ||
this.internalTarget.addEventListener('message', this.parentListener); | ||
this.internalTarget.addEventListener('messageerror', this.parentListener); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
each item of WorkerEventMap
is listened
const data: unknown = args[0]; | ||
const { transfer } = Array.isArray(args[1]) ? { transfer: args[1] } : { ...args[1] }; | ||
|
||
this.externalTarget.postMessage(data, { transfer }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
transferrable objects are accepted, but are likely to not serialize properly in parentListener
this doesn't matter for prax's use, where only json is sent.
this is pretty close, but i had other tasks today and now i'm out of time. tl;dr: a transaction build can take place successfully, but it seems like sessions live a bit too long. workers should be redesigned to terminate themselves when possible. be aware that when the this does not yet support use from multiple scripts - different global contexts will track offscreen use independently, and kill the shared document early. it might be good to check if terminating the offscreen document actually kills the web workers, or if it's cleaned up too fast to issue termination commands. this tool when complete would be incredibly useful to extension developers in general, so i'm likely to publish this polyfill independently to NPM. |
|
advocating to close the intermediate polyfill step in favor of proceeding directly with the migration. |
references penumbra-zone/web#1795
this PR creates a package implementing a polyfill of the DOM
Worker
interface for chrome extension workers, and applies that polyfill to consumed packages from penumbra-zone.pending penumbra-zone/web#1769