-
Notifications
You must be signed in to change notification settings - Fork 3
Dependency injection patterns for reducing bundle size #10
Comments
This is certainly interesting from my perspective, as smaller bundle sizes would help a great deal with building custom lightweight IPFS-based solutions in the browser. Presumably, for those concerned about not wanting to pre-determine which codecs they should support, there might be an "everything and the kitchen sink" variant or additional module that they could import? On a somewhat related note, I wonder if a similar approach could be taken to (help) address this issue somewhat (I don't mean to derail the conversion here, I can bring that up over there if it seems reasonable)? |
Very +1 I can understand this being an inconvenience for the IPFS use-case where you're expecting to encounter a broad range of codecs, but many of the use-cases I toy with have a narrower set of requirements, often just needing dag-cbor and only expecting to ever deal with dag-cbor. I think it's reasonable to expect applications building on IPLD (and not IPFS) to have such narrow requirements and the current kitchen-sink approach is just too heavy-weight. You might also be interested to hear that go-ipld-prime already has this kind of up-front feature injection already: https://github.com/rvagg/go-car-example/blob/4291971cc5ab84b878739711ff2ef60a031cd603/example.go#L312-L313 This may change in the future but currently it will fail to encode or decode codecs that you haven't told it about, you normally do this in an |
FWIW we already do something like this.
So in A way to reduce the number of hash functions would be good though, the crypto deps are the ones that really bloat the bundle sizes. |
That would still be ‘@ipld/block’, the version without any codecs would be ‘@ipld/block/bare’. |
I'd like to mention that I work on a similar approach for Rust IPLD. There we can use generics. Currently there is no Block API yet, but you can see how Multihash looks like with this approach here: multiformats/rust-multihash#60. You would pass on the multicodec (together with implementations) into the Block API, which passes it down to the Multihash level. |
+1 on this, the API looks reasonable to me. I'm looking to implement this approach in
@carsonfarmer I think so. Several of the multiformats repos could also benefit from this to help reduce size and add extensibility, such as multiformats/js-multihash#73 |
The docs over in https://github.com/multiformats/js-multiformats now suggest that the Block API supports a minimal setup with extensible codecs, essentially as outline here. Is this the case? Or will it be soon? Very excited about this 👍. |
I believe @mikeal is working on this library right now @carsonfarmer. We have various libraries (in various states) working with js-multiformats now including new @ipld/dag-cbor, @ipld/dag-json @ipld/dag-pb. My new Bitcoin work is against it too, there'll be an @ipld/bitcoin. and I'm about to land one for datastore-car that switches it entirely to js-multiformats too. We're all in on this now and full-speed ahead on making it work so it might be a good time to have a poke around at it and see how it feels. |
I’ll be getting to the Block API very soon! Took a small detour this week to get on ESM multiformats/js-multiformats#16 |
Amazing gang, this is stellar stuff! Happy to hear that (also happy to see the ESM stuff)! Going to dig into that as well, as we've been toying with the idea of moving to plain ESM output from our TS libraries. |
We only started considering it once ESM was unflagged in Node.js LTS https://nodejs.org/en/blog/release/v12.17.0/ Given that |
This issue is broader than just the
Block
API (it touches js-mutliformats, js-cid, js-ipfs, and js-libp2p) but this is as good a place as any to kick off the conversation.Currently, we ship with support for a large set of:
This is not sustainable. Even the multiformat table, which is only metadata, has become large enough to be a bundle size concern. The number of potential codecs and hashing functions will only grow over time.
In all the libraries we’ve written for IPLD in the last year, we depend on
Block
API throughrequire(‘@/ipld/block’)
. It’s how we create blocks which encode data (codecs) and CID’s (hashing and multiformats). Currently, these libraries justrequire(‘@ipld/block’)
which includes all the default codecs and has an interface for adding more codecs. There is no way to pair the default set down and no way to remove or add hashing functions.In considering this problem, I’ve been writing some of my newer libraries a little different. I’ve been doing the core implementation in a file called
bare.js
which exports a single function that accepts theBlock
class and the default codec. https://github.com/ipld/js-fbl/blob/master/src/bare.js#L8 Then inindex.js
I just return this module with the defaultrequire(‘@ipld/block’)
class. The idea is, we can expose aBlock
class in the future that comes without any codecs, hashing functions, or multiformat table entries, and the user can add entries for each that it would like to support. Then that class can be passed into the relevant IPLD libraries that need to create or decode blocks.As support for each format is added to the
Block
API the multiformat entries for each is added to the internal multiformat table. This means we’ll only have complete information for multiformat entries we actually have support for and this would cause a breaking change inCID
because the.codec
property would no longer work for codecs you didn’t explicitly add support for (the migration for this involves moving away from matching against the string and using the integer for the multiformat entry).I’d like get feedback on this approach and everyone’s thoughts on the API before I dig in and implement it everywhere.
I also want to make sure this is going to work for the other projects once they adopt the new
Block
API./cc @alanshaw @achingbrain @vmx @rvagg @jacobheun @carsonfarmer
The text was updated successfully, but these errors were encountered: