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

Prefer node-version-file: package.json over $PATH by default #912

Open
fregante opened this issue Dec 11, 2023 · 10 comments
Open

Prefer node-version-file: package.json over $PATH by default #912

fregante opened this issue Dec 11, 2023 · 10 comments
Labels
feature request New feature or request to improve the current logic

Comments

@fregante
Copy link

Description:

- uses: actions/setup-node@v5

Currently this uses what's available in $PATH, but I reckon it should prefer what the user already specified as a compatible node version in the package.json.

Justification:

It feels strange that a project clearly and "natively" specifies what version it's expected to run on, but the environment ignores it, requiring further config.

I love that node-version-file: package.json is possible now, but I wish I didn't have to clutter all of my jobs with

- uses: actions/setup-node@v4
  with: 
    node-version-file: package.json

. . . 


- uses: actions/setup-node@v4
  with: 
    node-version-file: package.json

. . .

- uses: actions/setup-node@v4
  with: 
    node-version-file: package.json

The drawback is that in the rare situation where the user wants to use what's in PATH, as opposed to what's in package.json, now they'd need something like node-version: PATH. But again this feels like it should be the minority.

Are you willing to submit a PR?

Potentially

@fregante fregante added feature request New feature or request to improve the current logic needs triage labels Dec 11, 2023
@marko-zivic-93
Copy link
Contributor

Hello @fregante
Thank you for creating this issue. We will investigate and come back to you as soon as we have some feedback.

@aparnajyothi-y
Copy link
Contributor

Hello @fregante, We've merged the fix for the issue as part of another issue. Could you please try to use actions/setup-node@main to confirm that it works as expected?

@fregante
Copy link
Author

fregante commented Jan 9, 2024

@aparnajyothi-y
Copy link
Contributor

Hello @fregante, Thank you for testing.
Could you please replace the setup-action repository actions/setup-node@v4 in your workflow with actions/setup-node@main because we have merged the code as part of this PR and yet to release.
Please replace the branch with main and update the results. Hope that should fulfil your requirement.

@aparnajyothi-y aparnajyothi-y self-assigned this Jan 18, 2024
@fregante
Copy link
Author

fregante commented Jan 18, 2024

we have merged the code as part of this PR

That PR is unrelated to this feature request.

The tests on main specifically verify the opposite of this feature request, right?

it('does not read node-version-file if node-version-file is not provided', async () => {
// Arrange
delete inputs['node-version-file'];

@hilja
Copy link

hilja commented Jan 19, 2024

Great issue and great research. I was just wondering the same thing, would be great if the version would just work™

@kleinfreund
Copy link

Additionally, this would provide a nice way to configure both the node and npm version. Currently, actions/setup-node will install a npm version that does generally not match pkg.engines.npm. If one wants to ensure that the npm version is the same as the one used locally, one has to run npm install --global npm@... after actions/setup-node which is cumbersome.

@fregante
Copy link
Author

fregante commented Apr 23, 2024

@kleinfreund that's unrelated, this request is exclusively about changing a default. For what you're asking refer to:

@mk-pmb
Copy link

mk-pmb commented Jun 12, 2024

I added this one to my suggested way of merging all the version number oracles over here in issue 939.

I wish I didn't have to clutter all of my jobs with

The many repetitions of the same "with:" parameters are not specifically related to this one option. It should thus be an issue of its own, and should probably be solved by a config file mechanism.
The current best stopgap seems to be to make a separate workflow job for installing node, and have your other jobs depend on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request to improve the current logic
Projects
None yet
Development

No branches or pull requests

8 participants
@fregante @hilja @mk-pmb @kleinfreund @marko-zivic-93 @aparnajyothi-y @shaanmugapriya and others