feat: filter origins & delegates #13
Conversation
only merge origins if mergeOrigins option is true
| constructor (private readonly heliaInstance: Helia, private readonly remotePinningClient: RemotePinningServiceClient, config?: HeliaRemotePinnerConfig) { | ||
| this.config = { | ||
| mergeOrigins: config?.mergeOrigins ?? false, |
There was a problem hiding this comment.
aren't these set as Required? does nullish check still apply?
There was a problem hiding this comment.
ah.. yes, I should remove from the Pick<Required. good catch
There was a problem hiding this comment.
wait, no, the this.config is the only place it's required. the passed in options are all optional.
There was a problem hiding this comment.
which means, in the constructor, we have to set the required options on this.config, but user doesn't need to pass anything.
There was a problem hiding this comment.
fixed an issue though: f0bf463
src/heliaRemotePinner.ts
Outdated
| const nodeOrigins = multiaddrs.map(multiaddr => multiaddr.toString()) | ||
| const filteredOrigins = this.config.filterOrigins?.(nodeOrigins) ?? nodeOrigins |
There was a problem hiding this comment.
can we call const nodeOrigins something else? in fact since this assignment is only used in case this.config.filterOrigins?.(nodeOrigins) ?? why spend time converting all multiaddrs.toString()?
There was a problem hiding this comment.
because filterOrigins takes an array of strings.. though it should probably take an array of Multiaddrs
src/heliaRemotePinner.ts
Outdated
| * | ||
| * @default (origins) => origins | ||
| */ | ||
| filterOrigins?: (origins: string[]) => string[] |
There was a problem hiding this comment.
wondering if there is a need support regexp.
There was a problem hiding this comment.
filtering multiaddrs with regex can be done in user provided function if they desire. full customization owned by fn caller
|
FYI: I have some additional updates here that i added while working on ipfs-examples/helia-examples#147 that I want to be sure are looked at, will update when i unblock ipfs-desktop and ipfs-webui issues |
|
🎉 This PR is included in version 1.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This PR adds more configuration options and no longer merges origins by default.
New configuration options:
boolean(origins: string[]) => string[](delegates: string[]) => string[]New functionality:
helia.libp2p.getMultiaddrs()origins.