fix(typescript): Remove node-fetch in TypeScript SDK #5476
Conversation
This file contains 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
remorses
changed the title
remorses
changed the title
remorses
changed the title
|
@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 cc @Swimburger |
|
The global fetch should already be used by default if available, this means that features like upload already work with it |
remorses
commented
Dec 27, 2024
| @@ -48,7 +47,6 @@ describe("Multipart Form Data Tests", () => { | |||
|
|
|||
| await fdw.appendFile("file", new Blob([new Uint8Array(y)]), "asda"); | |||
|
|
|||
| const fetch = await getFetchFn(); | |||
Swimburger
requested changes
Dec 27, 2024
| @@ -19,7 +19,7 @@ export interface CrossPlatformFormData { | |||
|
|
|||
| appendFile(key: string, value: unknown, fileName?: string): Promise<void>; | |||
|
|
|||
| getRequest(): MaybePromise<FormDataRequest<unknown>>; | |||
There was a problem hiding this comment.
We usually use unknown instead of any. Any reason why this was changed here?
There was a problem hiding this comment.
yes, now fetch type is more strict and no longer accepts a RequestInit<unknown> so many type checks in tests for example fail
That unknown has no reason to exist because there is no way to override it with a generic
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
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