-
Notifications
You must be signed in to change notification settings - Fork 24
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
Disable shared worker, use a single IPFS instance per app #404
Conversation
I'm converting all the old browser tests to node tests, because it loads a new js-ipfs node for all these tests now, essentially flooding the production ipfs node with new connections (connects to all peers for each test). |
Testing this out with the React TodoMVC app, and I'm seeing some bundler issues. Create React App (
|
Testing the latest changes after bundling Create React App (
|
I think that's a separate issue no? I've seen that in current production apps too.
Not good 😞 Is this with the same Vite config as in oddsdk/odd-bundler-test#2 with |
Ok, since we still have so many bundler issues, I added the CDN option as well and made that the default. To switch to the bundled ipfs-core, you can do: wn.ipfs.pkgFromBundle()
.then(wn.ipfs.nodeWithPkg)
.then(wn.ipfs.set) Let me know if you want better names for that. In addition to that I've added an ES6 build next to the UMD build. The ES6 build is code splitted, whereas the UMD build is not. Meaning the bundled ipfs-core is a separate file with the ES6 build, so that won't be loaded when you use the CDN. I've also done some testing with the various bundlers from https://github.com/fission-codes/webnative-bundler-test to see what the application code would look like. Vite:
Esbuild:
Webpack:
|
CI tests keep failing because of timeouts, did I mess something up or was this happening before? |
Ah yeah, was just mentioning that for context, but nothing we need to fix here.
Adding the
This sequence of events happens with both the Vite dev server and build. Will try things out with the other bundlers next. |
Alright, more test results with the latest pulling in That leaves Webpack 5. The dev server behaves similar to Vite, but slightly different. On initializes, things work great for a while but they slow down and get inconsistent after a little while. Refreshing the page in this state, webnative does initialize, but the filesystem is paralyzed and cannot do anything. By contrast, the Webpack 5 build is solid. Works great at initialization, keeps up after using it for a few minutes, and works fine after refreshing the page. |
53651a2
to
4f414bd
Compare
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.
Looks good and working great!
As a summary, I've tested this in the React TodoMVC codebase with Create React App, Vite, and Webpack 5. In all cases, these changes work for both dev server and builds.
I've also tested in the WIP Webnative App Template which handles it own auth without a trip to the lobby. Both the dev server and the build work great there as well!
Excellent work on this! Exciting to have per-app IPFS nodes 🎉
splitting: true, | ||
minify: true, | ||
sourcemap: true, | ||
platform: "browser", |
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.
will / should this work in a web worker?
i tried
const webnativeCDNURL = 'https://unpkg.com/webnative@0.34.0-alpha1//dist/index.umd.min.js'
const importRes = self.importScripts(webnativeCDNURL)
but i get an ReferenceError: document is not defined
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.
Ah good point, we totally forgot about that. Will fix 👍 Thanks!
@gotjoshua It's indeed harmless, just IPFS trying to connect to peers. We aren't sure where that's coming from exactly though, we're trying to disable all these random peer connections. |
Will this strategy block the UI thread while initialize is running or the fs is loading? i wonder if adding a blobURL to an iframe may be an alternative |
@gotjoshua I'm not entirely sure tbh, it doesn't feel like it's blocking it when testing it. We previously used a shared worker for IPFS to optimise this as much as possible out of the box. But, we had a bunch of issues with passing data across All that said, you can pass the |
exactly what i'm doing. Thanks! i still feel some ui lag sometimes, which i was concerned may be due to initialize, but it may be due to other factors. |
Summary
This PR disables the shared worker which we've had increasingly issues with lately. This should resolve the CBOR decoding issues and CID related issues, such as the ones in fission-codes/auth-lobby#115
The result should be slower, but more stable apps.Faster and more stable.Monitoring functions have been exposed through the
webnative.ipfs.node
object, example:Test plan (required)
Try a webnative app with this build. You can make a webnative build with
yarn build
and then use it in your app by pointing to the local webnative directory with{ "webnative": "file:../relative-path-to-webnative/" }
Closing issues
Closes #401