Skip to content

fix(explicabl): drop undeclared node-fetch import; use global fetch#37

Open
davidcrowe wants to merge 1 commit into
mainfrom
fix/explicabl-node-fetch
Open

fix(explicabl): drop undeclared node-fetch import; use global fetch#37
davidcrowe wants to merge 1 commit into
mainfrom
fix/explicabl-node-fetch

Conversation

@davidcrowe

Copy link
Copy Markdown
Collaborator

Problem

@gatewaystack/[email protected] is broken on fresh install — the same defect class as the proxyabl-core/jose break in #35.

src/health.ts:2 does import fetch from "node-fetch", but the package declares only express and express-rate-limit. Since dist/index.js re-exports health.js, every import of the package fails:

npm install @gatewaystack/explicabl
node -e "import('@gatewaystack/explicabl')"
# ERR_MODULE_NOT_FOUND: Cannot find package 'node-fetch'

This hits users who only want createConsoleLogger or explicablLoggingMiddleware and never touch the health route. It also contradicts the README's "Zero dependencies beyond Express" (line 20).

Fix

The two call sites (health.ts:73, :86) use only standard WHATWG fetch (.ok, .status, {method, headers, body}) — a drop-in for Node 18+'s global fetch. So: delete the import, add engines.node >=18 to make the requirement explicit, bump to 0.0.9. Declaring node-fetch would also fix the crash, but it's dead weight on modern Node and would keep the README claim false.

Verification

  • Packed the fixed tarball, installed into an empty project, imported → succeeds with all 5 exports (auth0LogsWebhook, createConsoleLogger, explicablLoggingMiddleware, explicablRouter, healthRoutes)
  • Full suite: 17 files / 159 tests green
  • Published 0.0.8 independently reproduced as failing from the registry

Needs a republish after merge (0.0.9).

…l fetch

health.ts imported `node-fetch` but the package declared only express and
express-rate-limit. Because dist/index.js re-exports health.js, the published
@gatewaystack/[email protected] threw ERR_MODULE_NOT_FOUND on ANY import from a
fresh install — including createConsoleLogger, which has nothing to do with the
health route. Same defect class as the proxyabl-core/jose break (PR #35).

The two call sites use only standard WHATWG fetch semantics (.ok/.status,
{method,headers,body}), so Node 18+'s global fetch is a drop-in. Removing the
import also makes the README's 'zero dependencies beyond Express' claim true.
Add engines.node >=18 to make the global-fetch requirement explicit; bump to
0.0.9. Verified: packed tarball installs into an empty project and imports with
all 5 exports; full suite (159 tests) green.
@davidcrowe

Copy link
Copy Markdown
Collaborator Author

Reviewed + confirmed green (build + test SUCCESS, MERGEABLE/CLEAN). Drops the undeclared node-fetch import for global fetch (Node 18+, backed by the new engines: node>=18) — correct, fixes the fresh-install break.

@davidcrowe — needs your merge + republish (OTP): [email protected].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant