Skip to content

Conversation

@joshmgross
Copy link
Contributor

#1841 could have been caught in unit tests.

As long as we have users of the toolkit with deprecated actions (Node 16) and running with the default Node version on hosted runners (Node 18), we should avoid breaking the toolkit by utilizing Node 20 features.

In the future, we may want to be more clear about what versions of Node we support in the toolkit.

@joshmgross joshmgross requested a review from a team as a code owner October 4, 2024 20:39
@joshmgross
Copy link
Contributor Author

This is failing as expected due to #1841, so I've confirmed that this would catch the reference error 🎉

ReferenceError: crypto is not defined
https://github.com/actions/toolkit/actions/runs/11186675222/job/31102183371?pr=1843

CI should be passing once #1842 is merged.

@joshmgross
Copy link
Contributor Author

Removed Node 16 as the Artifact tests were not passing

FAIL packages/github/__tests__/github.proxy.test.ts
  ● @actions/github › should only use default agent if one is not provided

    fetch is not set. Please pass a fetch implementation as new Octokit({ request: { fetch }}). Learn more at https://github.com/octokit/octokit.js/#fetch-missing

      at fetchWrapper (packages/github/node_modules/@octokit/request/dist-node/index.js:57:11)
      at request2 (packages/github/node_modules/@octokit/request/dist-node/index.js:180:14)
      at hook (packages/github/node_modules/@octokit/auth-token/dist-node/index.js:58:10)
      at packages/github/node_modules/before-after-hook/lib/register.js:25:15

That's not a difficult thing to fix, but the purpose of this PR is not to extend which versions of Node we support, only to recognize which versions users already depend on.

@joshmgross joshmgross merged commit bb2278e into main Nov 8, 2024
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