ci: don't install latest npm #168
Conversation
|
Is there precedence for this solution? Curious if other repositories have to do this too or if there is a simpler solution. |
|
Other repos I know just don't bother upgrading npm, and will use whatever version comes bundled with Node.js via |
|
I see. I don't see why we need to manually install/update npm in each of our Node version tests. If we need to install the latest npm manually at all, likely we only need to do that alongside the latest Node version, and not bother touching it for the other Node version tests. That way, we don't have to maintain this mapping of Node versions to npm versions. |
|
Fair point. The npm upgrade was introduced in this commit alongside other changes. At that time there were only three versions of Node.js to be tested, and the latest npm was version 7. I don't know what was the reason for manually upgrading npm, but if it's no longer necessary, I'd be happy to take it out and stick with the pre-installed version. |
|
Yeah, I don't think this is necessary. I'd rather update the supported Node.js versions and use the bundled npm to fix this problem. We're free to introduce breaking changes here whenever we want because this package is a CLI tool that generates code, so there are no real backcompat concerns. |
fasttime
changed the title
|
I have removed the step that was installing npm. There is already PR #167 to add Node.js 21 to the list of versions to be tested. |
This PR fixes the CI test jobs.
Currently, the jobs will attempt to install the latests version of npm regardless of the installed version of Node.js. This has been problematic since the release of npm 10, because npm 10 does not support all versions of Node.js that this package supports.
The fix is to explicitly indicate which version of npm (currently 9 or 10) should be installed along with Node.js.Upgrading npm doesn't seem necessary, so removing that step will fix the build.