[MAINT]: Tracking issue for dropping support for Node v14/v16 and Fetch dispatch breaking changes

[nickfloyd]

Describe the need

We need a clear picture of where things are and what needs to ship when pushing through this breaking change.

Octokit JS Dependency Graph/Breaking Change map

Based on the discussions with @gr2m, @wolfy1339, @kfcampbell, and on the data above, the rough plan would be to:

Step 1:

Based on the data in the doc - find out if any of the repos that do not have beta branches or a PR for dropping node support for v14/v16 should have one. Why: This will give us 100% certainty that all packages (that need to) have been updated so that we can move forward with the Fetch breaking change:

• [x] sandbox
  • [x] no node version defined in package.json ✅
  • [x] no node version defined in workflows ✅
• [x] graphql-action
  • [x] no node version defined in package.json ✅
  • [x] no node version defined in workflows ✅ (https://github.com/octokit/graphql-action/pull/223)
• [x] fixtures
  • [x] no node version defined in package.json ✅
  • [x] no node version defined in workflows (https://github.com/octokit/fixtures/pull/576) ✅
• [x] fixtures-server
  • [x] no node version defined in package.json ✅
  • [x] no node version defined in workflows (https://github.com/octokit/fixtures-server/pull/289) ✅
• [x] openapi-types.ts
  • [x] no node version defined in package.json ✅
  • [x] no node version defined in workflows (Node defined in update and prettier workflows but not testing) (https://github.com/octokit/openapi-types.ts/pull/323) ✅
• [x] request-action
  • [x] no node version defined in package.json ✅
  • [x] no node version defined in workflows (https://github.com/octokit/request-action/pull/255) ✅
• [x] graphql-schema
  • [x] no node version defined in package.json ✅
  • [x] no node version defined in workflows (Workflows are all @ v18) ✅
• [x] app-permissions
  • [x] no node version defined in package.json ✅
  • [x] no node version defined in workflows (Node defined in workflows is at LTS/* but update is at v16) ✅
• [x] openapi
  • [x] no node version defined in package.json (Node version is defined in package.json) (https://github.com/octokit/openapi/pull/370) ✅
  • [x] no node version defined in workflows (Update and update-dryrun are at 16, all other workflows are at LTS ) ✅
• [x] webhooks
  • [x] no node version defined in package.json ✅
  • [x] no node version defined in workflows (https://github.com/octokit/webhooks/pull/810) ✅

Step 2

Go through all of the existing repos that have a beta branch and/or PR for dropping node support for v14/v16 and determine if we can merge (most likely, they are the ones blocked by core). Why: This will get all of the remaining change sets that can go out the door in a place where when we ship the change to core we can do it confidently.

• [x] octokit.js | https://github.com/octokit/octokit.js/pull/2456
  • [x] Draft is green ✅
  • [x] Updated deps ✅ | https://github.com/octokit/octokit.js/pull/2493
  • [x] Merged ✅
• [x] app.js | https://github.com/octokit/app.js/pull/409
  • [x] Draft is green ✅ (blocked by fetch issue)
  • [x] Merged ✅
• [x] plugin-paginate-graphql.js | https://github.com/octokit/plugin-paginate-graphql.js/pull/88
  • [x] Draft is green ✅
  • [x] Merged ✅
• [x] core.js | https://github.com/octokit/core.js/pull/563
  • [x] Draft is green ✅ (blocked by fetch issue)
  • [x] Merged ✅
• [x] rest.js | https://github.com/octokit/rest.js/pull/305
  • [x] Draft is green ✅
  • [x] Merged ✅
• [x] action.js | https://github.com/octokit/action.js/pull/513
  • [x] Draft is green ✅
  • [x] Merged ✅
• [x] request-action.js | once https://github.com/octokit/action.js/pull/513 is merged
  • [x] Draft exists ✅
  • [x] Draft is green ✅
  • [x] Merged ✅
• [x] graphql-action
  • [x] Draft exists ✅
  • [x] Draft is green ✅
  • [x] Merged ✅

    Step 3

Remove request.agent option from request.js in a beta breaking change branch.

• [x] @octokit/request: Remove docs. on options | pass options transparently to fetch | https://github.com/octokit/request.js/pull/599
• [x] @octokit/types: Remove agent & timeout options from RequestRequestOptions.ts | https://github.com/octokit/types.ts/pull/561
• [x] Update core, as @gr2m suggests - Replacing the tests to implement a client proxy and certificate authentication using a fetch dispatcher. https://github.com/octokit/core.js/pull/585

Step 4

Ship the core changes to either beta branches or main for packages that have already shipped node v14/16 - using the sequence detailed in the doc above

Step 5

Fast follow all of this with docs and examples. As @gr2m suggested:

• [x] Add documentation how to create a custom fetch function with a dispatcher preset (it's easy, https://github.com/nodejs/undici/discussions/2167) - https://github.com/octokit/request.js/pull/614
• [x] Add documentation how to intercept all requests sent using Node's native fetch method by setting - https://github.com/octokit/request.js/pull/614
• [x] Be more explicit that this package (and other Octokit packages) does not support Node 14/16 - https://github.com/octokit/octokit.js/issues/2497

Reference:

• https://github.com/orgs/octokit/discussions/44
• https://github.com/octokit/octokit.js/discussions/2484

Note: As a group, we should stamp out a reasonably simple to consume dependency graph for all of our JS packages to help ease things like making breaking changes in the future.

[wolfy1339]

I think @gr2m laid it out pretty well on the dependency graph in the previous discussion.

I think there's 2 branches currently that don't interfere with each other with regards to breaking changes:

• @octokit/webhooks, and it's associated types/schema repository
• Packages that depend on @octokit/core or @octokit/request

[kfcampbell]

@nickfloyd thank you for putting this together! That's an excellent summary and procedure.

[gr2m]

Looks great! I'll help the best I can! Ping me anytime if my perspective would be helpful

[wolfy1339]

Many issues are coming up about dropping Node 14/16, and node-fetch.

How can we be more explicit that this package (and other Octokit packages) does not support Node 14/16?

When it comes to the message logged from @octokit/request, it does seem misleading and confusing given we don't support Node 14/16 anymore, see https://github.com/octokit/octokit.js/issues/2495

Error: Global "fetch" not found. Please provide options.request.fetch to octokit or upgrade to node@18 or newer.

[nickfloyd]

Solid call out @wolfy1339. I think the not found wording makes it seem like it's something octokit is supposed to take care of.

Perhaps we explicitly call out our lack of support there, something like:

Octokit ^3.x does not support NodeJS v16 and lower. You will need to either update to NodeJS ^18.x or pass fetch via options.request.fetch

IDK, perhaps something like that?

[gr2m]

Maybe we could add a section to the README and link to it from the error, so we can provide a more thorough explanation and most importantly we can update whenever needed.

I'd say the availability of fetch is independent of the Node version. fetch is still experimental and it can be disabled with --no-experiemental-fetch. Also Node is not the only place where Octokit is run, I'm not sure if e.g. Electron or React Native have a global fetch method.

I'd do something like this:

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

[nickfloyd]

I like @gr2m's approach here! @oscard0m you were asking how you might be able to lend a hand in all of this - is this something that you might be interested in picking up? I focusing on the actions libs (thankfully, @gr2m and @wolfy1339 have been incredibly helpful there) and will have to wrap that up before I can get to what they suggest above.

[oscard0m]

I like @gr2m's approach here! @oscard0m you were asking how you might be able to lend a hand in all of this - is this something that you might be interested in picking up? I focusing on the actions libs (thankfully, @gr2m and @wolfy1339 have been incredibly helpful there) and will have to wrap that up before I can get to what they suggest above.

https://github.com/octokit/octokit.js/issues/2497

[kfcampbell]

I'm going to close this issue now as all of our breaking changes are rolled out! :tada: :tada: :tada:

Thank you all for driving these changes through.