Skip to content

refactor(dev-middleware): drop node-fetch in favor of Node built-in fetch - #45227

Closed
byCedric wants to merge 1 commit into
react:mainfrom
byCedric:@bycedric/dev-middleware/drop-node-fetch
Closed

refactor(dev-middleware): drop node-fetch in favor of Node built-in fetch#45227
byCedric wants to merge 1 commit into
react:mainfrom
byCedric:@bycedric/dev-middleware/drop-node-fetch

Conversation

@byCedric

Copy link
Copy Markdown
Contributor

Summary:

Node 22 doesn't work well with node-fetch@2, as one of their polyfills is using the deprecated punycode module. This causes unnecessary warnings like:

image

Instead of upgrading to the much larger node-fetch@3, this change drops node-fetch in favor of Node's own built-in fetch implementation (using undici).

Note, @react-native/dev-middlewarealready has the engines.node >= 18 (which is required for fetch).

Changelog:

[GENERAL] [CHANGED] - Drop node-fetch in favor of Node's built-in fetch from undici in @react-native/dev-middleware

Test Plan:

See CI for passing tests

@facebook-github-botfacebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 30, 2024
@byCedricbyCedric changed the title refactor(dev-middleware): drop node-fetch in favor of Node fetchrefactor(dev-middleware): drop node-fetch in favor of Node built-in fetchJun 30, 2024
@facebook-github-botfacebook-github-bot added p: Expo Partner: Expo Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jun 30, 2024
@byCedric

Copy link
Copy Markdown
ContributorAuthor

Why would it fail on globalThis, it's the spec-guaranteed global 🙈

@analysis-bot

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a20,366,100-4
androidhermesarmeabi-v7an/a--
androidhermesx86n/a--
androidhermesx86_64n/a--
androidjscarm64-v8a23,562,903-5
androidjscarmeabi-v7an/a--
androidjscx86n/a--
androidjscx86_64n/a--

Base commit: c7988c9
Branch: main

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@robhogan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@NickGerleman

Copy link
Copy Markdown
Contributor

@byCedric if we use this with Node 18, or Node 20, do we get experimental API warnings?

@byCedric

byCedric commented Jul 3, 2024

Copy link
Copy Markdown
ContributorAuthor

@byCedric if we use this with Node 18, or Node 20, do we get experimental API warnings?

@NickGerleman No, there should not be any experimental warnings around fetch in the Node 18 or 20 (except versions older than Node 18.13.0 - released on Jan 6th, 2023):

Node 18.0.0Node 18.20.3Node 20.0.0
node-18.0.0node-18.20.3node-20.0.0

That warning was removed in Node 18.13.0, through PR 45287

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@robhogan merged this pull request in 30a3e6e.

@facebook-github-botfacebook-github-bot added the Merged This PR has been merged. label Jul 3, 2024
@github-actions

Copy link
Copy Markdown

This pull request was successfully merged by @byCedric in 30a3e6e.

When will my fix make it into a release? | How to file a pick request?

@byCedric
byCedric deleted the @bycedric/dev-middleware/drop-node-fetch branch July 3, 2024 16:38
facebook-github-bot pushed a commit that referenced this pull request Nov 27, 2024
Summary:
Removed `node-fetch` in favour of node builtin fetch to get rid of the deprecated `punycode` warning when using Node 22.
`react-native/community-cli-plugin` already requires Node >= 18 where it was made available by default (without `--experimental-fetch` flag).
This change is similar to the one made in #45227
## Changelog:
[GENERAL] [CHANGED] - Drop node-fetch in favor of Node's built-in fetch from undici in `react-native/community-cli-plugin`
Pull Request resolved: #47397
Test Plan: tests pass
Reviewed By: blakef
Differential Revision: D66512595
Pulled By: NickGerleman
fbshipit-source-id: c4e01baf388f9fae8cea7b4bfe25034bff28b461
robhogan pushed a commit that referenced this pull request Dec 2, 2024
Summary:
Removed `node-fetch` in favour of node builtin fetch to get rid of the deprecated `punycode` warning when using Node 22.
`react-native/community-cli-plugin` already requires Node >= 18 where it was made available by default (without `--experimental-fetch` flag).
This change is similar to the one made in #45227
## Changelog:
[GENERAL] [CHANGED] - Drop node-fetch in favor of Node's built-in fetch from undici in `react-native/community-cli-plugin`
Pull Request resolved: #47397
Test Plan: tests pass
Reviewed By: blakef
Differential Revision: D66512595
Pulled By: NickGerleman
fbshipit-source-id: c4e01baf388f9fae8cea7b4bfe25034bff28b461
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.MergedThis PR has been merged.p: ExpoPartner: ExpoPartnerShared with MetaApplied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@byCedric@analysis-bot@facebook-github-bot@NickGerleman