node-version as fallback of node-version-file

[y-nk]

Description

Currently the behavior is that node-version is preferred rather than node-version-file if both are provided.

setup-node/src/main.ts

Lines 85 to 95 in d86ebcd

	if (version && versionFileInput) {
	core.warning(
	'Both node-version and node-version-file inputs are specified, only node-version will be used'
	);
	}
	if (version) {
	return version;
	}
	if (versionFileInput) {

I would like to propose the opposite since node-version is actually hardcoded in the action parameters, but the file is not guaranteed to be there.

setup-node/src/util.ts

Lines 8 to 12 in d86ebcd

	if (!fs.existsSync(versionFilePath)) {
	throw new Error(
	`The specified node version file at: ${versionFilePath} does not exist`
	);
	}

Currently, scenarios resolved as following:

with:
  node-version: 8 # nothing to see here

with:
  node-version-file: fileExists.json # all good

with:
  node-version-file: fileDoesNotExists.json # throws error and crashes the pipeline

with:
  node-version: 8
  node-version-file: fileExists.json # ignored, picked version is 8

with:
  node-version: 8
  node-version-file: fileDoesNotExists.json # ignored, picked version is 8

What I'm requesting for is a change in the 2 lasts scenarios:

with:
  node-version: 8
  node-version-file: fileExists.json # has priority, so it's picked

with:
  node-version: 8
  node-version-file: fileDoesNotExists.json # does not throw and picks version 8 instead

Justification

This is just a nicer API, i think. it does not remove the case of conscious crash of the pipeline if no node-version is given, but it does nicely propose a fail-safe for people interested.

Note

We could also have another parameter such as node-version-fallback-if-file-not-found but it'd just increase the complexity of the api.

Are you willing to submit a PR?

Yes, i'm ok with that.

[aparnajyothi-y]

Hello @y-nk
Thank you for creating this issue, we will investigate it and come back to you as soon as we have some feedback.

[fregante]

To me this feels backwards; generally, config files are the default and "inline settings/flags" override the defaults. If you're specifying a node-version, then you most likely prefer that one over the "project-level" version defined in the package.json

[y-nk]

@fregante that is untrue, especially in monorepos where the node-version file can be there (or not) depending on the package/service running the job, but you could always use a hardcoded base version as a fallback.

[fregante]

I don’t think "monorepos" are the standard. I'm thinking more about CLIs and literally everything else:

1. Use call flag
2. Use local config
3. Use global config
4. Use default

I don’t think it should be anything but that.

[y-nk]

@fregante i understand your standpoint. as a mean not to remain unblocked, would you have any recommendation for something less verbose/more elegant than this:

    - if: ${{ !inputs.workspaceRelativePath }}
      uses: actions/setup-node@v4
      with:
        node-version: 20
        cache: 'pnpm'
        registry-url: https://npm.pkg.github.com/
        scope: '@company'

    - if: ${{ inputs.workspaceRelativePath }}
      uses: actions/setup-node@v4
      with:
        node-version-file: ${{ format('{0}/.node-version', inputs.workspaceRelativePath) }}
        cache: 'pnpm'
        registry-url: https://npm.pkg.github.com/
        scope: '@company'

?