fix(common): stop a chain viem removes from breaking every @tevm/common import - #2098
fix(common): stop a chain viem removes from breaking every @tevm/common import#2098valerioleo wants to merge 2 commits into
Conversation
…on import Fixes evmts#2097. Each generated preset does `import { <chain> } from 'viem/chains'` — a named import — so when viem retires a chain, `import '@tevm/common'` becomes an uncatchable SyntaxError at ESM link time for every consumer, whether or not they use that chain: SyntaxError: The requested module 'viem/chains' does not provide an export named 'ekta' That shipped. `@tevm/common@1.0.0-next.148` still had presets for `ekta`, `ektaTestnet`, `seiDevnet` and `zircuitTestnet` after viem removed them, so the published package cannot be imported on any viem >= 2.46.1. And because `viem` is a peerDependency, a consumer cannot resolve around it — tevm necessarily uses their viem. `main` has already dropped those four and is clean against viem 2.55.13, so this changes no generated output. It addresses why it happened and stops the next one: **A guard test.** `presets.spec.ts` asserts no preset imports a chain viem no longer exports. It reads the preset files as text rather than importing them, so it does not pull in `createCommon` and its native `@evmts/zevm` dependency — the check runs in 210ms with no Zig build, which matters for something meant to run on every CI job. It also checks each preset imports the chain matching its filename, and that index.ts and the files on disk agree. Restoring the published `ekta.js` fails it with `ekta.js imports 'ekta'`. **A generator that self-heals.** `allChains` is hand-maintained, so a retired chain became `undefined` and the template read `.name` off it — regenerating crashed with `TypeError: Cannot read properties of undefined` and left the stale preset in place. Retired names are now reported and their generated files deleted, so a viem bump plus a regenerate is enough. Verified the generator is idempotent on main: running it reproduces all 330 presets byte-for-byte, so the only diff here is this file. One thing I did not do, and would be glad to if you want it: `allChains` could be derived from `Object.entries(chains)` rather than hand-listed, which would remove the maintenance entirely. It would also add 398 presets (viem exports 727 chains today against the 329 listed), so that seemed like your call rather than mine.
🦋 Changeset detectedLatest commit: 6b2d7c5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@valerioleo is attempting to deploy a commit to the evmts Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe preset generator now detects retired Viem chains, removes stale preset files, and excludes them from generated exports. New tests validate preset imports, filenames, and index consistency against current Viem exports. ChangesChain preset guards
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/common/src/presets/__GENERATE_CHAIN_PRESETS__.js (1)
363-382: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winComplete the
partitionChainsJSDoc.Lines 363-375 explain the motivation but omit the return contract and a complete working example. Add
@returnsforpresentandretired. Add a runnable@examplewith required imports. Document@throwsif this helper exposes a supported failure path.As per coding guidelines, "
**/*.js: ... We always include complete jsdoc information including@throws@exampleetc."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/common/src/presets/__GENERATE_CHAIN_PRESETS__.js` around lines 363 - 382, Complete the JSDoc for partitionChains by documenting that it returns an object containing present and retired chain-name mappings, and add a runnable `@example` with all required imports and representative output or usage. Review the implementation for any supported failure path and document it with `@throws` if applicable; otherwise do not invent one.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/common/src/presets/presets.spec.ts`:
- Around line 75-79: Update the test around the regex in the “exports every
generated preset from index.ts” case to retain both the exported symbol and
referenced filename, then assert each export name matches its target filename.
Continue comparing the collected target filenames against presetFiles so swapped
export paths are detected.
---
Nitpick comments:
In `@packages/common/src/presets/__GENERATE_CHAIN_PRESETS__.js`:
- Around line 363-382: Complete the JSDoc for partitionChains by documenting
that it returns an object containing present and retired chain-name mappings,
and add a runnable `@example` with all required imports and representative output
or usage. Review the implementation for any supported failure path and document
it with `@throws` if applicable; otherwise do not invent one.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3dfad00c-24aa-4a79-b80a-88d5ce9408cc
📒 Files selected for processing (3)
.changeset/guard-removed-viem-chains.mdpackages/common/src/presets/__GENERATE_CHAIN_PRESETS__.jspackages/common/src/presets/presets.spec.ts
| it('exports every generated preset from index.ts, and nothing that has no file', () => { | ||
| const index = readFileSync(join(here, 'index.ts'), 'utf8') | ||
| const exported = [...index.matchAll(/export \{ (\w+) \} from '\.\/(\w+)\.js'/g)].map((match) => match[1]) | ||
|
|
||
| expect([...exported].sort()).toEqual([...presetFiles].sort()) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Validate each index.ts export target.
Line 77 captures the referenced filename but discards it. If two export paths are swapped, Line 79 still passes while index.ts exports each preset from the wrong file. Keep both captures. Assert that each export name equals its target filename. Compare target filenames with presetFiles.
Proposed test update
- const exported = [...index.matchAll(/export \{ (\w+) \} from '\.\/(\w+)\.js'/g)].map((match) => match[1])
+ const exported = [...index.matchAll(/export \{ (\w+) \} from '\.\/(\w+)\.js'/g)].map(([, name, file]) => ({
+ name,
+ file,
+ }))
+
+ expect(exported.filter(({ name, file }) => name !== file)).toEqual([])
- expect([...exported].sort()).toEqual([...presetFiles].sort())
+ expect(exported.map(({ file }) => file).sort()).toEqual([...presetFiles].sort())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('exports every generated preset from index.ts, and nothing that has no file', () => { | |
| const index = readFileSync(join(here, 'index.ts'), 'utf8') | |
| const exported = [...index.matchAll(/export \{ (\w+) \} from '\.\/(\w+)\.js'/g)].map((match) => match[1]) | |
| expect([...exported].sort()).toEqual([...presetFiles].sort()) | |
| it('exports every generated preset from index.ts, and nothing that has no file', () => { | |
| const index = readFileSync(join(here, 'index.ts'), 'utf8') | |
| const exported = [...index.matchAll(/export \{ (\w+) \} from '\.\/(\w+)\.js'/g)].map(([, name, file]) => ({ | |
| name, | |
| file, | |
| })) | |
| expect(exported.filter(({ name, file }) => name !== file)).toEqual([]) | |
| expect(exported.map(({ file }) => file).sort()).toEqual([...presetFiles].sort()) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/common/src/presets/presets.spec.ts` around lines 75 - 79, Update the
test around the regex in the “exports every generated preset from index.ts” case
to retain both the exported symbol and referenced filename, then assert each
export name matches its target filename. Continue comparing the collected target
filenames against presetFiles so swapped export paths are detected.
|
Closing this — on a closer look it does not contain a fix, and I would rather not add a test harness to a package that is mid-release-prep.
So there was nothing to fix here. What this PR actually added was a regression guard plus some generator resilience, and that is not what an unreleased package needs from a drive-by contributor. The real blocker is a publish gap rather than a code defect, and I have rewritten #2097 to say that instead. Sorry for the noise, and thanks for the pointer to where the presets are generated — that made the diagnosis quick. |
1 participant
Fixes #2097.
Description
Each generated preset imports its chain by name from
viem/chains, so when viem retires a chain,import '@tevm/common'becomes an uncatchableSyntaxErrorat ESM link time for every consumer — whether or not they use that chain:That shipped.
@tevm/common@1.0.0-next.148still had presets forekta,ektaTestnet,seiDevnetandzircuitTestnetafter viem removed them, so the published package cannot be imported on any viem ≥ 2.46.1. Becauseviemis apeerDependency, a consumer cannot resolve around it either — tevm necessarily uses their viem, and both pnpm nested-override forms leave@tevm/commonon the top-level copy.mainis already clean — those four presets are gone and nothing here changes generated output. This PR is about why it happened and stopping the next one.1. A guard test —
presets.spec.tsAsserts no preset imports a chain viem no longer exports.
It reads the preset files as text rather than importing them. That is deliberate: importing them pulls in
createCommonand its native@evmts/zevmdependency, which would make this check depend on a Zig build to tell you whether an import statement names something that exists. Static, it runs in 210 ms with no build — appropriate for something meant to run on every CI job.It also checks each preset imports the chain matching its own filename (a mismatch would silently produce a
Commondescribing the wrong network), and thatindex.tsand the files on disk agree.2. A generator that self-heals
allChainsis hand-maintained, so a retired chain becameundefinedand the template read.nameoff it — regenerating crashed withTypeError: Cannot read properties of undefinedand left the stale preset in place. Retired names are now reported and their generated files deleted, so a viem bump plus a regenerate is enough:Testing
ekta.jsgivesAssertionError: expected [ "ekta.js imports 'ekta'" ] to deeply equal [].main— 3 tests, 210 ms, against viem 2.55.13 (all 329 presets resolve).mainreproduces all 330 preset files byte-for-byte, so the only diff in this PR is the generator itself plus the new spec.index.tsinstead of throwing.biome checkandbiome formatclean on both changed files.Local
pnpm test:runfor the whole package needs@evmts/zevmbuilt, which I did not attempt — the new spec is written to not require it, and I verified it runs standalone.Additional Information
@tevm/common, patch).One thing I did not do, and would be glad to if you want it:
allChainscould be derived fromObject.entries(chains)instead of hand-listed, removing the maintenance entirely. It would also add 398 presets — viem exports 727 chains today against the 329 listed — so the scope felt like your call rather than mine.Context
Found while trying to move deployoor off viem 2.46.0. It uses tevm for node-free EVM tests and ships
@deployoor/testingwith tevm as a runtime dependency andviemas a peer, so its users hit this on import. Full write-up in #2097.Summary by CodeRabbit
Bug Fixes
Tests