Skip to content

Conversation

@Tosin-JD
Copy link
Contributor

@Tosin-JD Tosin-JD commented May 7, 2025

I only tried to add the node js version which is v24. Please, how can I be able to ensure that this issue does not affect the version support of v24?

@github-actions github-actions bot added the dependencies Changes to the project's dependencies label May 7, 2025
@ericcornelissen
Copy link
Owner

Thank you for taking the time to contribute!

I only tried to add the node js version which is v24.

Please undo your changes to the lockfile and update the manifest (package.json) instead of the lockfile (package-lock.json) - then sync the manifest and lockfile by running npm install.

Your current contribution is incorrect because 1) changing the version ranges in the lockfile does not affect users of the library and 2) you can't change the version ranges of dependencies.

Additionally, please add 24.0.0 as an entry to the matrix for the test-compatibility job in the checks.yml GitHub Actions Workflow.

Please, how can I be able to ensure that this nodejs/node#57199 (comment) does not affect the version support of v24?

This is primarily about documentation (compatibility of this library is not affected because the child_process API is not used, it is instead more like a peer dependency). The main question is whether the examples in the docs still make sense. If you don't feel familiar with this it can be handled in a separate PR.

@ericcornelissen ericcornelissen added enhancement New feature or request and removed dependencies Changes to the project's dependencies labels May 7, 2025
@Tosin-JD
Copy link
Contributor Author

Tosin-JD commented May 7, 2025

Thank you so much for your feedback. I have made the changes based on your feedback.

This is primarily about documentation (compatibility of this library is not affected because the child_process API is not used, it is instead more like a peer dependency). The main question is whether the examples in the docs still make sense. If you don't feel familiar with this it can be handled in a separate PR.

I would like to handle the above. However, I will need some guidance. Thank you for the opportunity to contribute.

@github-actions github-actions bot added dependencies Changes to the project's dependencies ci/cd Relates to ci/cd labels May 7, 2025
@ericcornelissen ericcornelissen linked an issue May 7, 2025 that may be closed by this pull request
@ericcornelissen
Copy link
Owner

Thank you so much for your feedback. I have made the changes based on your feedback.

👍

I would like to handle the above. However, I will need some guidance. Thank you for the opportunity to contribute.

To be able to do this, first understand what the change in Node.js entails. Then go through the usages of the affected APIs in this project (execFile and spawn) and check/test if they're affected. If they are, update the use if it's in code or add a note referencing the issue about it if it's in the docs (here's an example note for reference).

Having said that, if you're not familiar with this topic it's easier for both of us if you do not attempt to make this particular contribution because it's a nuanced and security sensitive change. There's no shame in making a contribution that's small.

@ericcornelissen ericcornelissen changed the title added Support Node.js v24 #1966 added Support Node.js v24 May 7, 2025
@ericcornelissen ericcornelissen removed the dependencies Changes to the project's dependencies label May 7, 2025
@ericcornelissen

This comment was marked as resolved.

@github-actions github-actions bot added the dependencies Changes to the project's dependencies label May 7, 2025
@ericcornelissen ericcornelissen self-assigned this May 8, 2025
@ericcornelissen ericcornelissen changed the title added Support Node.js v24 Add support for Node.js v24 May 8, 2025
@ericcornelissen
Copy link
Owner

@Tosin-JD could you provide an update on your intentions with this PR? Are you intended to look into the referenced issue or leave the changes as is?

@Tosin-JD
Copy link
Contributor Author

Tosin-JD commented May 9, 2025

Please sir, currently I cannot handle the second part that has to do with spawn so, I do not intend to handle it. I hope that there will be other issues that I can handle in the future so that I can keep contributing while I learn. Thank you

@ericcornelissen ericcornelissen merged commit 18d7bff into ericcornelissen:main May 9, 2025
40 checks passed
@ericcornelissen ericcornelissen removed the dependencies Changes to the project's dependencies label May 11, 2025
@ericcornelissen ericcornelissen removed the ci/cd Relates to ci/cd label May 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support Node.js v24

2 participants