Skip to content
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

fix(typescript): Remove node-fetch in TypeScript SDK #5476

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

remorses
Copy link

@remorses remorses commented Dec 24, 2024

Description

Removes node-fetch, Node.js 18 already comes with fetch global, Node 18 is the oldest version currently supported.

This change makes it possible to bundle the generated sdk for browser with bundlers like Vite and Rollup without marking node-fetch external

Fix #5468

Changes Made

Remove node-fetch

Testing

  • Unit tests added/updated
  • Manual testing completed

@remorses remorses changed the title Remove node-fetch in TypeScript SDK Fix: Remove node-fetch in TypeScript SDK Dec 24, 2024
@remorses remorses changed the title Fix: Remove node-fetch in TypeScript SDK Fix(typescript): Remove node-fetch in TypeScript SDK Dec 24, 2024
@remorses remorses changed the title Fix(typescript): Remove node-fetch in TypeScript SDK fix(typescript): Remove node-fetch in TypeScript SDK Dec 24, 2024
@remorses remorses marked this pull request as ready for review December 24, 2024 19:33
@remorses remorses requested a review from dsinghvi as a code owner December 24, 2024 19:33
@dsinghvi
Copy link
Member

@remorses this change will require some discussion on our end because there may be breaking changes related to file download and upload endpoints (where the current generated SDKs leverage node-fetch).

cc @Swimburger

@remorses
Copy link
Author

The global fetch should already be used by default if available, this means that features like upload already work with it

@@ -48,7 +47,6 @@ describe("Multipart Form Data Tests", () => {

await fdw.appendFile("file", new Blob([new Uint8Array(y)]), "asda");

const fetch = await getFetchFn();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main change

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

Successfully merging this pull request may close these issues.

Remove node-fetch and other dependencies from typescript sdk
4 participants