-
Notifications
You must be signed in to change notification settings - Fork 48
Replaced fetch with node-fetch #281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Fetch can be unavailable on some versions of node 18, this PR explicitly uses node-fetch for full backwards compatibility.
🦋 Changeset detectedLatest commit: 83b3146 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Fixed tests by mocking node-fetch instead of global fetch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR replaces the usage of the global fetch API with the explicit node-fetch package to ensure backwards compatibility with Node.js 18 versions that may not have the global fetch available.
- Adds
node-fetchand its type definitions as dependencies - Updates test mocking to use
node-fetchinstead of globalfetch - Imports
node-fetchin modules that make HTTP requests
Reviewed Changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/mcp-server/package.json | Adds node-fetch dependency and type definitions |
| packages/mcp-server/src/tests/core/tools.test.ts | Updates test mocking from global fetch to node-fetch |
| packages/mcp-server/src/mintlify/search.ts | Adds node-fetch import for HTTP requests |
| packages/mcp-server/src/docs/server.ts | Adds node-fetch import for HTTP requests |
| .changeset/lovely-sloths-help.md | Documents the change for version tracking |
Comments suppressed due to low confidence (1)
packages/mcp-server/package.json:33
- The node-fetch version ^2.7.0 uses CommonJS modules which may cause compatibility issues with ESM imports. Consider upgrading to node-fetch version 3.x which supports ESM, or ensure proper configuration for CommonJS compatibility.
"node-fetch": "^2.7.0",
kamwithak
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #281 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 70 70
Lines 843 844 +1
Branches 139 139
=========================================
+ Hits 843 844 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fetch can be unavailable on some versions of node 18, this PR explicitly uses node-fetch for full backwards compatibility.