ipfs / js-ipfs-block-service

JavaScript Implementation of Block and BlockService
MIT License
13 stars 20 forks source link

Assert that the argument is indeed a Exchange #68

Open daviddias opened 7 years ago

daviddias commented 7 years ago

Following up: https://github.com/ipfs/js-ipfs-block-service/pull/67#discussion_r125497435

we should probably add some lightweight checks that the expected functions exists on the object that gets passed in, to prevent scenarios when you set an exchange then call put and get a error about that the function is undefined, we can fail as early as when setting the exchange, with a more friendly error.

0x-r4bbit commented 6 years ago

This looks like a quick-win. I think though we should talk about what those "expected" functions are. Shall we just go with the ones being used right now (put(), putMany(), get())? Otherwise we can also do an instanceof check, but obviously this will only work with Bitswap instances, which might not be the desired behaviour.

Has there been any discussion about using TypeScript for the js-ipfs(related) implementations? Then, those checks won't be needed anymore as TS wouldn't even allow users of those API to pass anything else than anything that implements an Exchange API.

Just thinking out loud here :)

vmx commented 6 years ago

@PascalPrecht There's the plan to use Flow: https://github.com/ipfs/js-ipfs/issues/1260. Though it will be a long way to go as everyone would need to use the tooling in order to remove all checks (or at least a lot of ppl, so that you can say: "Things might break if you don't use certain tooling".

0x-r4bbit commented 6 years ago

Thanks for the feedback @vmx !

I personally have zero experience with flow, so I'm not even sure how it differs from TypeScript.

Regarding:

"Things might break if you don't use certain tooling"

So from a developer perspective I'd assume the build process (that is in use anyways) will take care of that. As a consumer of those APIs, I'd probably consume already transpiled artifacts and never have to know that those have been originally written in something like Flow.

I'll read through your linked issue.

Any thoughts on the other points I've raised here?