feat: share deviceType with local peers #461
Conversation
fixes #451 and #452. Does not store deviceType in project deviceInfo records, which will be done in a follow up PR (see #455) which also requires changes to @mapeo/schema. `deviceType` is optional, and must be set in the constructor of MapeoManager. Ideally this should not be possible to change, but not sure how to enforce that.
test-e2e/local-peers.js
Outdated
| t.teardown(() => { | ||
| disconnectPeers(managers) | ||
| }) |
There was a problem hiding this comment.
Nit: I think this callback should return a promise. One of these should work:
| t.teardown(() => { | |
| disconnectPeers(managers) | |
| }) | |
| t.teardown(async () => { | |
| await disconnectPeers(managers) | |
| }) |
...or:
| t.teardown(() => { | |
| disconnectPeers(managers) | |
| }) | |
| t.teardown(() => disconnectPeers(managers)) |
The latter doesn't immediately work because the callback to t.teardown() is expected to return void | Promise<void>, but disconnectPeers returns Promise<void[]>. I'd fix that by updating test-e2e/utils.js like this:
diff --git a/test-e2e/utils.js b/test-e2e/utils.js
index e5fd403..7dd5575 100644
--- a/test-e2e/utils.js
+++ b/test-e2e/utils.js
@@ -24,7 +24,7 @@ const clientMigrationsFolder = new URL('../drizzle/client', import.meta.url)
* @param {readonly MapeoManager[]} managers
*/
export async function disconnectPeers(managers) {
- return Promise.all(
+ await Promise.all(
managers.map(async (manager) => {
return manager.stopLocalPeerDiscovery({ force: true })
})There was a problem hiding this comment.
ugh this is where typescript annoys me. It gave the error when writing it the original way, and to keep it happy I added the braces, which ends up introducing this bug. Good example of typescript prompting me to write buggy code when how I wrote it (which errors typescript) would not actually cause any bugs.
There was a problem hiding this comment.
Not sure if it works for us, but we might be able to enable a "no floating promises" ESLint rule to help with this kind of problem.
fixes #451 and #452. Does not store deviceType in project deviceInfo
records, which will be done in a follow up PR (see #455) which also
requires changes to @mapeo/schema.
deviceTypeis optional, and must beset in the constructor of MapeoManager. Ideally this should not be
possible to change, but not sure how to enforce that.