Skip to content

chore: require Node.js 22 and update pnpm to 10.33.0#232

Open
yslpn wants to merge 7 commits intofeature-sliced:masterfrom
yslpn:chore/node-22-and-pnpm-10-33
Open

chore: require Node.js 22 and update pnpm to 10.33.0#232
yslpn wants to merge 7 commits intofeature-sliced:masterfrom
yslpn:chore/node-22-and-pnpm-10-33

Conversation

@yslpn
Copy link
Copy Markdown
Contributor

@yslpn yslpn commented Apr 18, 2026

This PR updates the project runtime and package manager baseline.

Why

Node.js 20 reaches end-of-life on April 30, 2026, which means its LTS support effectively ends on that date. Moving the project to Node.js 22 keeps the repository on a currently supported LTS line and aligns local development with CI.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 18, 2026

🦋 Changeset detected

Latest commit: 099295c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
steiger Patch
@feature-sliced/steiger-plugin Patch
@steiger/toolkit Patch
@steiger/integration-tests Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Solant Solant requested a review from illright April 19, 2026 18:38
Copy link
Copy Markdown
Contributor

@Solant Solant left a comment

Choose a reason for hiding this comment

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

I'm fine with updating the node version for the GitHub runner and the pnpm version, but I'm not sure if we need to add engines string to package.json, because we don't use any Node 22-specific APIs.

@yslpn
Copy link
Copy Markdown
Contributor Author

yslpn commented Apr 19, 2026

I'm fine with updating the node version for the GitHub runner and the pnpm version, but I'm not sure if we need to add engines string to package.json, because we don't use any Node 22-specific APIs.

In the long term, I think we should be ditching unnecessary npm dependencies in favor of Node.js platform features. This is a safer, maintainable and more performant option. But to achieve this, we need to specify versions for compatibility.

@Solant
Copy link
Copy Markdown
Contributor

Solant commented Apr 19, 2026

I'm not sure if any of the newer Node APIs can replace any of the dependencies, but we can definitely use something built-in. But I don't think we need to add the engnies property without a real reason, like a native dependency or usage of a newer APIs.

@illright
Copy link
Copy Markdown
Member

Agree with @Solant — I'm all for using platform-native features and shedding as many dependencies as possible, but raising the bar of minimum Node version support is a breaking change, and as with every breaking change, we should first explore all the ways that we can get what we want without a breaking change. In this case, let's upgrade Node and pnpm in CI, but not in engines or @types/node

@yslpn
Copy link
Copy Markdown
Contributor Author

yslpn commented Apr 20, 2026

Agree with @Solant — I'm all for using platform-native features and shedding as many dependencies as possible, but raising the bar of minimum Node version support is a breaking change, and as with every breaking change, we should first explore all the ways that we can get what we want without a breaking change. In this case, let's upgrade Node and pnpm in CI, but not in engines or @types/node

Okay, I'll remove engines.

But I don't understand why you're against updating @types/node? Those are our devDependencies.

@illright
Copy link
Copy Markdown
Member

Having @types/node at 18 means that if we ever use Node APIs that are not available in Node 18, we will get a type error. Just a safeguard to ensure we're actually compatible with Node 18 like we say we do

@yslpn
Copy link
Copy Markdown
Contributor Author

yslpn commented Apr 20, 2026

with Node 18 like we say we do

If we say support for node 18, maybe we should leave engines like this? What do you think?

  "engines": {
    "node": ">= 18"
  },

Some people may use a lower version of node js, but our types target version 18.

@illright
Copy link
Copy Markdown
Member

We could, though I don't think it's strictly necessary or even beneficial at all. We don't really know, maybe Steiger works even on Node 16, but since we don't want to bother testing, we just say that we don't guarantee support for anything lower than 18. Explicitly blocking installs for Node 16 seems unnecessary and might introduce unneeded complications

@yslpn
Copy link
Copy Markdown
Contributor Author

yslpn commented Apr 20, 2026

@illright I don't like this line of thinking - that we develop and test under one version, but point users to other versions. This is the wrong approach.

We either update according to the contract, or let's not update and continue using version 18.

After all, the library is in the early stages of development, and before the first stable version, we could make breaking changes

Note

The project is in beta and in active development. Some APIs may change.

@illright
Copy link
Copy Markdown
Member

that we develop and test under one version, but point users to other versions

I don't think I ever said that this is my position. We have a list of supported Node versions: from 18 onwards, and we develop and test with this minimum in mind. If someone runs Node 16 and chooses to run Steiger at their own risk, let them — maybe it will work, maybe it will not, but if they open an issue saying "it doesn't work with Node 16", that issue will not be fixed because 16 is not an officially supported version.

With all of that in mind, imposing an artificial restriction by preventing installs on Node <18 doesn't sound to me like it will bring any good whatsoever

@yslpn
Copy link
Copy Markdown
Contributor Author

yslpn commented Apr 20, 2026

If we don't bump @types/node to the current version, we get a warning from pnpm. Is this acceptable?

WARN Issues with peer dependencies found
integration-tests
└─┬ vite 7.0.6
└── ✕ unmet peer @types/node@"^20.19.0 || >=22.12.0": found 18.19.130
Done in 2.7s using pnpm v10.33.0

@illright
Copy link
Copy Markdown
Member

Yeah, it's alright. Weird that Vite imposes peer restrictions on @types/node, but whatever

@yslpn
Copy link
Copy Markdown
Contributor Author

yslpn commented Apr 20, 2026

that we develop and test under one version, but point users to other versions

I don't think I ever said that this is my position.

You didn't literally say this, but this is the conclusion we've reached. We'll be testing and developing on version 22, but for users, we're aiming for support on version 18.

Am I wrong?

@illright
Copy link
Copy Markdown
Member

Yes, this is not entirely accurate — while I have latest Node on my dev machine, the CI runs all the integration tests on Node 18, preventing code that doesn't work on Node 18 from ever hitting a release. The @types/node thing is just an extra line of defense, but not the primary one

@yslpn
Copy link
Copy Markdown
Contributor Author

yslpn commented Apr 20, 2026

the CI runs all the integration tests on Node 18, preventing code that doesn't work on Node 18 from ever hitting a release.

I updated the versions in CI. Or should I not do this?

@illright
Copy link
Copy Markdown
Member

Yes, please revert the version change in CI, it's important to run tests on 18

@illright
Copy link
Copy Markdown
Member

Could you also undo the change to engines and the TSConfig base? I feel like these should stay at 18

@yslpn
Copy link
Copy Markdown
Contributor Author

yslpn commented Apr 24, 2026

@illright Sorry, I forgot about that.

Done.

@illright
Copy link
Copy Markdown
Member

The engines too, please :)

illright
illright previously approved these changes Apr 25, 2026
Solant
Solant previously approved these changes Apr 25, 2026
@illright
Copy link
Copy Markdown
Member

oh, the CI is failing because the lockfile is not up-to-date, could you also run pnpm i please to generate a new lockfile?

@yslpn yslpn dismissed stale reviews from Solant and illright via 099295c April 25, 2026 19:57
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