-
Notifications
You must be signed in to change notification settings - Fork 25
fix(rpc): setup swarm connection on next tick to fix a bug with dht-relay #92
base: master
Are you sure you want to change the base?
Conversation
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.
Why timeout instead of nextTick, what is the reason for skipping tests?
Any ideas what caused changes in package-lock?
packages/rpc/index.js
Outdated
this._swarm.on('connection', (connection) => { | ||
// Setup a new RPC instance on every connection | ||
// waiting for next tick fixes an obscure bug when using @hyperswarm/dht-relay | ||
setTimeout(() => this.setup(connection), 0) |
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.
Why not use process.nextTick()
instead?
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.
browser support
what about skipped tests and lock file change? |
It is important to note that lock file doesn't get published on NPM, it is merely for development purposes. I often delete it and reinstall everything, just to make sure I am developing on latest versions of everything. Maybe we should remove it. Skipped tests are mostly dealing with deprecated API (stuff that doesn't use |
for publishing there is a shrinkwrap
Haven't you had a problem because of it recently? |
Does it mean that they can be safely removed? |
No, any code that is still using Drivestore, but updates Corestore will get errors. Luckily for now Bitkit is the main user, and lockfiles are helping there. After opening this issue (and delivering this fix manually in my local attempt to move Up to @rbndg, maybe we can close these two PRs and wait for |
Let me take 30 minutes to see if I can update drivestore to use CoreData as a temporary fix! |
Added in separate PR to make it easier to review #94 |
Updated after merging #94, it doesn't solve https://github.com/synonymdev/slashtags-core-data/pull/19 so not sure if it is worth merging now. Maybe after solving that it wouldn't be needed. |
No description provided.