Skip to content

chore: explicitly pass chainIds to detectNfts #5448

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

Merged
merged 10 commits into from
Mar 18, 2025

Conversation

sahar-fehri
Copy link
Contributor

@sahar-fehri sahar-fehri commented Mar 7, 2025

Explanation

PR updates detectNfts function to explicitly receive an array of chainIds to detectNfts for.

References

Changelog

@metamask/assets-controllers

  • BREAKING: detectNfts fct now accepts chainIds as a new arg
  • REMOVED: Removed networkClientId as an optional param in detectNfts fct in favor of new chainIds required arg.
  • ADDED: addNft fct now accepts chainId as an additional optional param

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@sahar-fehri sahar-fehri marked this pull request as ready for review March 7, 2025 16:10
@sahar-fehri sahar-fehri requested a review from a team as a code owner March 7, 2025 16:10
@sahar-fehri
Copy link
Contributor Author

@metamaskbot publish-preview

Copy link
Contributor

Preview builds have been published. See these instructions for more information about preview builds.

Expand for full list of packages and versions.
{
  "@metamask-previews/accounts-controller": "26.0.0-preview-912bed25",
  "@metamask-previews/address-book-controller": "6.0.3-preview-912bed25",
  "@metamask-previews/announcement-controller": "7.0.3-preview-912bed25",
  "@metamask-previews/approval-controller": "7.1.3-preview-912bed25",
  "@metamask-previews/assets-controllers": "53.0.0-preview-912bed25",
  "@metamask-previews/base-controller": "8.0.0-preview-912bed25",
  "@metamask-previews/bridge-controller": "5.0.0-preview-912bed25",
  "@metamask-previews/bridge-status-controller": "5.0.0-preview-912bed25",
  "@metamask-previews/build-utils": "3.0.3-preview-912bed25",
  "@metamask-previews/composable-controller": "11.0.0-preview-912bed25",
  "@metamask-previews/controller-utils": "11.6.0-preview-912bed25",
  "@metamask-previews/earn-controller": "0.7.0-preview-912bed25",
  "@metamask-previews/ens-controller": "15.0.2-preview-912bed25",
  "@metamask-previews/eth-json-rpc-provider": "4.1.8-preview-912bed25",
  "@metamask-previews/gas-fee-controller": "22.0.3-preview-912bed25",
  "@metamask-previews/json-rpc-engine": "10.0.3-preview-912bed25",
  "@metamask-previews/json-rpc-middleware-stream": "8.0.7-preview-912bed25",
  "@metamask-previews/keyring-controller": "21.0.0-preview-912bed25",
  "@metamask-previews/logging-controller": "6.0.4-preview-912bed25",
  "@metamask-previews/message-manager": "12.0.1-preview-912bed25",
  "@metamask-previews/multichain": "4.0.0-preview-912bed25",
  "@metamask-previews/multichain-network-controller": "0.2.0-preview-912bed25",
  "@metamask-previews/multichain-transactions-controller": "0.7.0-preview-912bed25",
  "@metamask-previews/name-controller": "8.0.3-preview-912bed25",
  "@metamask-previews/network-controller": "22.2.1-preview-912bed25",
  "@metamask-previews/notification-services-controller": "3.0.0-preview-912bed25",
  "@metamask-previews/permission-controller": "11.0.6-preview-912bed25",
  "@metamask-previews/permission-log-controller": "3.0.3-preview-912bed25",
  "@metamask-previews/phishing-controller": "12.4.0-preview-912bed25",
  "@metamask-previews/polling-controller": "12.0.3-preview-912bed25",
  "@metamask-previews/preferences-controller": "17.0.0-preview-912bed25",
  "@metamask-previews/profile-sync-controller": "10.0.0-preview-912bed25",
  "@metamask-previews/queued-request-controller": "9.0.1-preview-912bed25",
  "@metamask-previews/rate-limit-controller": "6.0.3-preview-912bed25",
  "@metamask-previews/remote-feature-flag-controller": "1.6.0-preview-912bed25",
  "@metamask-previews/selected-network-controller": "21.0.1-preview-912bed25",
  "@metamask-previews/signature-controller": "25.0.0-preview-912bed25",
  "@metamask-previews/token-search-discovery-controller": "2.1.0-preview-912bed25",
  "@metamask-previews/transaction-controller": "48.0.0-preview-912bed25",
  "@metamask-previews/user-operation-controller": "27.0.0-preview-912bed25"
}

