Repository navigation
feat: accept the native fetch directly as the HTTPProvider fetch option - #583
Conversation
FetchFunction declared `mode?: string`, which made the global native fetch non-assignable to it (native `RequestInit` types `mode` as `RequestMode`, and `string` is not assignable to `RequestMode`), forcing callers to cast with `fetch as unknown as FetchFunction`. HTTPProvider never sets `mode`, so dropping it lets the global native fetch — the same WHATWG signature on web and server — as well as native-fetch components (e.g. @dcl/fetch-component) and node-fetch be passed to `options.fetch` with no cast. Type-only change: the emitted JS is unchanged, and existing callers (including those still passing node-fetch) keep compiling. Adds compile-time assertions and regenerates the API report and docs.
Test this pull request
|
decentraland-bot
left a comment
There was a problem hiding this comment.
Review — feat: accept the native fetch directly as the HTTPProvider fetch option
Summary
Clean, well-motivated type-only change. Removes the unused mode?: string field from the exported FetchFunction type so that globalThis.fetch, @dcl/fetch-component, and node-fetch are directly assignable without casts.
Analysis
Type safety: The params object type is in contravariant position (parameter of the callback). Removing an optional field that HTTPProvider never sets makes the callback type strictly wider — more functions satisfy it, none are excluded. This is not a breaking change for any consumer that passes a fetch implementation.
Consumer impact: Searched across the org:
decentraland/lamb2— importsFetchFunctionand currently casts withas unknown as FetchFunction, with a comment atsrc/components.ts:141-143explaining exactly this type mismatch. After this publishes, lamb2 can drop the cast. ✅decentraland/governance/governance-ui— have unrelated localfetchFunctiondefinitions, not importing from eth-connect. No impact.- No consumers found that reference
modein the params object.
Runtime impact: None. The emitted JS is identical — mode was never set by HTTPProvider (it only sets body, method, headers).
Tests: Good approach — compile-time assertions that globalThis.fetch and a @dcl/fetch-component-shaped function are assignable without casts. Existing tests unaffected. CI passes.
Docs & API report: Updated consistently with the type change.
Git conventions (ADR-6): PR title (feat: ...) and branch (feat/fetchfunction-accept-native-fetch) both comply.
Security: No security issues found. Type-only change with no runtime impact.
Verdict
No P0 or P1 findings. LGTM — approve.
Reviewed by Jarvis 🤖 · Requested by Lautaro Petaccio via Slack
eth-connect 6.4.0 (decentraland/eth-connect#583) makes FetchFunction accept the native fetch directly, so the `fetch.fetch as unknown as FetchFunction` cast in the ws-connector HTTPProvider wiring is no longer needed. Bumped eth-connect in core and ws-connector, removed the cast and the now-unused FetchFunction import. Type-only change; no runtime behavior change.
) * chore: migrate from @well-known-components to @dcl core-components Replace the @well-known-components homonyms across the core, ws-connector and stats workspaces with the published @dcl/* core-components, and bump the existing @dcl dependencies to their latest versions. Component swaps: - http-server: @well-known-components/http-server -> @dcl/http-server@2 (core, stats) - metrics: @well-known-components/metrics -> @dcl/metrics (core, ws-connector, stats) - fetch: @well-known-components/fetch-component -> @dcl/fetch-component (ws-connector, stats) - test-helpers: @well-known-components/test-helpers -> @dcl/test-helpers (all workspaces) - bump: @dcl/uws-http-server -> ^1.0.1 (ws-connector) - add: @dcl/core-commons, @types/jest (the new @dcl/test-helpers declares @types/jest as a peer dependency instead of bundling it) Adaptations: - Move IHttpServerComponent / IFetchComponent to @dcl/core-commons in the component type maps so they match the native (global Request/Response) server and fetch returned by @dcl/http-server v2 and @dcl/fetch-component. The other interface types (IConfigComponent, ILoggerComponent, IBaseComponent, IMetricsComponent) stay on @well-known-components/interfaces. - eth-connect's HTTPProvider and dcl-catalyst-client still type their fetch against node-fetch's IFetchComponent; the native fetch is runtime-compatible, so cast it at those call sites. - Fix the createLocalFetchCompoment -> createLocalFetchComponent typo exposed by @dcl/test-helpers. Stays on @well-known-components (no core equivalent): interfaces, logger, env-config-provider, nats-component, pushable-channel. * fix: pin semver to v7 so jest coverage reporter resolves semver/functions/gte * chore: bump dcl-catalyst-client to v22 in stats to drop node-fetch v22 replaced its cross-fetch/wkc-fetch deps (which pulled node-fetch@2) with @dcl/fetch-component (native fetch) and accepts the native fetcher directly, so the as-unknown-as IFetchComponent cast and the node-fetch comment in stats/src/adapters/content.ts are no longer needed. * chore: bump eth-connect to 6.3.1 for the HTTPProvider response-body fix eth-connect 6.3.1 releases the response body on non-2xx HTTPProvider responses (decentraland/eth-connect#582), which the ws-connector relies on for the on-chain signature validation it runs on every WS authentication. Bumped in both core and ws-connector; both resolve to 6.3.1 (the remaining root 6.2.4 is @dcl/crypto's internal copy). The FetchFunction type is unchanged, so the existing native-fetch cast stays. * chore: bump eth-connect to 6.4.0 and drop the native-fetch cast eth-connect 6.4.0 (decentraland/eth-connect#583) makes FetchFunction accept the native fetch directly, so the `fetch.fetch as unknown as FetchFunction` cast in the ws-connector HTTPProvider wiring is no longer needed. Bumped eth-connect in core and ws-connector, removed the cast and the now-unused FetchFunction import. Type-only change; no runtime behavior change.
Problem
HTTPProvider'sFetchFunctionoption typed its params withmode?: string:The global native
fetchtypes its init asRequestInit, whosemodeisRequestMode(a string union). Sincestringis not assignable toRequestMode, the native fetch is not assignable toFetchFunction, so every caller has to cast:Fix
HTTPProvidernever setsmode(it only sendsbody/method/headers), so the field is dead weight. Dropping it makes the option accept, with no cast:fetch— the same WHATWG signature on web and server;@dcl/fetch-component'sfetch);This is a type-only change — the emitted JS is identical, so there is no runtime/behavior change.
Tests
Added compile-time assertions in
test/HTTPProvider.spec.tsthatglobalThis.fetchand a native-fetch-component-shaped function are assignable to the option without a cast. The existingnode-fetchintegration spec still typechecks.Verification
tscsrc + full test project (incl. the node-fetch integration spec): cleanmake build: succeeds; API report regenerated (idempotent) and committed; docs updatedFollow-up
Once published, consumers can drop their
as unknown as FetchFunctioncasts (e.g. archipelago-workers'ws-connector).🤖 Generated with Claude Code