Skip to content

Fix code quality issues in dynamic token implementation#1046

Merged
ebma merged 8 commits intostagingfrom
copilot/sub-pr-1014
Jan 30, 2026
Merged

Fix code quality issues in dynamic token implementation#1046
ebma merged 8 commits intostagingfrom
copilot/sub-pr-1014

Conversation

Copy link
Contributor

Copilot AI commented Jan 28, 2026

Addresses code quality feedback from PR #1014: import ordering violations, unused dependencies, type inconsistencies, and non-deterministic token deduplication. This PR has been rebased on the latest staging branch.

Import & Dependency Cleanup

  • Moved imports before code declarations in useOnchainTokenBalances.ts (ESM requirement)
  • Removed unused imports: useReadContracts, erc20ABI, Abi, EvmToken
  • Removed react-window dependency (replaced by @tanstack/react-virtual)

Type Consistency

  • Made balanceUsd required in EvmTokenDetailsWithBalance to match OnChainTokenDetailsWithBalance
  • Eliminates type casting needed when treating EVM tokens as generic on-chain tokens

Token Deduplication Logic

When multiple tokens share the same symbol on a network, the previous implementation silently overwrote entries. Now uses deterministic prioritization integrated with staging's refactored token management structure:

// Priority: native > higher USD price > lower address (lexicographic)
const shouldReplace =
  (token.isNative && !existingToken.isNative) ||
  (!token.isNative &&
    !existingToken.isNative &&
    (tokenPrice > existingPrice ||
      (tokenPrice === existingPrice &&
        token.erc20AddressSourceChain.toLowerCase() < existingToken.erc20AddressSourceChain.toLowerCase())));

The deduplication logic has been integrated into staging's refactored groupTokensByNetwork function, which now works alongside the new mergeWithStaticConfig and buildPriceLookup functions introduced in the staging branch.

Sort Comparator Fix

Fixed useOnchainTokenBalancesSorted to return 0 for equal elements instead of 1, preventing unstable/incorrect ordering:

// Tie-breaker: sort by symbol alphabetically
return a.assetSymbol.localeCompare(b.assetSymbol);

Logging & Configuration

  • Replaced console.log with logger.debug in squidrouter.ts
  • Fixed Prettier ignore path and added missing prettier-ignore-end directive

Merge with Staging

This PR includes all changes from the staging branch (96 files changed, 4071 insertions, 779 deletions), including:

  • Token management refactoring with separation of concerns
  • New animation components and stories
  • Phase handler improvements
  • Various bug fixes and enhancements

All code quality fixes have been preserved and integrated with the staging branch's refactored codebase.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI mentioned this pull request Jan 28, 2026
@netlify
Copy link

netlify bot commented Jan 28, 2026

Deploy Preview for vortex-sandbox ready!

Name Link
🔨 Latest commit 2839e36
🔍 Latest deploy log https://app.netlify.com/projects/vortex-sandbox/deploys/697d053c81ca210008cefaf7
😎 Deploy Preview https://deploy-preview-1046--vortex-sandbox.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copilot AI and others added 2 commits January 28, 2026 10:49
…eedback

Co-authored-by: ebma <6690623+ebma@users.noreply.github.com>
Co-authored-by: ebma <6690623+ebma@users.noreply.github.com>
Co-authored-by: ebma <6690623+ebma@users.noreply.github.com>
Copilot AI changed the title [WIP] Add support for dynamic on-chain tokens Fix code quality issues in dynamic token implementation Jan 28, 2026
Copilot AI requested a review from ebma January 28, 2026 10:55
Base automatically changed from dynamic-onchain-tokens to staging January 28, 2026 12:04
@ebma
Copy link
Member

ebma commented Jan 30, 2026

@copilot please bring this pull request up-to-date with the latest changes from staging branch and resolve the merge conflicts so that it makes sense.

- Resolved conflicts in dynamicEvmTokens.ts by integrating deduplication logic with staging's refactored structure
- Kept staging's separation of concerns (groupTokensByNetwork, mergeWithStaticConfig, buildPriceLookup)
- Added smart token deduplication in groupTokensByNetwork with priority: native > higher USD price > lower address
- Resolved conflict in index.ts by keeping staging's export order with prettier-ignore-end directive
- All other changes from staging branch merged successfully

Co-authored-by: ebma <6690623+ebma@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 30, 2026

@copilot please bring this pull request up-to-date with the latest changes from staging branch and resolve the merge conflicts so that it makes sense.

Merged staging branch successfully in commit f1d728c.

Resolved conflicts in:

  • packages/shared/src/tokens/evm/dynamicEvmTokens.ts - Integrated my deduplication logic with staging's refactored structure
  • packages/shared/src/tokens/index.ts - Kept staging's export order with prettier-ignore-end directive

The PR now includes all latest changes from staging while preserving the code quality improvements from the review feedback.

@netlify
Copy link

netlify bot commented Jan 30, 2026

Deploy Preview for vortexfi ready!

Name Link
🔨 Latest commit 2839e36
🔍 Latest deploy log https://app.netlify.com/projects/vortexfi/deploys/697d053c7e1962000831f075
😎 Deploy Preview https://deploy-preview-1046--vortexfi.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@ebma ebma marked this pull request as ready for review January 30, 2026 19:24
@ebma ebma merged commit 56485b6 into staging Jan 30, 2026
5 checks passed
@ebma ebma deleted the copilot/sub-pr-1014 branch January 30, 2026 19:36
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