Skip to content

Read version from .nvmrc or .tool-versions config #222

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Read version from .nvmrc or .tool-versions config #222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 25 additions & 4 deletions dist/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4695,17 +4695,15 @@ const auth = __importStar(__webpack_require__(202));
const path = __importStar(__webpack_require__(622));
const url_1 = __webpack_require__(835);
const os = __webpack_require__(87);
const fs = __webpack_require__(747);
function run() {
return __awaiter(this, void 0, void 0, function* () {
try {
//
// Version is optional. If supplied, install / use from the tool cache
// If not supplied then task is still used to setup proxy, auth, etc...
//
let version = core.getInput('node-version');
if (!version) {
version = core.getInput('version');
}
let version = parseNodeVersion();
let arch = core.getInput('architecture');
// if architecture supplied but node-version is not
// if we don't throw a warning, the already installed x64 node will be used which is not probably what user meant.
Expand Down Expand Up @@ -4742,6 +4740,29 @@ function isGhes() {
const ghUrl = new url_1.URL(process.env['GITHUB_SERVER_URL'] || 'https://github.com');
return ghUrl.hostname.toUpperCase() !== 'GITHUB.COM';
}
function parseNodeVersion() {
let nodeVersion = core.getInput('node-version') || core.getInput('version');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just wanted to ask here whether you want to check for a falsy value or just null or undefined? If the latter:

Suggested change
let nodeVersion = core.getInput('node-version') || core.getInput('version');
let nodeVersion = core.getInput('node-version') ?? core.getInput('version');

if (!nodeVersion) {
if (fs.existsSync('.nvmrc')) {
// Read from .nvmrc
nodeVersion = fs.readFileSync('.nvmrc', 'utf8').trim();
console.log(`Using ${nodeVersion} as input from file .nvmrc`);
}
else if (fs.existsSync('.tool-versions')) {
// Read from .tool-versions
const toolVersions = fs.readFileSync('.tool-versions', 'utf8').trim();
const nodeLine = toolVersions
.split(/\r?\n/)
.filter(e => e.match(/^nodejs\s/))[0];
nodeVersion = nodeLine.match(/^nodejs\s+(.+)$/)[1];
console.log(`Using ${nodeVersion} as input from file .tool-versions`);
}
else {
console.log(`Version not specified and not found in .nvmrc or .tool-versions`);
}
}
return nodeVersion;
}
//# sourceMappingURL=main.js.map

/***/ }),
Expand Down
33 changes: 28 additions & 5 deletions src/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,15 @@ import * as auth from './authutil';
import * as path from 'path';
import {URL} from 'url';
import os = require('os');
import fs = require('fs');

export async function run() {
try {
//
// Version is optional. If supplied, install / use from the tool cache
// If not supplied then task is still used to setup proxy, auth, etc...
//
let version = core.getInput('node-version');
if (!version) {
version = core.getInput('version');
}

let version = parseNodeVersion();
let arch = core.getInput('architecture');

// if architecture supplied but node-version is not
Expand Down Expand Up @@ -64,3 +61,29 @@ function isGhes(): boolean {
);
return ghUrl.hostname.toUpperCase() !== 'GITHUB.COM';
}

function parseNodeVersion(): string {
let nodeVersion = core.getInput('node-version') || core.getInput('version');

if (!nodeVersion) {
if (fs.existsSync('.nvmrc')) {
// Read from .nvmrc
nodeVersion = fs.readFileSync('.nvmrc', 'utf8').trim();
console.log(`Using ${nodeVersion} as input from file .nvmrc`);
} else if (fs.existsSync('.tool-versions')) {
// Read from .tool-versions
const toolVersions = fs.readFileSync('.tool-versions', 'utf8').trim();
const nodeLine = toolVersions
.split(/\r?\n/)
.filter(e => e.match(/^nodejs\s/))[0];
nodeVersion = nodeLine.match(/^nodejs\s+(.+)$/)![1];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.match() returns RegExpMatchArray | null, so rather than using definite assignment assertion (!), perhaps use the optional chain (?) to query the RegExpMatchArray. If nothing is returned here, it will assign undefined to nodeVersion.

This does then throw into question the flow of these statements, because if I have a .tool-versions with the nodejs key in (I think that's what you're testing?) but no value, then it will return undefined.

I would revisit this by allowing TS to guide you with the types & then making sure that a string is definitely being returned.

Suggested change
nodeVersion = nodeLine.match(/^nodejs\s+(.+)$/)![1];
nodeVersion = nodeLine.match(/^nodejs\s+(.+)$/)?.[1];

console.log(`Using ${nodeVersion} as input from file .tool-versions`);
} else {
console.log(
`Version not specified and not found in .nvmrc or .tool-versions`
);
}
}

return nodeVersion;
}