Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add balance field to BaseNft #123

Merged
merged 2 commits into from
Apr 15, 2022

Conversation

donutdaniel
Copy link
Contributor

Seems like the balance field returns when possible on a BaseNft object, through both the getNFTs call (with or without metadata). This is useful for determining the amount of the tokenId when referencing an erc1155.

Adding this type makes it so that typescript does not throw errors.

Btw, is there a difference between v1 and v2 api?

path: "/v1/getNFTs/",
. Current documentation on https://docs.alchemy.com/alchemy/enhanced-apis/nft-api/getnfts references v2.

Thanks for this useful library!

Copy link
Member

@thebrianchen thebrianchen left a comment

Choose a reason for hiding this comment

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

Thanks for noticing and including the fix! The balance field will always be populated in the response and isn't optional. Can you please update that?

There is no difference between the v1 and v2 versions of the NFT API. We ended up bumping up the version since all of the other endpoints were already on v2.

@donutdaniel
Copy link
Contributor Author

I left it as optional since an erroneous response will omit the balance field. Thoughts? Can certainly make the change though

@thebrianchen
Copy link
Member

@donutdaniel This actually a bug, and the balance field should always be included in the return. The backend team is working on a fix in the meantime. Thanks for catching, and can you please make the field required?

@donutdaniel
Copy link
Contributor Author

Gotcha, just updated. Thanks for the quick turnaround!

@thebrianchen thebrianchen merged commit 51f0417 into alchemyplatform:master Apr 15, 2022
@thebrianchen
Copy link
Member

thanks for your contribution! Will have a new release out in the coming week or two

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants