Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

Assert that the argument is indeed a Exchange #68

Open
daviddias opened this issue Jul 4, 2017 · 3 comments
Open

Assert that the argument is indeed a Exchange #68

daviddias opened this issue Jul 4, 2017 · 3 comments
Labels
exp/expert Having worked on the specific codebase is important kind/enhancement A net-new feature or improvement to an existing feature P2 Medium: Good to have, but can wait until someone steps up status/ready Ready to be worked

Comments

@daviddias
Copy link
Member

Following up: #67 (comment)

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.

@daviddias daviddias added exp/expert Having worked on the specific codebase is important kind/enhancement A net-new feature or improvement to an existing feature help wanted Seeking public contribution on this issue labels Jul 4, 2017
@daviddias daviddias added the status/deferred Conscious decision to pause or backlog label Jul 9, 2017
@daviddias daviddias added status/ready Ready to be worked and removed status/deferred Conscious decision to pause or backlog labels Sep 13, 2017
@daviddias daviddias added the P2 Medium: Good to have, but can wait until someone steps up label Oct 17, 2017
@0x-r4bbit
Copy link

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
Copy link
Member

vmx commented Jul 8, 2018

@PascalPrecht There's the plan to use Flow: ipfs/js-ipfs#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
Copy link

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?

@daviddias daviddias removed the help wanted Seeking public contribution on this issue label Oct 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/expert Having worked on the specific codebase is important kind/enhancement A net-new feature or improvement to an existing feature P2 Medium: Good to have, but can wait until someone steps up status/ready Ready to be worked
Projects
None yet
Development

No branches or pull requests

3 participants