search

nodejs / node-core-utils

CLI tools for Node.js Core collaborators
https://nodejs.github.io/node-core-utils/
MIT License
234 stars 106 forks source link

TypeError: fetch is not a function #594

Closed codebytere closed 2 years ago

codebytere commented 2 years ago

Seen after pulling latest master - I suspect https://github.com/nodejs/node-core-utils/commit/a571938d6210e1b3138a89037e3791f518bfc7d5.

Stacktrace ```console node on git:master ❯ g node land 40757 12:11PM TypeError: fetch is not a function at wrappedFetch (/Users/codebytere/Developer/node-core-utils/lib/request.js:17:10) at Request.fetch (/Users/codebytere/Developer/node-core-utils/lib/request.js:37:12) at Request.text (/Users/codebytere/Developer/node-core-utils/lib/request.js:45:17) at Request.json (/Users/codebytere/Developer/node-core-utils/lib/request.js:50:29) at Request.query (/Users/codebytere/Developer/node-core-utils/lib/request.js:108:31) at Request.gql (/Users/codebytere/Developer/node-core-utils/lib/request.js:68:33) at LandingSession.warnForWrongBranch (/Users/codebytere/Developer/node-core-utils/lib/landing_session.js:486:65) at main (/Users/codebytere/Developer/node-core-utils/components/git/land.js:175:21) at processTicksAndRejections (node:internal/process/task_queues:96:5) ```

cc @targos

targos commented 2 years ago

Did you run npm install ?

codebytere commented 2 years ago

@targos yes, and removed node_modules beforehand to be doubly sure! Using Node.js v16.2.0.

Update - installed v16.13.0 locally and it worked - we should probably error or specify somewhere the base version one now needs to run node-core-utils 🤔

targos commented 2 years ago

Oh wow, it looks like undici doesn't always export a fetch function:

PS C:\git\TEST> volta run --node 16.2.0 node -p "require('undici').fetch"
undefined
PS C:\git\TEST> volta run --node 16.13.0 node -p "require('undici').fetch"
[AsyncFunction: fetch]

I thought it would be a good idea to use the module to give it some real world test coverage...

@nodejs/undici

targos commented 2 years ago

We should probably go back to node-fetch. undici.fetch doesn't work on the latest version of Node.js 14

ronag commented 2 years ago

We only export fetch for node >= 16.5 since we depend on web streams

Ethan-Arrowood commented 2 years ago

That is strange, but I believe Undici fetch is still considered experimental even on node 16. Best to stick to node-fetch for now. I can take a closer look later on as well

codebytere commented 2 years ago

@targos yeah i did see that i docs here - i can open a PR to revert if that's the path forward!

targos commented 2 years ago

Go ahead! I removed node-fetch because version 3.x doesn't support commonjs, so you'll have to stay with version 2.x.

codebytere commented 2 years ago

Done: https://github.com/nodejs/node-core-utils/pull/595