holepunchto / corestore

A simple corestore that wraps a random-access-storage module
https://docs.holepunch.to
MIT License
74 stars 28 forks source link

Allow to set onwrite hook and provide context #12

Closed Frando closed 2 years ago

Frando commented 4 years ago

Hi, so hypercore supports an onwrite opt, if set the callback will be called on each write with (index, data, peer, cb), where index is the seq of the data written, data is the buffer to be written, peer is a peer object if the data was downloaded (and not appended).

When manually getting feeds from a Corestore with corestore.get, we can pass the onwrite option as usual. However, we usually will want to know the key of the feed in question inside the onwrite function. As the key is not passed into the function, it has to be in a closure where the key is available. When you want to track all writes in a Corestore, though, we run into a chicken-and-egg-problem - oftenly you know the feed's key only after creating it, making it quite cumbersome to have a generic onwrite handler for all feeds in Corestore.

I was thinking: What about adding an onwrite option to the Corestore opts, that, if set, sets an onwrite hook on all hypercores created in this corestore, while just handing off to the global onwrite with the feed's key as a first argument?

const store = new Corestore(storage, { onwrite })
function onwrite (key, index, data, peer, cb) {
// do your indexing business
}

I'd want to use this with kappa-sparse-indexer where I'm currently setting up event listeners for this, which has difficult edgecases with regard to timings (data is flowing in from downloads possibly in the same tick after ready is called).

If this sounds fine to you I'd open a PR.

Frando commented 4 years ago

Alternatively, hypercore could pass the key to the onwrite hook. For this to be a non-API breaking chagne, it would have to be the last argument, after the cb. That would be nicer, as there wouldn't be a need to differ between oorestore and hypercore. Not sure though what @mafintosh thinks (or if a breaking change is due anyway soonish where the key could just be the first argument to the onwrite hook).

andrewosh commented 4 years ago

I'd be fine to merge #14 for this and just have it handled at the corestore layer -- seems more natural than adding the key to hypercore's onwrite callback, since differentiating between keys makes less sense there.

mafintosh commented 2 years ago

no onwrite in 10 anymore