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

Node 18 and @octokit/core v5.0.0 incompatability #77

Open
ataylorme opened this issue Aug 17, 2023 · 6 comments · Fixed by #83
Open

Node 18 and @octokit/core v5.0.0 incompatability #77

ataylorme opened this issue Aug 17, 2023 · 6 comments · Fixed by #83
Labels
bug Something isn't working

Comments

@ataylorme
Copy link

Describe the bug
Moctokit does not work with Node 18. All .reply responses return the same result to Octokit. Setting the status as 200 and 404 do not differ the mock response`

I believe this is due to Node 18 using a new http client named undici for fetch API support

Looking at the code for Moctokit it uses nock, which has an open issue for undici support

To Reproduce
Use Moctokit with Node 18 and @octokit/core v5.0.0

Expected behavior
Proper mock responses are returned.

Additional context
I don't actually expect this to be solved until Nock is updated but an incompatibility note in the README would be nice.

@shubhbapna
Copy link
Collaborator

Thank you for pointing this out. I will keep an eye out for this or see if there are any alternative because of nock/nock#2397 (comment)

I will update the docs till then.

@shubhbapna shubhbapna added documentation Improvements or additions to documentation bug Something isn't working and removed documentation Improvements or additions to documentation labels Aug 17, 2023
@shubhbapna
Copy link
Collaborator

I have updated the docs but will keep this issue open

oliversalzburg added a commit to oliversalzburg/mock-github that referenced this issue Oct 27, 2023
Breaking change!

- Internally uses `undici`'s `MockAgent`, instead of `nock` to mock requests.
- `nock` is still used for data-matching to retain backwards
  compatibility.
- Tests now use `fetch` instead of `axios`.

Requires node>=18 and @octokit/rest>=20

Closes kiegroup#77
@shubhbapna
Copy link
Collaborator

experimental support for fetch added to nock. I will soon start testing compatibility of this with mock-github

nock/nock#2397 (comment)

@oliversalzburg
Copy link

Not sure if I'm doing anything wrong, but I upgraded the latest main of this project, ran npm add nock@beta and got all tests to pass with minimal effort. I'll send a PR.

oliversalzburg added a commit to oliversalzburg/mock-github that referenced this issue May 31, 2024
shubhbapna pushed a commit that referenced this issue Jul 17, 2024
* feat!: Use nock@14, support node@20/fetch

Fixes #77

* feat!: Upgrade to octokit/rest@v20

* fix: NodeJS version ranges
@shubhbapna
Copy link
Collaborator

Hey everyone, so sorry for the delay but thanks to @oliversalzburg a beta version of mock-github is available that supports mocking of the inbuilt fetch client in node 18 and up. It is currently beta since we depend on nock's beta release. Once nock releases a more stable version than I shall make a general release

@oliversalzburg
Copy link

I upgraded all my actions to use the new beta and all tests are passing. I don't have many tests yet, because I was waiting for this project to work flawlessly, but what's there works 😄

I noticed that I need to pass { request: { fetch } } explicitly to getOctoKit in my tests to make it work though. I don't understand yet why that is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants