fix(caip): keep Node's fs out of the browser-reachable module graph - #12507
Conversation
adapters/coingecko/index.ts re-exports fetchData and parseData from adapters/coingecko/utils.ts, and utils.ts imported `fs` for writeFiles - a codegen helper only ever called by generate.ts. That single edge put `import fs from 'fs'` in the graph every browser bundler reaches from the package entry, so consuming @shapeshiftoss/caip from a Next.js app fails with "Module not found: Can't resolve 'fs'". Move writeFiles into a codegen-only writeFiles.ts per adapter. The public API is unchanged: fetchCoingeckoData and parseCoingeckoData still export from utils.ts, and writeFiles was never in a barrel. coincap and coinbase get the same treatment - their utils.ts is not re-exported today, so the fs import there is latent rather than live, and this keeps it that way. Compiled generate.js/writeFiles.js still land in dist, like the compiled tests already do, but nothing reachable from index.js imports them.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughCoinbase, CoinCap, and CoinGecko filesystem code generation was separated from data utilities into dedicated ChangesAdapter codegen separation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/caip/src/adapters/coinbase/writeFiles.ts`:
- Around line 8-15: All extracted writer functions need explicit Promise<void>
return types instead of inferred types. Update writeFile and writeFiles in
packages/caip/src/adapters/coinbase/writeFiles.ts, writeFiles and its nested
callback in packages/caip/src/adapters/coincap/writeFiles.ts, and writeFiles and
its nested callback in packages/caip/src/adapters/coingecko/writeFiles.ts; make
no other behavioral changes.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 58880c65-b493-4aa7-90eb-9ef069ee4f56
📒 Files selected for processing (11)
packages/caip/src/adapters/coinbase/generate.tspackages/caip/src/adapters/coinbase/utils.tspackages/caip/src/adapters/coinbase/writeFiles.tspackages/caip/src/adapters/coincap/generate.tspackages/caip/src/adapters/coincap/utils.test.tspackages/caip/src/adapters/coincap/utils.tspackages/caip/src/adapters/coincap/writeFiles.tspackages/caip/src/adapters/coingecko/generate.tspackages/caip/src/adapters/coingecko/utils.test.tspackages/caip/src/adapters/coingecko/utils.tspackages/caip/src/adapters/coingecko/writeFiles.ts
💤 Files with no reviewable changes (2)
- packages/caip/src/adapters/coincap/utils.ts
- packages/caip/src/adapters/coinbase/utils.ts
…dules Per .cursor/rules/typescript-best-practices.mdc, which lists missing return types as an anti-pattern. The moved code carried no annotations, so these are new on the extracted files.
Description
@shapeshiftoss/caipcannot be consumed from a browser bundler today. In a Next.js app it fails with:The cause is a single edge.
adapters/coingecko/index.ts:56re-exports from./utils:and
utils.tsimportedfsforwriteFiles— a codegen helper whose only caller isgenerate.ts. That one re-export putsimport fs from 'fs'into the graph every browser bundler reaches from the package entry.I walked the published ESM graph from
dist/esm/index.jsto check the blast radius. 82 modules are reachable, and exactly one of them imports a Node built-in:This PR moves
writeFilesinto a codegen-onlywriteFiles.tsin each adapter directory.generate.tsimports it from the new path.The coincap and coinbase
utils.tsfiles importfsfor the same reason, but nothing re-exports them, so theirs is latent rather than live. They get the same treatment here so that re-exporting from them later can't reintroduce the bug.Worth noting why this is invisible in this repo:
packages/swap-widget/devDependenciesincludesvite-plugin-node-polyfills, so the dev server and demo build polyfillfssilently. Consumers who don't polyfill Node built-ins — the default for Next.js, and the right posture for browser bundles — get a hard failure.No public API change.
fetchCoingeckoDataandparseCoingeckoDatastill export fromutils.ts, andwriteFileswas never exposed in any barrel. The only non-mechanical edit is making theAssetMaptype exported sowriteFiles.tscan import it as a type.One thing left deliberately undone: the compiled
generate.jsandwriteFiles.jsstill land indist, exactly as the compiled.test.jsfiles already do. They're just unreachable fromindex.js, which is what bundlers care about. If you'd rather keep them out of the tarball entirely, excludingsrc/**/generate.tsandsrc/**/writeFiles.tsfromtsconfig.{esm,cjs}.jsonwould do it — at the cost of no longer type-checking the codegen, which is why I didn't assume it.Issue (if applicable)
None — reported from an external integration.
Risk
Low, and contained to
packages/caip. Pure code movement: no logic changes, no public export changes, no dependency or lockfile changes.writeFilesbodies are moved verbatim.The failure mode if something were wrong is loud and immediate —
pnpm generate:coingecko/generate:coincapwould fail to resolvewriteFiles, and the two moved test suites cover its behaviour.None. The moved code is build-time asset-map codegen and is never reachable at runtime.
Testing
Engineering
vi.mock('fs')in those suites is module-scoped, so it still intercepts the import from its new location.Verified on my side: no Node built-in imports remain anywhere in
packages/caip/srcoutsidegenerate.tsand the newwriteFiles.tsfiles;writeFilesappears in no barrel; and all touched files pass Prettier 3.0.3 with the options from.eslintrc.I could not run ESLint,
tsc, or Vitest locally — that needs a full monorepo install and I only had a shallow clone — so CI is the real check on those.Operations
No user-facing change. Regression surface is the CoinGecko/CoinCap/Coinbase asset-id adapter maps, which the codegen commands above regenerate byte-identically.
Screenshots (if applicable)
n/a
Made with Cursor
Summary by CodeRabbit