search

ipfs / helia

An implementation of IPFS in JavaScript
https://helia.io
Other
916 stars 95 forks source link

feat: export dag walkers #414

Closed saul-jb closed 7 months ago

saul-jb commented 8 months ago

This simple PR just exports the dag-walkers functionality out of utils saving the need to re-implement it if it is required.

achingbrain commented 7 months ago

Thanks for opening this PR.

Could you speak a little to the use-case that requires exporting these separately?

The default set of DAG Walkers are always configured on a Helia node and are available via the .dagWalkers property - any extras passed to the config are merged in to the default list.

saul-jb commented 7 months ago

Could you speak a little to the use-case that requires exporting these separately?

It's simply using them outside of Helia. This file implements the most common DAG walkers and rather than copy/paste them into my project it makes sense to just be able to import them.

SgtPooki commented 7 months ago

We discussed this in the Helia WG last week. I think we would need to move these to a separate utils package that can be maintained by the community instead of surfacing as a part of the public Helia API

achingbrain commented 7 months ago

The suggestion on maintainers call was to add a method to each IPLD codec that yields CIDs from a block corresponding to that codec, but I don't even think we need to do that - we can use the existing links() function on a multiformats/block to yield them instead.

This is how it used to be done in js-ipfs: https://github.com/ipfs/js-ipfs-repo/blob/master/packages/ipfs-repo/src/utils/walk-dag.js

We may end up removing the dag walkers here entirely to reduce code bloat, which is also why things shouldn't be exported unless they are being used.

achingbrain commented 7 months ago

I'm going to close this as it's not necessary to export these as part of the Helia API, though thank you for opening this PR and for taking the time to explain your reasoning.

Follow-up: #447