danger / danger-js

⚠️ Stop saying "you forgot to …" in code review
http://danger.systems/js/
MIT License
5.22k stars 363 forks source link

[BUG] Error: ReferenceError: fetch is not defined #1448

Closed squarefrog closed 2 months ago

squarefrog commented 2 months ago

Describe the bug After updating to Danger JS 12.x I'm unable to run Danger.

To Reproduce Steps to reproduce the behavior:

  1. Install danger through homebrew brew install danger/tap/danger-js
  2. Try to run danger

Expected behavior I expected Danger to work as I have a node version installed greater than the minimum.

Screenshots If applicable, add screenshots to help explain your problem.

Starting Danger PR on company/ios-app#5606
Error:  ReferenceError: fetch is not defined
    at defaultRequestHandler (/snapshot/danger-js/node_modules/@gitbeaker/rest/dist/index.js:101:22)
    at Object.requester.<computed> [as get] (/snapshot/danger-js/node_modules/@gitbeaker/requester-utils/dist/index.js:53:16)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at process.runNextTicks [as _tickCallback] (node:internal/process/task_queues:65:3)
    at Function.runMain (pkg/prelude/bootstrap.js:1980:13)
    at node:internal/main/run_main_module:17:47
    at async /snapshot/danger-js/node_modules/@gitbeaker/core/dist/index.js:105:22

Your Environment

software version
danger.js 12.2.0
node v18.20.2, also tried v22.1.0
npm 10.5.0
Operating System macOS 14.4.1

Additional Context We run this in an iOS project without any JS dependencies, and I'm not too familiar with JS, so forgive me if this is a trivial issue to solve for an experienced JS engineer!

squarefrog commented 2 months ago

If I run this through yarn it works fine, so I guess this could be something to do with installing through brew?

orta commented 2 months ago

Yeah, brew must be referencing an old node version somehow

SvenMuc commented 2 months ago

I had the same issue with danger-swift installed via brew. Seems danger-swift uses danger-js under the hood.

https://github.com/danger/swift/issues/608

squarefrog commented 2 months ago

Aye that’s right, it does. I do have node installed though. If I do node —version it reports v22 what is what I expect.

Looking at the brew manifest, all it does is download the latest bin from here. Running that same bin manually works fine.

I’m really not sure why installing it through brew uses an old version. All I can think is perhaps there’s something in my PATH that is getting intercepted early by the brew installation but not the manual one.

fbartho commented 2 months ago

Speculation: do you install node via brew? (Or instead nvm/nodeenv or other tools?)

I'd expect brew's install of node to supersede other tools during brew-installs, or at least that was what I saw years ago when I debugged a similar issue. My solution was to uninstall brew-based node so the only node on my system came from my node-manager.

squarefrog commented 2 months ago

That’s a really good question. Yeah I installed it with brew. I’ll look and see tomorrow how to install it a different way and try that!

SvenMuc commented 2 months ago

If that helps, on my machine I installed no node js at all. I just installed danger by

brew install danger/tap/danger-swift

this installs danger-js and danger-swift

% brew info danger-swift
==> danger/tap/danger-swift: stable 3.18.1, HEAD
Write your Dangerfiles in Swift
https://github.com/danger/danger-swift
Installed
/opt/homebrew/Cellar/danger-swift/3.18.1 (17 files, 5.0MB) *
  Built from source on 2024-05-06 at 23:14:54
From: https://github.com/danger/homebrew-tap/blob/HEAD/danger-swift.rb
==> Dependencies
Required: danger/tap/danger-js ✔
==> Requirements
Build: Xcode (on macOS) ✔
==> Options
--HEAD
    Install HEAD version
% brew info danger-js   
==> danger/tap/danger-js: stable 64
https://github.com/danger/danger-js
Installed
/opt/homebrew/Cellar/danger-js/64 (3 files, 107.1MB) *
  Built from source on 2024-05-06 at 22:32:49
From: https://github.com/danger/homebrew-tap/blob/HEAD/danger-js.rb
fbartho commented 2 months ago

I’ve never written a Homebrew Tap; could it be that neither tap expresses a Node dependency?

So either we’re getting node from the OS, node from Xcode, or node randomly from other brew state?

squarefrog commented 2 months ago

So interestingly, I get the same error even if node is not installed.

$ brew uninstall node
Uninstalling /opt/homebrew/Cellar/node/22.1.0... (2,069 files, 75.4MB)

$ which node
node not found

$ node -v
zsh: command not found: node

$ danger pr --dangerfile danger-prebuild.js https://example.com/company/ios-app/-/merge_requests/5606
Starting Danger PR on company/ios-app#5606
Error:  ReferenceError: fetch is not defined
    at defaultRequestHandler (/snapshot/danger-js/node_modules/@gitbeaker/rest/dist/index.js:101:22)
    at Object.requester.<computed> [as get] (/snapshot/danger-js/node_modules/@gitbeaker/requester-utils/dist/index.js:53:16)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at process.runNextTicks [as _tickCallback] (node:internal/process/task_queues:65:3)
    at Function.runMain (pkg/prelude/bootstrap.js:1980:13)
    at node:internal/main/run_main_module:17:47
    at async /snapshot/danger-js/node_modules/@gitbeaker/core/dist/index.js:105:22

Does the compiled binary version come with node bundled?

orta commented 2 months ago

Good guess, looks like it does:

https://github.com/danger/danger-js/blob/main/package.json#L69-L70 - https://github.com/yao-pkg/pkg-fetch is a fork which supports node 18

heltoft commented 2 months ago

This one is on me - sorry for this!

For the testing of the GitLab changes I made I was running with a custom yarn pkg command, and I see now that the changes to the package command never made it into the pull requests I did.

I believe it should suffice for now to simply update those yarn pkg commands to use the node18 targets. I can confirm it works locally on my machine at least.

I can submit a pull request for this.

SvenMuc commented 2 months ago

I can confirm the fix. After reinstalling danger-js and danger-swift via brew everything works as expected.

squarefrog commented 2 months ago

This is amazing, thank you!