Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

fix: only accept cid for ipfs.dag.get #3675

Merged
merged 6 commits into from
May 7, 2021

Conversation

achingbrain
Copy link
Member

If you have a path within the DAG to resolve, pass it as path in the options object.

Makes the API conform to the documentation.

Fixes #3637

If you have a path within the DAG to resolve, pass it as `path` in
the options object as per the documentation.

Fixes #3637
@rvagg
Copy link
Member

rvagg commented May 7, 2021

Travis is failing because examples/traverse-ipld-graphs/eth.js is passing string CIDs, I guess we don't have type checking extending down into there too?

@achingbrain achingbrain requested review from rvagg and vasco-santos May 7, 2021 11:39
Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@achingbrain
Copy link
Member Author

I guess we don't have type checking extending down into there too?

This is intentional - the idea is to show a single idea as simply as possible and to not make any other technical decisions for the user, e.g. just plain js as far as possible, as they'll likely copy/paste example code into their own app so we don't want them to then have to remove extra scaffolding or framework code etc.

@achingbrain achingbrain merged commit bb8f8bc into master May 7, 2021
@achingbrain achingbrain deleted the fix/only-accept-cid-in-dag-get branch May 7, 2021 12:58
@CSDUMMI CSDUMMI mentioned this pull request May 16, 2021
@CSDUMMI
Copy link
Contributor

CSDUMMI commented May 16, 2021

Travis is failing because examples/traverse-ipld-graphs/eth.js is passing string CIDs, I guess we don't have type checking extending down into there too?

@rvagg the issue still exists in the release. #3689

@CSDUMMI
Copy link
Contributor

CSDUMMI commented May 16, 2021

I do not see, why we need to restrict the types.
This is a breaking change and not intuitive!

Because most js-IPFS functions (see Files API)
accept both CID and String.

This was referenced Dec 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Types] DAGAPI get requires CID but should also be string|Uint8Array?
4 participants