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

Use node.js v20 #51

Merged
merged 10 commits into from
Nov 29, 2023
Merged

Use node.js v20 #51

merged 10 commits into from
Nov 29, 2023

Conversation

jtsaito
Copy link
Contributor

@jtsaito jtsaito commented Nov 23, 2023

@jtsaito jtsaito self-assigned this Nov 23, 2023
package.json Outdated
"@octokit/core": "*",
"@octokit/graphql": "*",
"@octokit/plugin-paginate-graphql": "*",
"fetch-mock": "^9.11.0"
"fetch-mock": "npm:@gr2m/fetch-mock@9.11.0-pull-request-644.1"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have tried to find an alternative for fetch-mock for mocking requests produced by @octokit but ended up following the example of @octokit's mocking for their own tests.

Context (grab some popcorn and make your self comfy): @octokit packages moved to support for node.js 20. Node.js 20 switched from http to defaulting to global fetch function based on undici-fetch.

As a consequence, standard mocking libraries like nock are (still) not supporting this default. Thus when using @ocotokit we had to use fetch-mock as recommended by the owners of @octokit.

Unfortunately, fetch-mock has not been actively maintained for ... two years. Therefore, the @octokit team submitted a PR and is now referencing this for mocking in their own project.

Far from being ideal, I still suggest to follow the lead of @octokit because this is the set of packages we need to mock.

I tried the new built-in node:test mock which has zero convenience and did not work for mocking fetch - at least for me. I also tried nock as mentioned above and looked for up-to date alternatives but nothing seemed to work. Mock Service Worker has recently become popular, but this seems to follow be its own framework and so I did not even test it.

Copy link
Member

Choose a reason for hiding this comment

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

😱🍿🥹🍿🫣🍿🫠

@jtsaito jtsaito marked this pull request as ready for review November 23, 2023 16:16
@jtsaito jtsaito requested a review from a team November 23, 2023 16:25
Copy link
Member

@jansiwy jansiwy left a comment

Choose a reason for hiding this comment

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

LGTM!

@jtsaito
Copy link
Contributor Author

jtsaito commented Nov 23, 2023

It might work with MSW mswjs/msw#1543 ... .

@jtsaito
Copy link
Contributor Author

jtsaito commented Nov 29, 2023

Merged #52 replacing fetch-mock with MSW.

@jtsaito jtsaito merged commit 1e822ea into main Nov 29, 2023
1 check passed
@jtsaito jtsaito deleted the node.js-v20 branch November 29, 2023 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants