feat: pass libp2pOptions to the bundle function #2591
Conversation
This allows bundle creation by merging/overriding option defaults instead of creating them in their entirety from scratch.
e.g.
```js
const ipfs = await IPFS.create({
libp2p: ({ libp2pOptions, peerInfo }) => {
libp2pOptions.transports.push(...)
return new Libp2p(defaultOptions)
}
})
```
License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
| const libp2pOptions = getLibp2pOptions({ options, config, datastore, peerInfo, peerBook }) | ||
| let libp2p | ||
|
|
||
| if (typeof options.libp2p === 'function') { | ||
| libp2p = options.libp2p({ libp2pOptions, options, config, datastore, peerInfo, peerBook }) | ||
| } else { | ||
| // Required inline to reduce startup time | ||
| const Libp2p = require('libp2p') | ||
| libp2p = new Libp2p(mergeOptions(libp2pOptions, get(options, 'libp2p', {}))) | ||
| } |
There was a problem hiding this comment.
So, options is getting passed into getLibp2pOptions, but then it is still passed into libp2p creation. Should getLibp2pOptions just handle merging all of that so options doesn't need to be passed again?
options.libp2p({ libp2pOptions, options, config, datastore, peerInfo, peerBook })
Along those lines, why is options being passed here? It feels like the result of getLibp2pOptions should just be able to get passed directly to both new Libp2p and options.libp2p. No other params should be needed right?
There was a problem hiding this comment.
Should getLibp2pOptions just handle merging all of that so options doesn't need to be passed again?
There may be some other IPFS option you want to use in your custom bundle function. It's "passed again" because that's what currently happens, and I'm trying to avoid a breaking change.
options.libp2p({ libp2pOptions, options, config, datastore, peerInfo, peerBook })
Along those lines, why is
optionsbeing passed here? It feels like the result ofgetLibp2pOptionsshould just be able to get passed directly to bothnew Libp2pandoptions.libp2p. No other params should be needed right?
options is IPFS options, passed to the IPFS constructor. It's passed here because that's currently what happens when you provide a custom bundle function. If we removed it, we'd have a breaking change.
getLibp2pOptions is passed IPFS options and returns libp2p options (I would call them defaults, but they're not really because they're based off the IPFS options and the IPFS config from the repo).
There was a problem hiding this comment.
Okay, so options and config are for IPFS? If so, would it be easier to understand if in the scope of the custom bundler options were the libp2p options and the IPFS ones were prefixed? If those are going to be exposed I think it might be helpful to make that distinction.
There was a problem hiding this comment.
nvm, that would be a breaking change. I think that would be clearer, but since the goal is to avoid the breaking change I think this is fine.
There was a problem hiding this comment.
I think that would be clearer
FWIW I agree!
This is PoC use of ipfs/js-ipfs#2591
There was a problem hiding this comment.
I am a simple man, I see change that lets me remove a lot of code, I 👍
Check ipfs/ipfs-companion#811 for how this PR simplifies customizing libp2p for js-ipfs in Brave.
| // Merge defaults with Node.js/browser/other environments options and configuration | ||
| return mergeOptions( | ||
| libp2pDefaults, | ||
| getEnvLibp2pOptions({ options, config, datastore, peerInfo, peerBook }), |
There was a problem hiding this comment.
nit: easier to read if options-to-be-merged follow same naming convention (also, runtime is the name of directory, so let's reuse that name)
| getEnvLibp2pOptions({ options, config, datastore, peerInfo, peerBook }), | |
| libp2pRuntimeDefaults({ options, config, datastore, peerInfo, peerBook }), |
This is PoC use of ipfs/js-ipfs#2591 BREAKING CHANGE: switched to Async Iterators version of JS API https://blog.ipfs.io/2020-02-01-async-await-refactor/ Code that is not related to embedded node with chromesockets will be refactored in separate commit.
Switches JS API to async iterators where possible. Includes switch to libp2p config override via ipfs/js-ipfs#2591 BREAKING CHANGE: switched to Async Iterators version of JS API https://blog.ipfs.io/2020-02-01-async-await-refactor/
Switches JS API to async iterators where possible. Includes switch to libp2p config override via ipfs/js-ipfs#2591 BREAKING CHANGE: switched to Async Iterators version of JS API https://blog.ipfs.io/2020-02-01-async-await-refactor/
Switches JS API to async iterators where possible. Includes switch to libp2p config override via ipfs/js-ipfs#2591 BREAKING CHANGE: switched to Async Iterators version of JS API https://blog.ipfs.io/2020-02-01-async-await-refactor/
This allows bundle creation by merging/overriding option defaults instead of creating them in their entirety from scratch. This is for convenience.
The
libp2pOptionspassed to the bundle function include any options defined in the required runtime files (for Node.js/browser config).To allow this, there is a slight change to
runtime/libp2p-nodejs.jsandruntime/libp2p-browser.jsto return their options instead of a libp2p instance. This actually removes some repeated boilerplate.e.g.
resolves #2579