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

chore: update types #87

Merged
merged 4 commits into from
Apr 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
strategy:
matrix:
os: [windows-latest, ubuntu-latest, macos-latest]
node: [12, 14]
node: [14, 15]
fail-fast: true
steps:
- uses: actions/checkout@v2
Expand Down
28 changes: 16 additions & 12 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@
"extends": "ipfs"
},
"scripts": {
"prepare": "aegir build --no-bundle",
"lint": "aegir lint",
"build": "aegir build",
"prepare": "aegir build --no-bundle",
"test": "aegir test",
"test:node": "aegir test --target node",
"test:browser": "aegir test --target browser",
Expand All @@ -47,39 +47,43 @@
},
"homepage": "https://github.com/libp2p/js-interfaces#readme",
"dependencies": {
"@types/bl": "^2.1.0",
"@types/bl": "4.1.0",
Copy link

Choose a reason for hiding this comment

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

has this lost the ^ for a reason?

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch, mistyped! I will PR this shortly

"abort-controller": "^3.0.0",
"abortable-iterator": "^3.0.0",
"chai": "^4.2.0",
"chai": "^4.3.4",
"chai-checkmark": "^1.0.1",
"debug": "^4.3.1",
"delay": "^4.4.0",
"delay": "^5.0.0",
"detect-node": "^2.0.4",
"dirty-chai": "^2.0.1",
"err-code": "^2.0.3",
"err-code": "^3.0.1",
"it-goodbye": "^2.0.2",
"it-length-prefixed": "^3.1.0",
"it-pair": "^1.0.0",
"it-pipe": "^1.1.0",
"it-pushable": "^1.4.0",
"it-pushable": "^1.4.2",
"libp2p-crypto": "^0.19.0",
"libp2p-tcp": "^0.15.0",
"multiaddr": "^8.1.2",
"multibase": "^3.1.1",
"multihashes": "^3.1.1",
"multibase": "^4.0.2",
"multihashes": "^4.0.2",
"p-defer": "^3.0.0",
"p-limit": "^3.1.0",
"p-wait-for": "^3.2.0",
"peer-id": "^0.14.2",
"protons": "^2.0.0",
"sinon": "^9.2.4",
"sinon": "^10.0.0",
"streaming-iterables": "^5.0.4",
"uint8arrays": "^2.0.5"
"uint8arrays": "^2.1.3"
},
"devDependencies": {
"aegir": "^29.2.0",
"@types/debug": "^4.1.5",
Copy link

Choose a reason for hiding this comment

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

I'm assuming this is a devdep because it's an internal concern and we only want it to typecheck internal consistency and don't expose debug to users in any way form here. Is that correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, that is correct!

"aegir": "^32.1.0",
"cids": "^1.1.6",
"events": "^3.3.0",
"it-handshake": "^1.0.2",
"rimraf": "^3.0.2"
"rimraf": "^3.0.2",
"util": "^0.12.3"
},
"contributors": [
"Alan Shaw <[email protected]>",
Expand Down
9 changes: 9 additions & 0 deletions src/connection/connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,15 @@ class Connection {

module.exports = Connection

/**
* @param {multiaddr|undefined} localAddr
* @param {PeerId} localPeer
* @param {PeerId} remotePeer
* @param {(protocols: string | string[]) => Promise<{ stream: import("../stream-muxer/types").MuxedStream; protocol: string; }>} newStream
* @param {() => Promise<void>} close
* @param {() => import("../stream-muxer/types").MuxedStream[]} getStreams
* @param {{ direction: any; timeline: any; multiplexer?: string | undefined; encryption?: string | undefined; }} stat
*/
function validateArgs (localAddr, localPeer, remotePeer, newStream, close, getStreams, stat) {
if (localAddr && !multiaddr.isMultiaddr(localAddr)) {
throw errCode(new Error('localAddr must be an instance of multiaddr'), 'ERR_INVALID_PARAMETERS')
Expand Down
1 change: 1 addition & 0 deletions src/connection/tests/connection.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// @ts-nocheck interface tests
/* eslint-env mocha */

'use strict'
Expand Down
1 change: 1 addition & 0 deletions src/connection/tests/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// @ts-nocheck interface tests
/* eslint-env mocha */

'use strict'
Expand Down
11 changes: 11 additions & 0 deletions src/content-routing/types.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export = ContentRouting;

import PeerId from 'peer-id'
import Multiaddr from 'multiaddr'
import CID from 'cids'

declare class ContentRouting {
constructor (options: Object);
provide (cid: CID): Promise<void>;
findProviders (cid: CID, options: Object): AsyncIterable<{ id: PeerId, multiaddrs: Multiaddr[] }>;
Copy link

Choose a reason for hiding this comment

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

maybe better to define a Provider here rather than an anonymous type for {id, multiaddrs}?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is more a representation of a peer rather then a Provider. As you pointed below, findPeer also has the same. We have been discussing about potentially change the API return type of this in #85

We will probably look back into this

}
Empty file removed src/content-routing/types.ts
Empty file.
1 change: 1 addition & 0 deletions src/crypto/tests/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// @ts-nocheck interface tests
/* eslint-env mocha */
'use strict'

Expand Down
File renamed without changes.
1 change: 1 addition & 0 deletions src/peer-discovery/tests/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// @ts-nocheck interface tests
/* eslint-env mocha */
'use strict'

Expand Down
10 changes: 10 additions & 0 deletions src/peer-discovery/types.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export = PeerDiscovery;

import events from 'events';

declare class PeerDiscovery extends events.EventEmitter {
constructor (options: Object);
start (): Promise<void>;
stop (): Promise<void>;
tag: string;
}
10 changes: 10 additions & 0 deletions src/peer-routing/types.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export = PeerRouting;

import PeerId from 'peer-id'
import Multiaddr from 'multiaddr'

declare class PeerRouting {
Copy link
Member

Choose a reason for hiding this comment

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

its not exported ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was doing a named import on libp2p, but it makes sense to export this by default. I changed it here and in the other relevant types files

constructor (options?: Object);
findPeer (peerId: PeerId, options?: Object): Promise<{ id: PeerId, multiaddrs: Multiaddr[] }>;
Copy link

Choose a reason for hiding this comment

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

the {id,multiaddrs} appears twice here again, further suggests it should be it's own Provider type

getClosestPeers(key: Uint8Array, options?: Object): AsyncIterable<{ id: PeerId, multiaddrs: Multiaddr[] }>;
}
46 changes: 26 additions & 20 deletions src/pubsub/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ const { pipe } = require('it-pipe')

const MulticodecTopology = require('../topology/multicodec-topology')
const { codes } = require('./errors')
/**
* @type {typeof import('./message')}
*/

const message = require('./message')
const PeerStreams = require('./peer-streams')
const { SignaturePolicy } = require('./signature-policy')
Expand All @@ -29,9 +27,9 @@ const {
* @typedef {import('bl')} BufferList
* @typedef {import('../stream-muxer/types').MuxedStream} MuxedStream
* @typedef {import('../connection/connection')} Connection
* @typedef {import('./message').RPC} RPC
* @typedef {import('./message').SubOpts} RPCSubOpts
* @typedef {import('./message').Message} RPCMessage
* @typedef {import('./message/types').RPC} RPC
* @typedef {import('./message/types').SubOpts} RPCSubOpts
* @typedef {import('./message/types').Message} RPCMessage
* @typedef {import('./signature-policy').SignaturePolicyType} SignaturePolicyType
*/

Expand All @@ -44,6 +42,16 @@ const {
* @property {Uint8Array} data
* @property {Uint8Array} [signature]
* @property {Uint8Array} [key]
*
* @typedef {Object} PubsubProperties
* @property {string} debugName - log namespace
* @property {Array<string>|string} multicodecs - protocol identificers to connect
* @property {Libp2p} libp2p
*
* @typedef {Object} PubsubOptions
* @property {SignaturePolicyType} [globalSignaturePolicy = SignaturePolicy.StrictSign] - defines how signatures should be handled
* @property {boolean} [canRelayMessage = false] - if can relay messages not subscribed
* @property {boolean} [emitSelf = false] - if publish should emit to self, if subscribed
*/

/**
Expand All @@ -52,13 +60,7 @@ const {
*/
class PubsubBaseProtocol extends EventEmitter {
/**
* @param {Object} props
* @param {string} props.debugName - log namespace
* @param {Array<string>|string} props.multicodecs - protocol identificers to connect
* @param {Libp2p} props.libp2p
* @param {SignaturePolicyType} [props.globalSignaturePolicy = SignaturePolicy.StrictSign] - defines how signatures should be handled
* @param {boolean} [props.canRelayMessage = false] - if can relay messages not subscribed
* @param {boolean} [props.emitSelf = false] - if publish should emit to self, if subscribed
* @param {PubsubProperties & PubsubOptions} props
* @abstract
*/
constructor ({
Expand All @@ -83,8 +85,9 @@ class PubsubBaseProtocol extends EventEmitter {

super()

this.log = debug(debugName)
this.log.err = debug(`${debugName}:error`)
this.log = Object.assign(debug(debugName), {
err: debug(`${debugName}:error`)
})

/**
* @type {Array<string>}
Expand Down Expand Up @@ -122,7 +125,7 @@ class PubsubBaseProtocol extends EventEmitter {

// validate signature policy
if (!SignaturePolicy[globalSignaturePolicy]) {
throw errcode(new Error('Invalid global signature policy'), codes.ERR_INVALID_SIGUATURE_POLICY)
throw errcode(new Error('Invalid global signature policy'), codes.ERR_INVALID_SIGNATURE_POLICY)
}

/**
Expand Down Expand Up @@ -379,7 +382,9 @@ class PubsubBaseProtocol extends EventEmitter {

if (subs.length) {
// update peer subscriptions
subs.forEach((subOpt) => this._processRpcSubOpt(idB58Str, subOpt))
subs.forEach((/** @type {RPCSubOpts} */ subOpt) => {
this._processRpcSubOpt(idB58Str, subOpt)
})
this.emit('pubsub:subscription-change', peerStreams.id, subs)
}

Expand All @@ -389,8 +394,9 @@ class PubsubBaseProtocol extends EventEmitter {
}

if (msgs.length) {
msgs.forEach(message => {
if (!(this.canRelayMessage || message.topicIDs.some((topic) => this.subscriptions.has(topic)))) {
// @ts-ignore RPC message is modified
msgs.forEach((message) => {
if (!(this.canRelayMessage || message.topicIDs.some((/** @type {string} */ topic) => this.subscriptions.has(topic)))) {
this.log('received message we didn\'t subscribe to. Dropping.')
return
}
Expand Down Expand Up @@ -589,7 +595,7 @@ class PubsubBaseProtocol extends EventEmitter {
for (const topic of message.topicIDs) {
const validatorFn = this.topicValidators.get(topic)
if (!validatorFn) {
continue
continue // eslint-disable-line
Copy link

Choose a reason for hiding this comment

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

you could just flip the logic here to if (validatorFn) { await validatorFn(..) } to avoid the disable

Copy link
Member Author

Choose a reason for hiding this comment

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

I will also update this in the follow up PR

}
await validatorFn(topic, message)
}
Expand Down
17 changes: 8 additions & 9 deletions src/pubsub/message/index.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
'use strict'

// @ts-ignore protons not typed
const protons = require('protons')

const rpcProto = protons(require('./rpc.proto.js'))
const RPC = rpcProto.RPC
const topicDescriptorProto = protons(require('./topic-descriptor.proto.js'))

/**
* @module pubsub/message/index
*/
exports = module.exports
exports.rpc = rpcProto
exports.td = topicDescriptorProto
exports.RPC = RPC
exports.Message = RPC.Message
exports.SubOpts = RPC.SubOpts
module.exports = {
Copy link

Choose a reason for hiding this comment

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

out of interest: was this change forced by the type checking?

Copy link
Member Author

Choose a reason for hiding this comment

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

By the linter, but I think this can cause problems with inferred types and some bundlers. @hugomrdias has the bigger picture here on why we should avoid this

rpc: rpcProto,
td: topicDescriptorProto,
RPC,
Message: RPC.Message,
SubOpts: RPC.SubOpts
}
5 changes: 5 additions & 0 deletions src/pubsub/message/types.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { RPC, Message, SubOpts } from './types'

export type RPC = RPC
export type Message = Message
export type SubOpts = SubOpts
11 changes: 6 additions & 5 deletions src/pubsub/peer-streams.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
'use strict'

const debug = require('debug')
const log = Object.assign(debug('libp2p-pubsub:peer-streams'), {
error: debug('libp2p-pubsub:peer-streams:err')
})

/** @typedef {import('../types').EventEmitterFactory} Events */
/** @type Events */
const EventEmitter = require('events')
Expand All @@ -10,10 +15,6 @@ const pushable = require('it-pushable')
const { pipe } = require('it-pipe')
const { source: abortable } = require('abortable-iterator')
const AbortController = require('abort-controller').default
const debug = require('debug')

const log = debug('libp2p-pubsub:peer-streams')
log.error = debug('libp2p-pubsub:peer-streams:error')

/**
* @typedef {import('../stream-muxer/types').MuxedStream} MuxedStream
Expand Down Expand Up @@ -168,7 +169,7 @@ class PeerStreams extends EventEmitter {
this.outboundStream,
lp.encode(),
this._rawOutboundStream
).catch(err => {
).catch(/** @param {Error} err */ err => {
log.error(err)
})

Expand Down
1 change: 1 addition & 0 deletions src/pubsub/tests/api.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// @ts-nocheck interface tests
/* eslint-env mocha */
'use strict'

Expand Down
1 change: 1 addition & 0 deletions src/pubsub/tests/emit-self.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// @ts-nocheck interface tests
/* eslint-env mocha */
'use strict'

Expand Down
1 change: 1 addition & 0 deletions src/pubsub/tests/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// @ts-nocheck interface tests
/* eslint-env mocha */
'use strict'

Expand Down
1 change: 1 addition & 0 deletions src/pubsub/tests/messages.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// @ts-nocheck interface tests
/* eslint-env mocha */
'use strict'

Expand Down
1 change: 1 addition & 0 deletions src/pubsub/tests/multiple-nodes.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// @ts-nocheck interface tests
/* eslint-env mocha */
/* eslint max-nested-callbacks: ["error", 6] */
'use strict'
Expand Down
1 change: 1 addition & 0 deletions src/pubsub/tests/two-nodes.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// @ts-nocheck interface tests
/* eslint-env mocha */
/* eslint max-nested-callbacks: ["error", 6] */
'use strict'
Expand Down
1 change: 1 addition & 0 deletions src/pubsub/tests/utils.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// @ts-nocheck interface tests
'use strict'

const { expect } = require('chai')
Expand Down
Loading