perf(deps): drop node-fetch in favour of the platform fetch - #116
Merged
Conversation
|
Total Coverage: 87.21% Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
node-fetch was the single largest entry in the bundle at 302 KB minified, and pulling it in also dragged along whatwg-url, form-data and mime-db. Every runtime this package supports already ships a global fetch: it is stable in Node from v18 and available in every browser target, so the dependency bought nothing. Bundling `dist/index.js` with esbuild (minified, esm): before 1,685,983 bytes (438,782 gzipped) after 1,333,533 bytes (345,961 gzipped) delta -352,450 bytes (-20.9%), -92,821 gzipped (-21.2%) The saving exceeds node-fetch's own 302 KB because form-data and mime-db leave with it. Alongside the swap: - Add a 30s AbortSignal timeout to requests. `promiseRetry` only advances when the underlying promise settles, so a request that never settled previously pinned the whole retry chain open indefinitely. - Remove the dead FetchError branch in MetaNamesContractRepository. It could never run for two independent reasons: the guarded call was returned without `await`, so rejections escaped the try/catch entirely, and node-fetch sets FetchError#code for system errors such as ENOTFOUND rather than HTTP statuses, so `code === '404'` never held. A missing AVL value is already reported as `undefined` by `getRequest`, which resolves a body only on HTTP 200. The call is now awaited and transport failures are logged via console.error. - Drop jest-environment-jsdom. main already runs the suite on the node environment; jsdom does not implement fetch either, so the dependency is now dead weight and its removal is what the environment switch was for. Rebased onto main after #115. Full suite is 255/255 passing; test wall time for the lookup suite dropped from 40s (timing out) to 1.2s. Requires Node >= 20, already declared in engines. Claude-Session: https://claude.ai/code/session_01GceWCwGXu66D1xEBDxZWBb
yeboster
force-pushed
the
perf/native-fetch
branch
from
August 28, 2026 19:35
de6e921 to
155e9f6
Compare
|
Total Coverage: 87.12% Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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
1 participant
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
node-fetchwas the single largest entry in the bundle at 302 KB minified — 18% of the total — and it draggedwhatwg-url,form-dataandmime-dbin with it. Every runtime this package supports already ships a globalfetch: stable in Node since v18, and present in every browser target. The dependency bought nothing.Bundle impact
Measured with esbuild (
--bundle --minify --format=esm) overdist/index.js:The saving exceeds node-fetch's own 302 KB because
form-dataandmime-dbleave with it.Changes beyond the swap
Request timeouts. Added a 30s
AbortSignaltimeout.promiseRetryonly advances when the underlying promise settles, so a request that never settled previously pinned the entire retry chain open indefinitely — an unbounded resource hold with no way to cancel.Removed dead error handling in
MetaNamesContractRepository.getStateAvlValue. TheFetchErrorbranch could never execute, for two independent reasons:returned withoutawait, so the rejection escaped thetryentirely.FetchError#codefor system errors (ENOTFOUND,ECONNRESET), never HTTP statuses — socode === '404'never held even if it had been reachable.A missing AVL value is already reported as
undefinedbygetRequest, which resolves a body only on HTTP 200. The call is now awaited and genuine transport failures go toconsole.error.Test environment jsdom → node, dropping
jest-environment-jsdom. jsdom does not implementfetch, so it would have failed every network test after this change. No test in the suite touches a DOM API — jsdom was masking the fact that the package depended on a non-browserfetch.Verification
npx tsc --noEmit→ clean.npx jest -i→ 255/255 passing, all 20 suites green.Worth noting: on
mainthe live-testnet integration suites fail intermittently (two different suites failed on consecutive runs of unmodifiedmain). They now pass consistently. Thedomain-lookupsuite went from timing out at 40s to passing in 1.2s.Compatibility
Requires Node ≥ 20 (
engines.nodedeclares this). No public API change —getRequestgains an optionaltimeoutMsparameter with a default.https://claude.ai/code/session_01GceWCwGXu66D1xEBDxZWBb