Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

RFC: allow passing constructor for the libp2p modules #1383

Closed
wants to merge 1 commit into from

Conversation

pgte
Copy link
Contributor

@pgte pgte commented Jun 4, 2018

WIP

Context: allow overriding the libp2p modules, permitting this type of options:

const ipfs = new IPFS({
  libp2p: { modules }
})

return ipfs

function modules (peerInfo) {
  const appTransport = AppTransport(appName, new WebSocketStar({ id: peerInfo.id }))
  return {
    transport: [ appTransport ],
    discovery: [ appTransport.discovery ]
  }
}

Some libp2p modules require the peer id in the constructor. Instead of a fancy DI framework, I propose a simple solution that allows to pass a function that will act as a module constructor.

This could be made to be even more flexible, like, for instance, by allowing some transports to be passed as instances and others as a constructor, like this:

const ipfs = new IPFS({
  libp2p: {
    modules: {
      transport: [
        ws,
        (peerInfo) => myWonderfulTransport(peerInfo.id)
      ]
    }
  }
})

(What this doesn't solve is the transport -> discovery coupling, which sometimes may be required. I suggest this by adding transport.discovery to the list of discovery modules if it exists).

I prefer this type of simplistic approach over some others that have been discussed, as this does not require libp2p module constructors to implement specific interfaces or require generic DI libs.

@ghost ghost assigned pgte Jun 4, 2018
@ghost ghost added the status/in-progress In progress label Jun 4, 2018
@pgte pgte changed the title WIP: allow passing constructor for the libp2p modules RFC: allow passing constructor for the libp2p modules Jun 4, 2018
@daviddias
Copy link
Member

@pgte see libp2p/js-libp2p#166. I'm adding support to instances and classes there as well

@alanshaw
Copy link
Member

alanshaw commented Jun 8, 2018

I like this approach in general but it sounds like @diasdavid wants to solve the problem in the libp2p layer. I think that's sensible as folks using libp2p directly will run into the same issue.

@pgte
Copy link
Contributor Author

pgte commented Jun 11, 2018

@alanshaw Absolutely, that’s a more permanent solution, I’ll use this one instead. Thanks!

@pgte pgte closed this Jun 11, 2018
@ghost ghost removed the status/in-progress In progress label Jun 11, 2018
@daviddias daviddias deleted the feat/libp2p-dynamic-modules branch June 19, 2018 04:57
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.

3 participants