Thanks to the efforts from @dryajov and @tswindell we are close to having full support for running js-ipfs in webworkers. This issue tracks the last outstanding issues for this.
Aegir
- [x] Merge webworkers support into aegir
- [x] Release aegir with webworker test support
PRs
- [x] https://github.com/libp2p/js-libp2p-crypto/pull/61
- [x] https://github.com/multiformats/js-multihashing-async/pull/11 and https://github.com/multiformats/js-multihashing-async/pull/12
Others
- [x] Enable webworker tests for all js-ipfs* repos
- [x] Enable webworker tests for all js-libp2p* repos
- [x] Enable webworker tests for all js-multi* repos
dignifiedquire
All 42 comments
@dignifiedquire @diasdavid I can take this on:
Enable webworker tests for all js-ipfs* repos
Enable webworker tests for all js-libp2p* repos
Enable webworker tests for all js-multi* repos
This is required to move forward with #127
dryajov
on 27 Jan 2017
This is awesome! Thank you @dryajov!
Can we then make a lot of noise sure everyones voice is represented on the request to have WebRTC in WebWorkers?
- https://lists.w3.org/Archives/Public/public-webrtc/2015Oct/0000.html
- https://github.com/w3c/webrtc-pc/issues/230
- https://bugzilla.mozilla.org/show_bug.cgi?id=922363
- https://bugs.chromium.org/p/chromium/issues/detail?id=302019
- https://groups.google.com/forum/#!topic/discuss-webrtc/ulHSVgHnnrk
- https://github.com/w3c/webrtc-pc/pull/317
daviddias
on 29 Jan 2017
Wanted to check in with where things are with this endeavor.
So far, I've managed to switch the use of window to self, and this seem to fix 90% of the issues. I've got most of it committed in my forks on github, if anyone is interested to check it out, please do. I'm not entirely sure about using self being the right way of doing it, and I'm not as familiar with webpack to know if there is a better approach (i'd rather a bundler taking care of context/env), but so far this has worked. @diasdavid @dignifiedquire @kumavis If you guys know of a better way please let me know; otherwise, I think we have a best practice suggestion to make to the community at large - ditch window in favor of self. 👅
So far, the biggest issue I ran into is with brorand, which is not assuming the webworker env correctly. I've got a PR coming that addresses it, and that fixes most of the issues with multihashes. But I'm not sure when thats going to be merged in, I might need help creating noise around it.
I will be creating PRs for each project shortly (today/tomorrow), so that everypbody can keep an eye on the progress and contribute where they see fit.
The rest of the issues are less trivial, and I will be going over them one by one and fixing as I go.
I'll keep everyone posted on progress.
Cheers!
dryajov
on 1 Feb 2017
PR to address brorand - https://github.com/indutny/brorand/pull/7
dryajov
on 2 Feb 2017
i bet you could polyfill it
if (typeof window === 'undefined') self.window = self
kumavis
on 2 Feb 2017
wouldn't that mess things up a bit inside a WW for other packages as well? As in incorrectly assuming that its running in a dom enabled env? Otherwise, thats a better approach.
dryajov
on 2 Feb 2017
yeah thats a good point, its a bit hacky so beware
kumavis
on 2 Feb 2017
The linked PRs should address the full webworker support, with this changes I'm able to run js-ipfs tests in webworkers.
dryajov
on 8 Feb 2017
Looks like brorand shipped yesterday with the fixes :)
dignifiedquire
on 8 Feb 2017
woooot! So now, we just need to enable webworker tests in all the repos? Let's make all of this on time for #756 \o/
daviddias
on 8 Feb 2017
@diasdavid all you have to do for that is upgrade to aegir@10 which runs them by default
dignifiedquire
on 8 Feb 2017
daviddias
on 8 Feb 2017
@diasdavid for projects that we want to skip webworker tests you can pass the --dom parameter to aegir.
dryajov
on 8 Feb 2017
@diasdavid I would also merge and release this PRs before bumping aegir on the rest of the projects, because they would fail without those changes, I've seen multihashing-async being the culprit in most cases...
dryajov
on 8 Feb 2017
why --dom? That doesn't seem like to be something obvious for what it does
daviddias
on 8 Feb 2017
there are two flags now, --webworker which will run webworker only tests and --dom which will run tests in a dom enabled env™ only, I've no attachment to that flag whatsoever and open to hearing suggestions - naming is hard. 😛
dryajov
on 8 Feb 2017
indeed :) let's do --webworker=only and --webworker=false for only webworker and except webworker respectively, being --webworker=true the default
daviddias
on 8 Feb 2017
that seems reasonable :)
dryajov
on 8 Feb 2017
daviddias
on 9 Feb 2017
daviddias
on 10 Feb 2017
@dryajov what is the state of this? Would love to have it complete.
daviddias
on 8 Mar 2017
I believe the only outstanding issue is this PR https://github.com/dignifiedquire/aegir/pull/99, but support of ww itself is ready.
dryajov
on 8 Mar 2017
I'm intrigued, I still see this today:
13 03 2017 15:38:40.086:INFO [Chrome 56.0.2924 (Mac OS X 10.12.3)]: Connected on socket f6zxk4cTtR4TZ6sgAAAB with id 86174125
Chrome 56.0.2924 (Mac OS X 10.12.3) ERROR
[karma-mocha-webworker] RPCRequestError<Error>: Uncaught ReferenceError: window is not defined at RPCRequestError<Error>: Uncaught ReferenceError: window is not defined
at node_modules/karma-mocha-webworker/karma-mocha-webworker-client/adapter.js:4234:57
Caused by Remote Error: Uncaught ReferenceError: window is not defined
at Object.importScripts (node_modules/karma-mocha-webworker/karma-mocha-webworker-client/worker.js:109:47)
at node_modules/karma-mocha-webworker/karma-mocha-webworker-client/worker.js:3400:35
And yet, after https://github.com/dignifiedquire/aegir/pull/99#discussion_r104973985, tests still don't fail after the webworker tests fail.
daviddias
on 13 Mar 2017
@diasdavid what project is that? This might be a project that needs WW disabled. As for them not failing, are you on the latest Aegir with those fixes applied?
dryajov
on 13 Mar 2017
This is js-ipfs itself
daviddias
on 13 Mar 2017
Ah, thats weird. I'll take a look at it, wonder if any changes have been introduced which could bring window in again? In any case will take a look ASAP.
dryajov
on 13 Mar 2017
@dryajov still seeing this one, any developments?
daviddias
on 16 Mar 2017
Since @dignifiedquire asked me to update aegir in all the repos (thanks dig!), I'm going to list out here which repos still don't have webworker support.
I've checked so far:
- [x] multiformats
- [x] needs to move the
eslint-disable-linebydirty-chai
- [x] needs to move the
- [x] ipld
- [x] needs to move the
eslint-disable-linebydirty-chai
- [x] needs to move the
- [ ] libp2p (missing: below)
- https://github.com/libp2p/js-peer-book
- https://github.com/libp2p/js-libp2p-tcp
- https://github.com/libp2p/js-libp2p-websockets
- https://github.com/libp2p/js-libp2p-webrtc-star
- https://github.com/libp2p/js-libp2p-crypto-secp256k1
- https://github.com/libp2p/js-libp2p-crypto
- https://github.com/libp2p/js-libp2p-ping
- https://github.com/libp2p/js-libp2p-multiplex
- https://github.com/libp2p/js-libp2p-spdy
- https://github.com/libp2p/js-libp2p-identify
- https://github.com/libp2p/js-libp2p-swarm
- https://github.com/libp2p/js-libp2p-secio
- https://github.com/libp2p/js-libp2p
- [ ] ipfs (missing: all, except js-ipfs)
Need webworker support
- [ ] js-multihashing https://github.com/multiformats/js-multihashing/pull/19
- [ ] js-ipfs https://github.com/ipfs/js-ipfs
- [x] js-ipfs-block-service https://github.com/ipfs/js-ipfs-block-service/pull/58/commits/a54e113495fc334f840394d3f8602ef731da573d
- [ ] js-ipfs-bitswap https://github.com/ipfs/js-ipfs-bitswap/issues/120
daviddias
on 16 Mar 2017
@diasdavid, we also need this https://github.com/dignifiedquire/aegir/issues/107 in aegir. For now, we can just disable window as a global without any specific message, if you have a chance to update aegirs eslint with the rule, it might be a good time.
dryajov
on 16 Mar 2017
@diasdavid webrtcsupport lib is the one causing issues, it is the last lib we need fixed before full WW support is finally ready. Here is a PR addressing it https://github.com/HenrikJoreteg/webrtcsupport/pull/30.
dryajov
on 17 Mar 2017
@dryajov: i'd advise against using the webrtcsupport package. Its legacy and contains 90% stuff that you don't need anymore in 2017.
@diasdavid i would strongly advise against "making noise", in particular making the kind of noise that has happened on the worker topic in the past.. Trying to get someone else to do work that benefits you like this typically doesn't work out. If you want to be constructive you work together with the people who are able to do that work or do the work (including specifying how things should work) yourself.
fippo
on 18 Mar 2017
@fippo thanks for pointing this out. Indeed all we use it for is detection of the presence of webrtc support in the environment, perhaps don't need that in the browser anymore, but what about nodejs?
dryajov
on 18 Mar 2017
@fippo I clearly didn't word that in the best way possible. Entirely agree with you, it is orders of magnitude more valuable to provide constructive help and willingness to be part of the process than a simple 'I want this too'. Currently, I'm underwater and unavailable to provide significant help to that development, however, I do hope in the best way possible, that by bringing other people that could also benefit from it, we can signal that there is value on making it happen.
re: webrtcsupport - What is your favourite nowadays?
daviddias
on 20 Mar 2017
@diasdavid at least you didn't tweet about it like certain other people. If you just want to check for support RTCPeerConnection && ('createDataChannel' in RTCPeerConnection.prototype) should be sufficient.
fippo
on 20 Mar 2017
@diasdavid I'll do what @fippo is suggesting.
dryajov
on 20 Mar 2017
@dignifiedquire found this recently https://github.com/Joris-van-der-Wel/karma-mocha-webworker
may be of interest
kumavis
on 4 Apr 2017
@kumavis thats what we ended up using.
To get the last mile of this, we need a pretty small change in how we detect WebRTC. Looking up that issue...
ah... its right in this thread:
https://github.com/ipfs/js-ipfs/issues/725#issuecomment-287792875
dryajov
on 4 Apr 2017
As an outside observer, I have to ask: what does all this mean? Are you actually able to use WebRTC APIs in worker code as has been proposed as standard here? Or is this issue just to test that the code should theoretically work the same from a worker context, to be ready when worker WebRTC is standardized?
IamCarbonMan
on 8 May 2017
Good question, glad you ask - this effort is to get IPFS to run in Web and Service workers. The specific outstanding issue right now is related to WebRTC detection functionality that breaks inside Workers because of explicit usage of window. WebRTC is tangential to this effort, since its not supported in web/service workers at all at this point.
dryajov
on 8 May 2017
@IamCarbonMan you are not able to, but libp2p dark magic uses whatever the runtime has to offer it, it won't use WebRTC if there is no support but the issue is that we are checking the support by checking the window object which is also not defined.
daviddias
on 16 May 2017
WebWorker tests are now passing! :D woot!
daviddias
on 19 May 2017
fully supported in v0.24.0
daviddias
on 29 May 2017
Related issues
fazo96
·
3Comments
Ntmf
·
4Comments
OliverUv
·
3Comments
daviddias
·
3Comments
mnts
·
3Comments
Most helpful comment
fully supported in v0.24.0