refactor: aggregate Tokens namespace from per-chain modules#14
Merged
Conversation
The root `Tokens` namespace used to import each chain's JSON directly and re-run the per-row → symbol-keyed transform. That duplicated the transform the per-chain modules already perform at their own module load: two code paths reading the same source files. After this change, `src/tokens/index.ts` consumes the per-chain modules (`./ethereum`, `./sepolia`, `./base`, `./base-sepolia`, `./bnb-mainnet`) as its source of truth. Per-chain modules stay the canonical typed access for chain-bound consumers (via the existing subpath exports added in 0.6.0); the root `Tokens` becomes a pure flatten over their pre-validated symbol → entry maps. Behavior preserved: - Public surface unchanged: `Tokens.SYMBOL[chainId]`, `lookupToken()`, and the per-chain subpath exports all keep working identically. - The cross-validation test in `tests/per-chain.test.ts` becomes a tautology (same source flowing through both paths), but stays valid as a regression guard if anyone changes the aggregator. - Null-prototype + own-property-check hardening from #12 carried through; the (symbol, chainId) duplicate check is dropped because per-chain modules already guarantee single-symbol-per-chain at their own load — the aggregator's slot is always empty. All 57 tests still pass. Build emits the same six entries and the same five JSON sidecars.
There was a problem hiding this comment.
Pull request overview
Refactors the root Tokens aggregator to use the per-chain token modules as the single source of truth, avoiding duplicate JSON imports and duplicate per-row transformation while preserving the existing public API shape (Tokens.SYMBOL[chainId], lookupToken, and subpath exports).
Changes:
- Switches
src/tokens/index.tsto import./ethereum,./sepolia,./base,./base-sepolia,./bnb-mainnetinstead of importing./data/*.jsondirectly. - Rebuilds the root
Tokensnamespace by flattening the per-chaintokensmaps (symbol → entry) into the cross-chain shape (symbol → chainId → entry). - Removes the redundant
(symbol, chainId)duplicate check previously performed during aggregation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+82
to
86
| for (const [chainId, chainTokens] of PER_CHAIN) { | ||
| for (const [symbol, entry] of Object.entries(chainTokens)) { | ||
| if (!Object.prototype.hasOwnProperty.call(merged, symbol)) { | ||
| merged[symbol] = Object.create(null); | ||
| } |
chrisli30
added a commit
that referenced
this pull request
Jun 9, 2026
…#17) Addresses Copilot review on #14: the tokens-aggregator refactor dropped duplicate protection, so a chain listed twice in PER_CHAIN would silently overwrite the first chain's entries (wrong Tokens.SYMBOL[chainId], no error at module load). Now throws fast pointing at the duplicate. Patch changeset; no API change. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactor — no behavior change, no public surface change.
The root
Tokensnamespace was importing each chain's JSON directly and re-running the per-row → symbol-keyed transform that the per-chain modules (added in v0.6.0) already perform. Two code paths reading the same source files, with a cross-validation test keeping them honest.After this PR,
src/tokens/index.tsconsumes the per-chain modules (./ethereum,./sepolia,./base,./base-sepolia,./bnb-mainnet) as its source of truth. Per-chain modules stay the canonical typed access for chain-bound consumers (via subpath exports); the rootTokensbecomes a pure flatten over their pre-validated symbol → entry maps.What's preserved
Tokens.SYMBOL[chainId],lookupToken(),@avaprotocol/protocols/tokens/<chain>all keep working unchanged. No consumer disruption.tests/per-chain.test.ts(the cross-validation test) becomes a tautology — both paths now read from the same source — but stays valid as a regression guard if anyone changes the aggregator.What's dropped
buildTokensFromData(). Per-chain modules already guarantee single-symbol-per-chain at load viabuildChainTokenMap, so the aggregator's slot is always empty for any (symbol, chainId) pair. Dropping the redundant check is a small simplification../data/*.json— those now live exclusively in the per-chain modules.TokenDataRowinterface — moved to./per-chain.tsand exported from there (no surface change since this PR's tests still reference it through the helper).Diff size
src/tokens/index.ts—+50 / -57. One file.Verification
yarn build— emits the same six entries (root + 5 chain modules) as beforeyarn typecheck— cleanyarn test:run— 57/57 pass (same count, same suite)dist/tokens/{ethereum,sepolia,base,base-sepolia,bnb-mainnet}.jsonrebuild with identical entry counts (104/6/23/4/2 = 139 total)Test plan
Tokens.USDC[Chains.Sepolia]still returns the same address/decimals after the refactor.