chore!: remove Node.js 21.x from the range of supported versions

[MikeMcC399]

• closes Update better-sqlite3 to >= 11.5.0 #592

Issue

1. Node.js 21 entered end-of-life on Jun 6, 2024. It is however still allowed by:

corepack/package.json

Lines 12 to 14 in 0ae3c3c

	"engines": {
	"node": "^18.17.1 || >=20.10.0"
	},

2. The workflow ci.yml continues to test it:

corepack/.github/workflows/ci.yml

Lines 47 to 52 in 0ae3c3c

	matrix:
	node:
	- 18
	- 20
	- 21
	- 22

3. There is no version of better-sqlite3 (used in devDependencies) which supports both Node.js 21 and 23, so continuing support for Node.js 21 blocks supporting Node.js 23.

Change

• Drop support for Node.js 21 in package.json engines and testing in ci.yml.
• Update better-sqlite3 to [email protected] (Edit: was latest when this PR was originally submitted).

Additional change

• Add Node.js 23 to workflow and exclude windows-latest from testing due to issue ci vitest fails "handle integrity checks" on Windows Node.js 23 #597

[aduh95]

It looks like the exit code is different on Windows with Node.js 23.x for some reason 🤔

[MikeMcC399]

@aduh95

It looks like the exit code is different on Windows with Node.js 23.x for some reason 🤔

That is going to need separate investigation. The test appears to be flaky, as I had previously seen it successful.

I am going to remove Node.js 23 tests from this PR so it should then pass all tests. Stand by!

[aduh95]

I don't think removing 23.x is the correct way forward, it looks like it would be more reasonable to either skip this particular test on 23.x, or account for the other exit code. AFAICT it's not flaky on CI, it looks like it's consistently failing with the same error

[MikeMcC399]

@aduh95

I don't think removing 23.x is the correct way forward, it looks like it would be more reasonable to either skip this particular test on 23.x, or account for the other exit code. AFAICT it's not flaky on CI, it looks like it's consistently failing with the same error

I talking about moving the update to test in 23.x into a separate PR and this is a different issue which needs to be resolved.

In https://github.com/MikeMcC399/corepack/actions/runs/12744446123/job/35517323345 it is passing, so that is why I say it is flaky.

[MikeMcC399]

@aduh95

I suggest to merge this now, since it cleanly passes CI. It's a good basis now for working out what to do with Node.js 23 separately.

[MikeMcC399]

@aduh95

• I've logged the Windows / Node.js 23.x test failure issue in ci vitest fails "handle integrity checks" on Windows Node.js 23 #597 since I was able to reproduce it locally. I don't have any explanation why it worked once in CI and is now failing all the time. If it's now reproducible, it should make it easier to deal with it anyway.

[aduh95]

I talking about moving the update to test in 23.x into a separate PR and this is a different issue which needs to be resolved.

Is it a different issue? It seems to me that it's very much related, we're dropping 21.x support because we want to support 23.x and there's no other solution because of what supports better-sqlite3.
I think I would prefer we address it in this PR, either by editing the test and adding a TODO, or by fixing the underlying issue if we're able to figure it out

[MikeMcC399]

@aduh95

• You are right that the motivation was to test in Node.js 23, however dropping Node.js 21 should be done in any case since it is EOL. That was done separately for Node.js 19 in PR chore!: remove Node.js 19.x from the range of supported versions #375.

• I would prefer to merge this PR as is. I generally prefer smaller self-contained PRs. Would it break something if it is merged now? Perhaps I am missing something?

• Your preference is different and involves solving ci vitest fails "handle integrity checks" on Windows Node.js 23 #597 as a prerequisite. It's your decision of course. At this point in time I don't think I can contribute much more however. I also can't predict how long it will take to resolve this issue, and therefore how long potentially this PR would need to wait before it could be updated.

[MikeMcC399]

@aduh95

• Thank you for progressing the issue ci vitest fails "handle integrity checks" on Windows Node.js 23 #597! At this time I would assume that the test problem in corepack is caused by a regression in Node.js 23.0.0 for Windows, that it will be fixed in Node.js and that it will need at least a few weeks to get a fix into a new Node.js 23.x release.

In the meantime I propose to add Node.js 23 to the test matrix for corepack and temporarily exclude testing on Windows with Node.js 23. I think this is the cleanest solution. Windows is already being tested on other Node.js versions (18, 20 & 22), so the testing gap is minimal.

The advantage is that introducing tests with Node.js 23.x for Ubuntu and macOS is not delayed further.

Once the Windows issue is solved, it is easy to remove the exclusion.

I believe this PR is now in a state where you could consider merging it.

[aduh95]

Thanks for pushing for it :)