Skip to content
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

Add support for SharedWorker #458

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

852Kerfunkle
Copy link
Contributor

@852Kerfunkle 852Kerfunkle commented Feb 2, 2023

The master side of things should be pretty much done - I hope.

Not too happy about the use of in as a would-be type guard, but what can you do.... Feedback welcome.

Closes #401.

@852Kerfunkle
Copy link
Contributor Author

852Kerfunkle commented Feb 3, 2023

Have another commit coming up. It almost works now.

I can't seem to get the shared worker to run at all in the puppeteer tests but it maybe kinda does in the webpack tests.

The only thing missing now is the SharedWorker AbstractedWorkerAPI and handling expose. It'll go roughly like this: If it's a SharedWorker, only register connect and re-run most of expose for every client (master) connecting. Or something close to it.

@852Kerfunkle
Copy link
Contributor Author

It does prove somewhat difficult due to all the outdated dependencies.

@andywer What's the status of threads.js and related dependencies? It feels a little discontinued. :)

needed to add a context to AbstractedWorkerAPI
@852Kerfunkle
Copy link
Contributor Author

This should be the bulk of it.

Needed to add a context to AbstractedWorkerAPI, so onconnect can register multiple handlers for multiple masters. Maybe there's a better way to do it.

Feedback welcome.

It might already work, but it's difficult to test, since I can't get SharedWorker to work at all in the puppeteer tests (it just does nothing).

@852Kerfunkle
Copy link
Contributor Author

852Kerfunkle commented Feb 3, 2023

Another issue is that it seems to be difficult (if not impossible) to detect if a client closed the port to the SharedWorker.

Maybe I'm missing something.

I suppose the master could send a disconnect message before closing the port, then the SharedWorker can remove listeners and close the port to the disconnecting master.

@852Kerfunkle
Copy link
Contributor Author

Well, whatever I try, I can't seem to get a SharedWorker to run any code. So I don't think I can finish this.
If anyone knows what's going on here, please let me know.

To anyone who may be looking into this in the future: it's pretty much done,iIt might even already work (if SharedWorkers work for you at all), maybe needs to handle master disconnects.

@852Kerfunkle
Copy link
Contributor Author

Alright, figured out why shared workers won't work:

Browsers do not like SharedWorker to be subclassed.

Not sure how to proceed from here.

@852Kerfunkle
Copy link
Contributor Author

852Kerfunkle commented Feb 6, 2023

To add to the annoyance, at least chrome does not allow SharedWorkers to have nested Workers.

Which kinda was what I needed from this.

https://bugs.chromium.org/p/chromium/issues/detail?id=31666

@852Kerfunkle
Copy link
Contributor Author

Even not extending and exposing the SharedWorker implementation like it's done for Worker and BlobWorker, I can't get it to work with threads.js.

I'm out of ideas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Shared Web Workers
1 participant