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: fix issues with native node fetch #245

Merged
merged 4 commits into from
Jun 6, 2023

Conversation

arunanshub
Copy link
Contributor

@arunanshub arunanshub commented Jun 5, 2023

This is an attempt to fix the issue highlighted in nuxt/nuxt#19245 and nuxt/nuxt#19245/issuecomment-1575671354.

@danielroe danielroe requested a review from pi0 June 5, 2023 19:19
@codecov
Copy link

codecov bot commented Jun 5, 2023

Codecov Report

Merging #245 (0f4249d) into main (5524f8a) will increase coverage by 0.06%.
The diff coverage is 100.00%.

❗ Current head 0f4249d differs from pull request most recent head 22c30be. Consider uploading reports for the commit 22c30be to get more accurate results

@@            Coverage Diff             @@
##             main     #245      +/-   ##
==========================================
+ Coverage   86.50%   86.57%   +0.06%     
==========================================
  Files           5        5              
  Lines         415      417       +2     
  Branches       61       62       +1     
==========================================
+ Hits          359      361       +2     
  Misses         56       56              
Impacted Files Coverage Δ
src/fetch.ts 89.31% <100.00%> (+0.09%) ⬆️

@pi0 pi0 changed the title Fix tests failing on node>=17 fix: fix issues with native node fetch Jun 6, 2023
Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

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

Thanks!

@pi0 pi0 merged commit 9f570b7 into unjs:main Jun 6, 2023
@arunanshub arunanshub deleted the fix-fetch-body-unusable branch June 6, 2023 19:45
@171h
Copy link

171h commented Jun 8, 2023

I saw where the PR code was modified in Files changed, I remember this issue antfu/vitesse-nuxt3, the issue error is still exists when I update with nuxi upgrage -f and the ofetch has update to v1.1.0.
I hope the information I provide can provide assistance for possible future repairs.



Snipaste_2023-06-08_23-31-05

 ERROR  Failed to fetch web fonts                                                                                                                            23:15:36  


 ERROR  fetch failed (https://fonts.googleapis.com/css2?family=DM+Sans&family=DM+Serif+Display&family=DM+Mono&display=swap)                                  23:15:36  

  at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
  at async $fetchRaw2 (/G:/repo/test/learn-X6/node_modules/.pnpm/[email protected]/node_modules/ofetch/dist/shared/ofetch.d438bb6f.mjs:215:14)
  at async $fetchRaw2 (/G:/repo/test/learn-X6/node_modules/.pnpm/[email protected]/node_modules/ofetch/dist/shared/ofetch.d438bb6f.mjs:215:14)
  at async $fetchRaw2 (/G:/repo/test/learn-X6/node_modules/.pnpm/[email protected]/node_modules/ofetch/dist/shared/ofetch.d438bb6f.mjs:215:14)
  at async Module.$fetch2 (/G:/repo/test/learn-X6/node_modules/.pnpm/[email protected]/node_modules/ofetch/dist/shared/ofetch.d438bb6f.mjs:239:15)

@arunanshub
Copy link
Contributor Author

arunanshub commented Jun 8, 2023

@171h I'm using Node v20 too, but it somehow works on my machine. Have you tried deleting node_modules, .nuxt, .output folders?

On the other hand, I personally don't like the recursive implementation of $fetchRaw and onError functions.

Edit: I am unable to replicate the error in Windows 11.

@171h
Copy link

171h commented Jun 8, 2023

I have deleted node_modules, .nuxt, .output.
This error exist on Node v20 and v19, but v16 will be ok. So I still use v16 in the past few months due to the issue.

@171h
Copy link

171h commented Jun 8, 2023

I used nvm to manage the nodejs version. I just upgraded nvm to the latest version 1.11.1 and then reinstalled nodejs. Amazingly, this error disappeared on node v20.3.0, but still exists on v19.9.0.

@171h
Copy link

171h commented Jun 9, 2023

This problem may already have been fixed by this pr nodejs/node#47732 since node v20.2.0

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

Successfully merging this pull request may close these issues.

3 participants