@sahar-fehri sahar-fehri merged commit b43f657 into main Mar 18, 2025
181 checks passed
@sahar-fehri sahar-fehri deleted the feat/explicitly-pass-chainIds-to-detectNFts branch March 18, 2025 11:27
github-merge-queue bot pushed a commit to MetaMask/metamask-extension that referenced this pull request Mar 19, 2025
## **Description**

PR to add network filter to NFT tab.
This core PR MetaMask/core#5448 should be
released before updating this one.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/30903?quickstart=1)

## **Related issues**

Fixes:

## **Manual testing steps**

1. Choose Ethereum mainnet
2. Click on NFTs tab
3. You should see network filter + your detected NFTs on Ethereum
mainnet and linea (if you have any)
4. Switch between networks and try importing manually your NFTs on
networks that are not (Ethereum/linea)
5. Make sure you see your NFTs correctly

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->


https://github.com/user-attachments/assets/81b99e68-3d25-40ca-9c0f-f103b5904fa6



## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

---------

Co-authored-by: MetaMask Bot <[email protected]>
github-merge-queue bot pushed a commit to MetaMask/metamask-extension that referenced this pull request Mar 19, 2025
## **Description**

PR to add network filter to NFT tab.
This core PR MetaMask/core#5448 should be
released before updating this one.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/30903?quickstart=1)

## **Related issues**

Fixes:

## **Manual testing steps**

1. Choose Ethereum mainnet
2. Click on NFTs tab
3. You should see network filter + your detected NFTs on Ethereum
mainnet and linea (if you have any)
4. Switch between networks and try importing manually your NFTs on
networks that are not (Ethereum/linea)
5. Make sure you see your NFTs correctly

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->


https://github.com/user-attachments/assets/81b99e68-3d25-40ca-9c0f-f103b5904fa6



## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

---------

Co-authored-by: MetaMask Bot <[email protected]>
github-merge-queue bot pushed a commit to MetaMask/metamask-extension that referenced this pull request Mar 19, 2025
## **Description**

PR to add network filter to NFT tab.
This core PR MetaMask/core#5448 should be
released before updating this one.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/30903?quickstart=1)

## **Related issues**

Fixes:

## **Manual testing steps**

1. Choose Ethereum mainnet
2. Click on NFTs tab
3. You should see network filter + your detected NFTs on Ethereum
mainnet and linea (if you have any)
4. Switch between networks and try importing manually your NFTs on
networks that are not (Ethereum/linea)
5. Make sure you see your NFTs correctly

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->


https://github.com/user-attachments/assets/81b99e68-3d25-40ca-9c0f-f103b5904fa6



## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

---------

Co-authored-by: MetaMask Bot <[email protected]>
github-merge-queue bot pushed a commit to MetaMask/metamask-extension that referenced this pull request Mar 24, 2025
## **Description**

PR to add network filter to NFT tab.
This core PR MetaMask/core#5448 should be
released before updating this one.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/30903?quickstart=1)

## **Related issues**

Fixes:

## **Manual testing steps**

1. Choose Ethereum mainnet
2. Click on NFTs tab
3. You should see network filter + your detected NFTs on Ethereum
mainnet and linea (if you have any)
4. Switch between networks and try importing manually your NFTs on
networks that are not (Ethereum/linea)
5. Make sure you see your NFTs correctly

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->


https://github.com/user-attachments/assets/81b99e68-3d25-40ca-9c0f-f103b5904fa6



## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

---------

Co-authored-by: MetaMask Bot <[email protected]>
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