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

[SDK] Add option to NOT load NFT metadata #1823

Closed
wants to merge 1 commit into from
Closed

Conversation

kien-ngo
Copy link
Contributor

Problem solved

Short description of the bug fixed or feature added

Changes made

  • Public API changes: list the public API changes made if any
  • Internal API changes: explain the internal logic changes

How to test

  • Automated tests: link to unit test file
  • Manual tests: step by step instructions on how to test

@changeset-bot
Copy link

changeset-bot bot commented Oct 21, 2023

⚠️ No Changeset found

Latest commit: 10dda31

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@kien-ngo kien-ngo added the DO NOT MERGE This pull request is still in progress and is not ready to be merged. label Oct 21, 2023
type: "ERC721",
supply: "1",
};
return nft as T extends true | undefined ? NFT : NFTWithoutMetadata;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find a way to solve this without using type assertion.
Googling leads to this open issue: microsoft/TypeScript#33912

@@ -425,11 +444,10 @@ export class Erc721<
public async getOwned(
walletAddress?: AddressOrEns,
queryParams?: QueryAllParams,
) {
): Promise<NFT[] | NFTWithoutMetadata[]> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't wrap my head around how to deal with the typings here

@kien-ngo
Copy link
Contributor Author

kien-ngo commented Oct 21, 2023

@joaquim-verges The code is not complete but I'd love to have a review before I go too far. I have left a few comments above in the code ^

@kien-ngo kien-ngo self-assigned this Nov 30, 2023
@kien-ngo kien-ngo closed this Dec 13, 2023
@kien-ngo kien-ngo deleted the kien/loadMetadata branch March 25, 2024 12:10
jnsdls pushed a commit that referenced this pull request Jun 19, 2024
…0 to 4.2.1 (#1823)

build(deps-dev): bump @trivago/prettier-plugin-sort-imports

Bumps [@trivago/prettier-plugin-sort-imports](https://github.com/trivago/prettier-plugin-sort-imports) from 4.2.0 to 4.2.1.
- [Release notes](https://github.com/trivago/prettier-plugin-sort-imports/releases)
- [Changelog](https://github.com/trivago/prettier-plugin-sort-imports/blob/main/CHANGELOG.md)
- [Commits](trivago/prettier-plugin-sort-imports@v4.2.0...v4.2.1)

---
updated-dependencies:
- dependency-name: "@trivago/prettier-plugin-sort-imports"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO NOT MERGE This pull request is still in progress and is not ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant