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.
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
feat(experimental): option to polyfill
fetch
usingundici
in Node.js <18 #40318feat(experimental): option to polyfill
fetch
usingundici
in Node.js <18 #40318Changes from 33 commits
66408d8
28a09b8
0daf850
1c5c027
f4169d7
0b2475a
07be524
32d3677
4cbc65f
eb44e6b
7fd115a
2a962a4
6593733
a11d117
ebc33d7
f623f65
502db7f
88c65cd
1651916
8393aaf
76d7b6c
367862f
3866700
58ecb4c
454d917
1b26d54
0de1fb4
ec21630
e8550fa
7e6b1f9
4919546
ef8b44f
789b43b
245ff7b
60d99d7
0de3c26
989f9c8
ed03b2b
421891b
acf39cb
203c3e7
bbc2ac5
cbafb7f
d2087ce
f770bd5
31beccc
7722ea5
424531a
7da8b81
72c90fe
e5f3c60
fbc3e4d
fcfa989
08fefca
b101ec6
82a0961
ef18f28
05ca18f
519c717
6002dba
8ed03ee
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
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.
As discussed @Ethan-Arrowood let's wrap this in an
if
and check ifprocess.version
is at leastv16.8.0
as pointed out in #40318 (review)We should also warn if the developer is setting this flag with a lower Node.js version
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.
Yes, it is not ideal, but fetch will only work in Node 16.8+ because of the APIs it depends on from Node.js itself. See this PR nodejs/undici#1534 that expands on this
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.
This could be a problem because the agent is created for every
fetch()
. It should be reused, right?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.
No, the Agent is created and assigned to
global.__NEXT_HTTP_AGENT
orglobal.__NEXT_HTTPS_AGENT
. This function just returns the correct agent every time fetch is called.But since we can't do that for the Undici agent we have to have all that
!global.__NEXT_UNDICI_AGENT_SET
logic.