Skip to content
This repository was archived by the owner on Jun 26, 2023. It is now read-only.

Conversation

achingbrain
Copy link
Member

Refactors interfaces and classes used by libp2p-interfaces to use the async peer store from libp2p/js-libp2p#1058

BREAKING CHANGE: peerstore methods are now all async

Refactors interfaces and classes used by `libp2p-interfaces` to use
the async peer store from libp2p/js-libp2p#1058
Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is looking good 🎉
just left a nit on the tests

})

// 'change:protocols' event handler is async
await delay(10)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not listen for the event?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hander that invokes topology._updatePeers is async, so we can't just listen for the event as we're interested in the side-effects of the handler being invoked.

From a practical perspective the mock peer store used in the tests doesn't really do any async work, it just defers execution using the async keyword so delay here puts the rest of the test into the microtask queue - the handler is ahead of it in the same queue so this is enough for the test to pass.

Longer term we shouldn't test for side-effects like this as it's really fragile and makes refactoring hard.

@achingbrain achingbrain merged commit d7a1975 into release/v2.x.x Dec 30, 2021
@achingbrain achingbrain deleted the feat/async-peer-store branch December 30, 2021 06:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants