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

NodeJs 22 support is missing #1503

Open
1 task done
mcm1957 opened this issue Oct 8, 2024 · 7 comments
Open
1 task done

NodeJs 22 support is missing #1503

mcm1957 opened this issue Oct 8, 2024 · 7 comments
Labels
bug Something isn't working

Comments

@mcm1957
Copy link

mcm1957 commented Oct 8, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Describe The Bug

The api currently does not support node.js 22 currently. According to engines clause at package.json only node.js 18 and 20 are supported. (see

"node": "^18 || ^20"
).

As node.js is the current node.js version since May 2024 and will become the activer version starting November 2024 this version should be supported by rins api unless some incompatibilities exist. (Please note if this is tha case). Starting November 2024 node.js 20 will change it's state to maintainance. (see https://nodejs.org/en/about/previous-releases) .

So please add node.js 22 support to ring-api. or remove the engines restriction if there is no technical reason.


Note: This issue was created on request of @theimo1221 who maintains the ioBroker adapter ioBroker.ring (https://github.com/iobroker-community-adapters/ioBroker.ring)

To Reproduce

take a look at package,json

Expected behavior

Adapter should support at least all official supported node releases including node.js 22

Relevant log output

No response

Screenshots

No response

Additional context

No response

OS

Raspian

Node.js Version

v22.9.0

NPM Version

v10.17.0

ring-client-api

13.1.0

Operating System

Raspian

@tsightler
Copy link
Collaborator

tsightler commented Oct 8, 2024

As a general rule we focus support on LTS releases, not active versions, as early releases of active NodeJS have historically not been the most stable and it's not really worth our time to troubleshoot such issues as it just leads to extra support burden.

That being said, I understand that NodeJS 22 is preparing to go to LTS very soon, sometime this month I believe, so I agree it would be good to look test this and add it assuming nothing is broken (I don't expect that there would be any issue, but it's entirely untested).

@mcm1957
Copy link
Author

mcm1957 commented Oct 8, 2024

Thanks for your understanding and the ultrafast reply.

In general I can understand that you limit resoureces to LTS releases. ioBroker currently recommands node 20 ony. But we encourage users to use node 22 for testing systems already to detect problems related to node before node 22 is recommended for a stable system. We ran into the restriction as we wanted to test ioBroker adapter with node 22. This only as a background info.

I'll wait to hear from you.

And THANKS a LOT for your excellent work.

@tsightler
Copy link
Collaborator

But does this cause any issues with testing though? I mean, sure, it will cause npm to spit our a warning in the logs that it is an unsupported engine, but the default behavior is not to strictly enforce anything, it's just guidance as to what is officially supported.

I just upgraded my test system to Node 22.9.0, ran the code, and everything seems to work with no issue (just a smoke test for sure), so trying to understand why this would negatively impact the testing case. Not arguing against the idea that it should probably be added, just trying to understand why it is a blocker for the testing case you mentioned.

@dgreif
Copy link
Owner

dgreif commented Oct 8, 2024

I'm open to adding Node 22 to the supported list. I've been using 22 exclusively for the past month or so and it's been very stable. Will keep this in mind for the next release I put out

@mcm1957
Copy link
Author

mcm1957 commented Oct 9, 2024

But does this cause any issues with testing though? I mean, sure, it will cause npm to spit our a warning in the logs that it is an unsupported engine, but the default behavior is not to strictly enforce anything, it's just guidance as to what is officially supported.

You are right for dedicated testing purposes this is correct. But I referred to more public testing, which is a sort of user Installtion for Useres who have opted in to use latest "beta" software. The normal user installation of ioBroker enforces engines (strict mode). This is done to avoid that users ignore the warnign and create issues anyway causing avoidable load :-). And automated tests fail too as they enforce the engine to ensure that we do not publish a package which has unsupported engines included. (Scanning the log for warnings manually would not be a good option)

Bute thats off topic here so I will stop to go into more details. Anyway thanks for the hint that npm can bypass the check - mayby others here are not awar of this.

@tsightler
Copy link
Collaborator

Got it, thanks @mcm1957 for the details.

As it seems there are no objections, and I have some other small updates I'd like to get pushed out as well, I'll work with @dgreif and hopefully we can get a minor update out within a few weeks.

@mcm1957
Copy link
Author

mcm1957 commented Oct 9, 2024

It's Ok and THANKS for your excellent work. ioBroker adapter woult not exist without your api.
Will se changes either when closing this issue or when notified by dependabot about a new release.

dgreif added a commit that referenced this issue Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants